From 934a08b254e99acb3fce4282170b612ee2cf286e Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Sat, 4 Jun 2022 11:04:55 +0000 Subject: Creating the document metastore read guard is expensive and is not necessary to do for every imported attribute. We do it once per metastore and cache it in the ImportedAttributeContext. It would be even better if we could drop support for the default makeReadGuard(bool). Then we would also avoid copying the shared_ptr. --- .../attribute/imported_attribute_vector.cpp | 8 ++++- .../attribute/imported_attribute_vector.h | 3 ++ .../imported_attribute_vector_read_guard.cpp | 8 ++--- .../imported_attribute_vector_read_guard.h | 38 ++++++++++------------ .../tensor/imported_tensor_attribute_vector.cpp | 8 ++++- .../tensor/imported_tensor_attribute_vector.h | 2 ++ ...imported_tensor_attribute_vector_read_guard.cpp | 6 ++-- .../imported_tensor_attribute_vector_read_guard.h | 3 +- 8 files changed, 46 insertions(+), 30 deletions(-) (limited to 'searchlib') diff --git a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector.cpp b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector.cpp index 1ae2b6ee749..6218f3d72b2 100644 --- a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector.cpp +++ b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector.cpp @@ -43,7 +43,13 @@ ImportedAttributeVector::~ImportedAttributeVector() = default; std::unique_ptr ImportedAttributeVector::makeReadGuard(bool stableEnumGuard) const { - return std::make_unique(*this, stableEnumGuard); + return makeReadGuard(_target_document_meta_store->getReadGuard(), stableEnumGuard); +} + +std::unique_ptr +ImportedAttributeVector::makeReadGuard(std::shared_ptr targetMetaStoreReadGuard, bool stableEnumGuard) const +{ + return std::make_unique(std::move(targetMetaStoreReadGuard), *this, stableEnumGuard); } void ImportedAttributeVector::clearSearchCache() { diff --git a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector.h b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector.h index 0e631287708..29172ef1831 100644 --- a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector.h +++ b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector.h @@ -3,6 +3,7 @@ #pragma once #include "readable_attribute_vector.h" +#include #include namespace search { struct IDocumentMetaStoreContext; } @@ -26,6 +27,7 @@ class ReferenceAttribute; class ImportedAttributeVector : public ReadableAttributeVector { public: using SP = std::shared_ptr; + using MetaStoreReadGuard = search::IDocumentMetaStoreContext::IReadGuard; ImportedAttributeVector(vespalib::stringref name, std::shared_ptr reference_attribute, std::shared_ptr document_meta_store, @@ -61,6 +63,7 @@ public: } std::unique_ptr makeReadGuard(bool stableEnumGuard) const override; + virtual std::unique_ptr makeReadGuard(std::shared_ptr targetMetaStoreReadGuard, bool stableEnumGuard) const; protected: vespalib::string _name; diff --git a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.cpp b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.cpp index a6a0dac9097..489b2fb5e6e 100644 --- a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.cpp +++ b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.cpp @@ -10,11 +10,11 @@ namespace search::attribute { -ImportedAttributeVectorReadGuard::ImportedAttributeVectorReadGuard( - const ImportedAttributeVector &imported_attribute, - bool stableEnumGuard) +ImportedAttributeVectorReadGuard::ImportedAttributeVectorReadGuard(std::shared_ptr targetMetaStoreReadGuard, + const ImportedAttributeVector &imported_attribute, + bool stableEnumGuard) : AttributeReadGuard(this), - _target_document_meta_store_read_guard(imported_attribute.getTargetDocumentMetaStore()->getReadGuard()), + _target_document_meta_store_read_guard(std::move(targetMetaStoreReadGuard)), _imported_attribute(imported_attribute), _targetLids(), _reference_attribute_guard(imported_attribute.getReferenceAttribute()), diff --git a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.h b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.h index 233ce5d06df..fd9856a032c 100644 --- a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.h +++ b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.h @@ -31,26 +31,9 @@ class ImportedAttributeVectorReadGuard : public IAttributeVector, public AttributeReadGuard, public IMultiValueAttribute { -private: - using AtomicTargetLid = vespalib::datastore::AtomicValueWrapper; - using TargetLids = vespalib::ConstArrayRef; - IDocumentMetaStoreContext::IReadGuard::UP _target_document_meta_store_read_guard; - const ImportedAttributeVector &_imported_attribute; - TargetLids _targetLids; - AttributeGuard _reference_attribute_guard; - std::unique_ptr _target_attribute_guard; - const ReferenceAttribute &_reference_attribute; -protected: - const IAttributeVector &_target_attribute; - -protected: - uint32_t getTargetLid(uint32_t lid) const { - // Check range to avoid reading memory beyond end of mapping array - return lid < _targetLids.size() ? _targetLids[lid].load_acquire() : 0u; - } - public: - ImportedAttributeVectorReadGuard(const ImportedAttributeVector &imported_attribute, bool stableEnumGuard); + using MetaStoreReadGuard = search::IDocumentMetaStoreContext::IReadGuard; + ImportedAttributeVectorReadGuard(std::shared_ptr targetMetaStoreReadGuard, const ImportedAttributeVector &imported_attribute, bool stableEnumGuard); ~ImportedAttributeVectorReadGuard() override; const vespalib::string &getName() const override; @@ -106,8 +89,23 @@ public: const IWeightedSetReadView* make_read_view(WeightedSetTag tag, vespalib::Stash& stash) const override; const IArrayEnumReadView* make_read_view(ArrayEnumTag tag, vespalib::Stash& stash) const override; const IWeightedSetEnumReadView* make_read_view(WeightedSetEnumTag tag, vespalib::Stash& stash) const override; - +private: + using AtomicTargetLid = vespalib::datastore::AtomicValueWrapper; + using TargetLids = vespalib::ConstArrayRef; + std::shared_ptr _target_document_meta_store_read_guard; + const ImportedAttributeVector &_imported_attribute; + TargetLids _targetLids; + AttributeGuard _reference_attribute_guard; + std::unique_ptr _target_attribute_guard; + const ReferenceAttribute &_reference_attribute; protected: + const IAttributeVector &_target_attribute; + + uint32_t getTargetLid(uint32_t lid) const { + // Check range to avoid reading memory beyond end of mapping array + return lid < _targetLids.size() ? _targetLids[lid].load_acquire() : 0u; + } + long onSerializeForAscendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const override; long onSerializeForDescendingSort(DocId doc, void * serTo, long available, diff --git a/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector.cpp b/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector.cpp index 7520d7e81d2..e532c609101 100644 --- a/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector.cpp +++ b/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector.cpp @@ -38,7 +38,13 @@ ImportedTensorAttributeVector::~ImportedTensorAttributeVector() = default; std::unique_ptr ImportedTensorAttributeVector::makeReadGuard(bool stableEnumGuard) const { - return std::make_unique(*this, stableEnumGuard); + return makeReadGuard(_target_document_meta_store->getReadGuard(), stableEnumGuard); +} + +std::unique_ptr +ImportedTensorAttributeVector::makeReadGuard(std::shared_ptr targetMetaStoreReadGuard, bool stableEnumGuard) const +{ + return std::make_unique(std::move(targetMetaStoreReadGuard), *this, stableEnumGuard); } } diff --git a/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector.h b/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector.h index 853b2b54beb..4dc4e6a619a 100644 --- a/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector.h +++ b/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector.h @@ -31,7 +31,9 @@ public: std::shared_ptr search_cache); ~ImportedTensorAttributeVector() override; + // TODO balder: Can we fail the default makeReadGuard, and only use a reference for the targetMetaStoreReadGuard std::unique_ptr makeReadGuard(bool stableEnumGuard) const override; + std::unique_ptr makeReadGuard(std::shared_ptr targetMetaStoreReadGuard, bool stableEnumGuard) const override; }; } diff --git a/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector_read_guard.cpp b/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector_read_guard.cpp index 3b9abcf6420..a9bc69c36c9 100644 --- a/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector_read_guard.cpp +++ b/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector_read_guard.cpp @@ -18,10 +18,10 @@ getTensorAttribute(const search::attribute::IAttributeVector &attr) } -ImportedTensorAttributeVectorReadGuard::ImportedTensorAttributeVectorReadGuard(const attribute::ImportedAttributeVector &imported_attribute, +ImportedTensorAttributeVectorReadGuard::ImportedTensorAttributeVectorReadGuard(std::shared_ptr targetMetaStoreReadGuard, + const attribute::ImportedAttributeVector &imported_attribute, bool stableEnumGuard) - : ImportedAttributeVectorReadGuard(imported_attribute, - stableEnumGuard), + : ImportedAttributeVectorReadGuard(std::move(targetMetaStoreReadGuard), imported_attribute, stableEnumGuard), _target_tensor_attribute(getTensorAttribute(_target_attribute)) { } diff --git a/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector_read_guard.h b/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector_read_guard.h index d0f09da9281..acfd1821e1f 100644 --- a/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector_read_guard.h +++ b/searchlib/src/vespa/searchlib/tensor/imported_tensor_attribute_vector_read_guard.h @@ -24,7 +24,8 @@ class ImportedTensorAttributeVectorReadGuard : public attribute::ImportedAttribu using BitVectorSearchCache = attribute::BitVectorSearchCache; const ITensorAttribute &_target_tensor_attribute; public: - ImportedTensorAttributeVectorReadGuard(const attribute::ImportedAttributeVector &imported_attribute, + ImportedTensorAttributeVectorReadGuard(std::shared_ptr targetMetaStoreReadGuard, + const attribute::ImportedAttributeVector &imported_attribute, bool stableEnumGuard); ~ImportedTensorAttributeVectorReadGuard(); -- cgit v1.2.3