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.

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();