diff options
7 files changed, 69 insertions, 25 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)); diff --git a/searchlib/src/vespa/searchcommon/attribute/iattributecontext.h b/searchlib/src/vespa/searchcommon/attribute/iattributecontext.h index 9c89b6a0f8b..cf7b1d2f959 100644 --- a/searchlib/src/vespa/searchcommon/attribute/iattributecontext.h +++ b/searchlib/src/vespa/searchcommon/attribute/iattributecontext.h @@ -46,6 +46,11 @@ public: virtual void releaseEnumGuards() {} /** + * Must be called before multiple threads will access the context. + */ + virtual void enableMultiThreadSafe() {} + + /** * Virtual destructor to allow safe subclassing. **/ virtual ~IAttributeContext() = default; diff --git a/searchlib/src/vespa/searchlib/attribute/attributecontext.cpp b/searchlib/src/vespa/searchlib/attribute/attributecontext.cpp index 3c85c076eb2..97a7dc8bcb1 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributecontext.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributecontext.cpp @@ -30,8 +30,15 @@ AttributeContext::getAttribute(AttributeMap & map, const string & name, bool sta } } +const IAttributeVector * +AttributeContext::getAttributeMtSafe(AttributeMap &map, const string &name, bool stableEnum) const { + std::lock_guard<std::mutex> guard(_cacheLock); + return getAttribute(map, name, stableEnum); +} + AttributeContext::AttributeContext(const IAttributeManager & manager) : _manager(manager), + _mtSafe(false), _attributes(), _enumAttributes(), _cacheLock() @@ -42,20 +49,26 @@ AttributeContext::~AttributeContext() = default; const IAttributeVector * AttributeContext::getAttribute(const string & name) const { - std::lock_guard<std::mutex> guard(_cacheLock); - return getAttribute(_attributes, name, false); + return _mtSafe + ? getAttributeMtSafe(_attributes, name, false) + : getAttribute(_attributes, name, false); } const IAttributeVector * AttributeContext::getAttributeStableEnum(const string & name) const { - std::lock_guard<std::mutex> guard(_cacheLock); - return getAttribute(_enumAttributes, name, true); + return _mtSafe + ? getAttributeMtSafe(_enumAttributes, name, true) + : getAttribute(_enumAttributes, name, true); } void AttributeContext::releaseEnumGuards() { - std::lock_guard<std::mutex> guard(_cacheLock); - _enumAttributes.clear(); + if (_mtSafe) { + std::lock_guard<std::mutex> guard(_cacheLock); + _enumAttributes.clear(); + } else { + _enumAttributes.clear(); + } } void diff --git a/searchlib/src/vespa/searchlib/attribute/attributecontext.h b/searchlib/src/vespa/searchlib/attribute/attributecontext.h index 8b8cf5daead..28b05a76f65 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributecontext.h +++ b/searchlib/src/vespa/searchlib/attribute/attributecontext.h @@ -21,12 +21,13 @@ private: using IAttributeFunctor = attribute::IAttributeFunctor; const IAttributeManager & _manager; + bool _mtSafe; mutable AttributeMap _attributes; mutable AttributeMap _enumAttributes; mutable std::mutex _cacheLock; const IAttributeVector *getAttribute(AttributeMap & map, const string & name, bool stableEnum) const; - + const IAttributeVector *getAttributeMtSafe(AttributeMap & map, const string & name, bool stableEnum) const; public: AttributeContext(const IAttributeManager & manager); ~AttributeContext() override; @@ -37,6 +38,7 @@ public: const attribute::IAttributeVector * getAttributeStableEnum(const string & name) const override; void getAttributeList(std::vector<const IAttributeVector *> & list) const override; void releaseEnumGuards() override; + void enableMultiThreadSafe() override { _mtSafe = true; } // Give acces to the underlying manager const IAttributeManager & getManager() const { return _manager; } |