Merge pull request #22913 from vespa-engine/balder/cache-the-document-metastore-readguard-in-the-single-contextv7.594.36

Creating the document metastore read guard is expensive and is not ne…

10 files changed, 60 insertions, 31 deletions

diff --git a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.cpp b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.cpp
index 0aaa88db859..376c84c12d6 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.cpp
+++ b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.cpp
@@ -23,7 +23,15 @@ ImportedAttributesContext::getOrCacheAttribute(const vespalib::string &name, Att
     }
     const ImportedAttributeVector::SP & result = _repo.get(name);
     if (result) {
-        auto insRes = attributes.emplace(name, result->makeReadGuard(stableEnumGuard));
+        auto metaItr = _metaStores.find(result->getTargetDocumentMetaStore().get());
+        std::shared_ptr<MetaStoreReadGuard> metaGuard;
+        if (metaItr == _metaStores.end()) {
+            metaGuard = result->getTargetDocumentMetaStore()->getReadGuard();
+            _metaStores.emplace(result->getTargetDocumentMetaStore().get(), metaGuard);
+        } else {
+            metaGuard = metaItr->second;
+        }
+        auto insRes = attributes.emplace(name, result->makeReadGuard(std::move(metaGuard), stableEnumGuard));
         return insRes.first->second->attribute();
     } else {
         return nullptr;
@@ -34,6 +42,7 @@ ImportedAttributesContext::ImportedAttributesContext(const ImportedAttributesRep
     : _repo(repo),
       _guardedAttributes(),
       _enumGuardedAttributes(),
+      _metaStores(),
       _cacheMutex()
 {
 }
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.h b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.h
index 4b7058fdb83..39fde4f5fb7 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.h
+++ b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.h
@@ -2,6 +2,7 @@
 #pragma once
 
 #include <vespa/searchcommon/attribute/iattributecontext.h>
+#include <vespa/searchlib/common/i_document_meta_store_context.h>
 #include <vespa/vespalib/stllike/hash_fun.h>
 #include <vespa/vespalib/stllike/hash_map.h>
 #include <mutex>
@@ -30,13 +31,16 @@ private:
     using IAttributeVector = search::attribute::IAttributeVector;
     using ImportedAttributeVector = search::attribute::ImportedAttributeVector;
     using IAttributeFunctor = search::attribute::IAttributeFunctor;
+    using MetaStoreReadGuard = search::IDocumentMetaStoreContext::IReadGuard;
 
     using AttributeCache = std::unordered_map<vespalib::string, std::unique_ptr<AttributeReadGuard>, vespalib::hash<vespalib::string>>;
+    using MetaStoreCache = std::unordered_map<const void *, std::shared_ptr<MetaStoreReadGuard>>;
     using LockGuard = std::lock_guard<std::mutex>;
 
     const ImportedAttributesRepo &_repo;
     mutable AttributeCache _guardedAttributes;
     mutable AttributeCache _enumGuardedAttributes;
+    mutable MetaStoreCache _metaStores;
     mutable std::mutex _cacheMutex;
 
     const IAttributeVector *getOrCacheAttribute(const vespalib::string &name, AttributeCache &attributes,
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<AttributeReadGuard>
 ImportedAttributeVector::makeReadGuard(bool stableEnumGuard) const
 {
-    return std::make_unique<ImportedAttributeVectorReadGuard>(*this, stableEnumGuard);
+    return makeReadGuard(_target_document_meta_store->getReadGuard(), stableEnumGuard);
+}
+
+std::unique_ptr<AttributeReadGuard>
+ImportedAttributeVector::makeReadGuard(std::shared_ptr<MetaStoreReadGuard> targetMetaStoreReadGuard,  bool stableEnumGuard) const
+{
+    return std::make_unique<ImportedAttributeVectorReadGuard>(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 <vespa/searchlib/common/i_document_meta_store_context.h>
 #include <vespa/vespalib/stllike/string.h>
 
 namespace search { struct IDocumentMetaStoreContext; }
@@ -26,6 +27,7 @@ class ReferenceAttribute;
 class ImportedAttributeVector : public ReadableAttributeVector {
 public:
     using SP = std::shared_ptr<ImportedAttributeVector>;
+    using MetaStoreReadGuard = search::IDocumentMetaStoreContext::IReadGuard;
     ImportedAttributeVector(vespalib::stringref name,
                             std::shared_ptr<ReferenceAttribute> reference_attribute,
                             std::shared_ptr<IDocumentMetaStoreContext> document_meta_store,
@@ -61,6 +63,7 @@ public:
     }
 
     std::unique_ptr<AttributeReadGuard> makeReadGuard(bool stableEnumGuard) const override;
+    virtual std::unique_ptr<AttributeReadGuard> makeReadGuard(std::shared_ptr<MetaStoreReadGuard> 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<MetaStoreReadGuard> 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<uint32_t>;
-    using TargetLids = vespalib::ConstArrayRef<AtomicTargetLid>;
-    IDocumentMetaStoreContext::IReadGuard::UP _target_document_meta_store_read_guard;
-    const ImportedAttributeVector   &_imported_attribute;
-    TargetLids                       _targetLids;
-    AttributeGuard                   _reference_attribute_guard;
-    std::unique_ptr<attribute::AttributeReadGuard> _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<MetaStoreReadGuard> targetMetaStoreReadGuard, const ImportedAttributeVector &imported_attribute, bool stableEnumGuard);
     ~ImportedAttributeVectorReadGuard() override;
 
     const vespalib::string &getName() const override;
@@ -106,8 +89,23 @@ public:
     const IWeightedSetReadView<const char*>* make_read_view(WeightedSetTag<const char*> 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<uint32_t>;
+    using TargetLids = vespalib::ConstArrayRef<AtomicTargetLid>;
+    std::shared_ptr<MetaStoreReadGuard>  _target_document_meta_store_read_guard;
+    const ImportedAttributeVector       &_imported_attribute;
+    TargetLids                           _targetLids;
+    AttributeGuard                       _reference_attribute_guard;
+    std::unique_ptr<attribute::AttributeReadGuard> _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<attribute::AttributeReadGuard>
 ImportedTensorAttributeVector::makeReadGuard(bool stableEnumGuard) const
 {
-    return std::make_unique<ImportedTensorAttributeVectorReadGuard>(*this, stableEnumGuard);
+    return makeReadGuard(_target_document_meta_store->getReadGuard(), stableEnumGuard);
+}
+
+std::unique_ptr<attribute::AttributeReadGuard>
+ImportedTensorAttributeVector::makeReadGuard(std::shared_ptr<MetaStoreReadGuard> targetMetaStoreReadGuard,  bool stableEnumGuard) const
+{
+    return std::make_unique<ImportedTensorAttributeVectorReadGuard>(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<BitVectorSearchCache> search_cache);
     ~ImportedTensorAttributeVector() override;
 
+    // TODO balder: Can we fail the default makeReadGuard, and only use a reference for the targetMetaStoreReadGuard
     std::unique_ptr<attribute::AttributeReadGuard> makeReadGuard(bool stableEnumGuard) const override;
+    std::unique_ptr<attribute::AttributeReadGuard> makeReadGuard(std::shared_ptr<MetaStoreReadGuard> 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<MetaStoreReadGuard> 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<MetaStoreReadGuard> targetMetaStoreReadGuard,
+                                           const attribute::ImportedAttributeVector &imported_attribute,
                                            bool stableEnumGuard);
     ~ImportedTensorAttributeVectorReadGuard();