The AttributeContext is a short lived cache for attributes guards. Until we use the thread bundle

we do not need to use expensive locking to fill the cache.
Most of the attributes are pulled in when building the blueprint tree, and that always happens singlethreaded.

4 files changed, 42 insertions, 18 deletions

diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp
index ee04f17d378..96d99008309 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp
+++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp
@@ -451,7 +451,7 @@ namespace {
 class CombinedAttributeContext : public IAttributeContext {
 private:
     using IAttributeFunctor = search::attribute::IAttributeFunctor;
-    AttributeContext _ctx;
+    AttributeContext          _ctx;
     ImportedAttributesContext _importedCtx;
 
 public:
@@ -483,6 +483,10 @@ public:
         _ctx.releaseEnumGuards();
         _importedCtx.releaseEnumGuards();
     }
+    void enableMultiThreadSafe() override {
+        _ctx.enableMultiThreadSafe();
+        _importedCtx.enableMultiThreadSafe();
+    }
     void asyncForAttribute(const vespalib::string &name, std::unique_ptr<IAttributeFunctor> func) const override {
         _ctx.asyncForAttribute(name, std::move(func));
     }
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 cb05ac7e9f8..ede86051efb 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.cpp
+++ b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.cpp
@@ -14,8 +14,7 @@ using LockGuard = std::lock_guard<std::mutex>;
 namespace proton {
 
 const IAttributeVector *
-ImportedAttributesContext::getOrCacheAttribute(const vespalib::string &name, AttributeCache &attributes,
-                                               bool stableEnumGuard, const LockGuard &) const
+ImportedAttributesContext::getOrCacheAttribute(const vespalib::string &name, AttributeCache &attributes, bool stableEnumGuard) const
 {
     auto itr = attributes.find(name);
     if (itr != attributes.end()) {
@@ -38,8 +37,15 @@ ImportedAttributesContext::getOrCacheAttribute(const vespalib::string &name, Att
     }
 }
 
+const IAttributeVector *
+ImportedAttributesContext::getOrCacheAttributeMtSafe(const vespalib::string &name, AttributeCache &attributes, bool stableEnumGuard) const {
+    LockGuard guard(_cacheMutex);
+    return getOrCacheAttribute(name, attributes, stableEnumGuard);
+}
+
 ImportedAttributesContext::ImportedAttributesContext(const ImportedAttributesRepo &repo)
     : _repo(repo),
+      _mtSafe(false),
       _guardedAttributes(),
       _enumGuardedAttributes(),
       _metaStores(),
@@ -52,15 +58,17 @@ ImportedAttributesContext::~ImportedAttributesContext() = default;
 const IAttributeVector *
 ImportedAttributesContext::getAttribute(const vespalib::string &name) const
 {
-    LockGuard guard(_cacheMutex);
-    return getOrCacheAttribute(name, _guardedAttributes, false, guard);
+    return _mtSafe
+        ? getOrCacheAttributeMtSafe(name, _guardedAttributes, false)
+        : getOrCacheAttribute(name, _guardedAttributes, false);
 }
 
 const IAttributeVector *
 ImportedAttributesContext::getAttributeStableEnum(const vespalib::string &name) const
 {
-    LockGuard guard(_cacheMutex);
-    return getOrCacheAttribute(name, _enumGuardedAttributes, true, guard);
+    return _mtSafe
+           ? getOrCacheAttributeMtSafe(name, _enumGuardedAttributes, true)
+           : getOrCacheAttribute(name, _enumGuardedAttributes, true);
 }
 
 void
@@ -76,8 +84,12 @@ ImportedAttributesContext::getAttributeList(std::vector<const IAttributeVector *
 void
 ImportedAttributesContext::releaseEnumGuards()
 {
-    LockGuard guard(_cacheMutex);
-    _enumGuardedAttributes.clear();
+    if (_mtSafe) {
+        LockGuard guard(_cacheMutex);
+        _enumGuardedAttributes.clear();
+    } else {
+        _enumGuardedAttributes.clear();
+    }
 }
 
 void
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 a90012a6ef8..aefffd959de 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.h
+++ b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.h
@@ -38,13 +38,14 @@ private:
     using LockGuard = std::lock_guard<std::mutex>;
 
     const ImportedAttributesRepo &_repo;
-    mutable AttributeCache _guardedAttributes;
-    mutable AttributeCache _enumGuardedAttributes;
-    mutable MetaStoreCache _metaStores;
-    mutable std::mutex     _cacheMutex;
+    bool                          _mtSafe;
+    mutable AttributeCache        _guardedAttributes;
+    mutable AttributeCache        _enumGuardedAttributes;
+    mutable MetaStoreCache        _metaStores;
+    mutable std::mutex            _cacheMutex;
 
-    const IAttributeVector *getOrCacheAttribute(const vespalib::string &name, AttributeCache &attributes,
-                                                bool stableEnumGuard, const LockGuard &) const;
+    const IAttributeVector *getOrCacheAttribute(const vespalib::string &name, AttributeCache &attributes, bool stableEnumGuard) const;
+    const IAttributeVector *getOrCacheAttributeMtSafe(const vespalib::string &name, AttributeCache &attributes, bool stableEnumGuard) const;
 
 public:
     ImportedAttributesContext(const ImportedAttributesRepo &repo);
@@ -55,6 +56,7 @@ public:
     const IAttributeVector *getAttributeStableEnum(const vespalib::string &name) const override;
     void getAttributeList(std::vector<const IAttributeVector *> &list) const override;
     void releaseEnumGuards() override;
+    void enableMultiThreadSafe() override { _mtSafe = true; }
 
     void asyncForAttribute(const vespalib::string &name, std::unique_ptr<IAttributeFunctor> func) const override;
 };
diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp
index e2c5f4ef559..d0c1f99af11 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp
+++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp
@@ -63,7 +63,8 @@ struct StupidMetaStore : search::IDocumentMetaStore {
     void foreach(const search::IGidToLidMapperVisitor &) const override { }
 };
 
-size_t numThreads(size_t hits, size_t minHits) {
+size_t
+numThreads(size_t hits, size_t minHits) {
     return static_cast<size_t>(std::ceil(double(hits) / double(minHits)));
 }
 
@@ -74,16 +75,17 @@ public:
         _threadBundle(threadBundle),
         _maxThreads(std::min(maxThreads, static_cast<uint32_t>(threadBundle.size())))
     { }
-private:
     size_t size() const override { return _maxThreads; }
     void run(vespalib::Runnable* const* targets, size_t cnt) override {
         _threadBundle.run(targets, cnt);
     }
+private:
     vespalib::ThreadBundle &_threadBundle;
     const uint32_t          _maxThreads;
 };
 
-bool willNotNeedRanking(const SearchRequest & request, const GroupingContext & groupingContext) {
+bool
+willNotNeedRanking(const SearchRequest & request, const GroupingContext & groupingContext) {
     return (!groupingContext.needRanking() && (request.maxhits == 0))
            || (!request.sortSpec.empty() && (request.sortSpec.find("[rank]") == vespalib::string::npos));
 }
@@ -221,6 +223,7 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl
         }
         const Properties *feature_overrides = &request.propertiesMap.featureOverrides();
         if (shouldCacheSearchSession) {
+            // These should have been moved instead.
             owned_objects.feature_overrides = std::make_unique<Properties>(*feature_overrides);
             feature_overrides = owned_objects.feature_overrides.get();
         }
@@ -248,6 +251,9 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl
         LimitedThreadBundleWrapper limitedThreadBundle(threadBundle, numThreadsPerSearch);
         MatchMaster master;
         uint32_t numParts = NumSearchPartitions::lookup(rankProperties, _rankSetup->getNumSearchPartitions());
+        if (limitedThreadBundle.size() > 1) {
+            attrContext.enableMultiThreadSafe();
+        }
         ResultProcessor::Result::UP result = master.match(request.trace(), params, limitedThreadBundle, *mtf, rp,
                                                           _distributionKey, numParts);
         my_stats = MatchMaster::getStats(std::move(master));