From 3b2428cd74d5369dd74ff290cad4cfd2aef5567d Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 16 Nov 2023 15:02:29 +0000 Subject: Do not hold the guard when creating the iterator. --- .../proton/matching/attribute_limiter.cpp | 23 ++++++++++++++-------- .../searchcore/proton/matching/attribute_limiter.h | 1 + 2 files changed, 16 insertions(+), 8 deletions(-) (limited to 'searchcore') diff --git a/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp b/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp index 4dec253946f..497e0b36fc7 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp @@ -76,19 +76,19 @@ toString(AttributeLimiter::DiversityCutoffStrategy strategy) return (strategy == AttributeLimiter::DiversityCutoffStrategy::STRICT) ? STRICT_STR : LOOSE_STR; } -std::unique_ptr -AttributeLimiter::create_search(size_t want_hits, size_t max_group_size, double hit_rate, bool strictSearch) -{ +search::fef::MatchData & +AttributeLimiter::create_match_data(size_t want_hits, size_t max_group_size, double hit_rate, bool strictSearch) { std::lock_guard guard(_lock); const uint32_t my_field_id = 0; search::fef::MatchDataLayout layout; auto my_handle = layout.allocTermField(my_field_id); - if ( ! _blueprint ) { + if (!_blueprint) { RangeLimitMetaInfo rangeInfo = _rangeQueryLocator.locate(); const uint32_t no_unique_id = 0; - string range_spec = fmt("[%s;%s;%s%zu", rangeInfo.low().c_str(), rangeInfo.high().c_str(), _descending? "-" : "", want_hits); + string range_spec = fmt("[%s;%s;%s%zu", rangeInfo.low().c_str(), rangeInfo.high().c_str(), + _descending ? "-" : "", want_hits); if (max_group_size < want_hits) { - size_t cutoffGroups = (_diversityCutoffFactor*want_hits)/max_group_size; + size_t cutoffGroups = (_diversityCutoffFactor * want_hits) / max_group_size; range_spec.append(fmt(";%s;%zu;%zu;%s]", _diversity_attribute.c_str(), max_group_size, cutoffGroups, toString(_diversityCutoffStrategy).c_str())); } else { @@ -99,12 +99,19 @@ AttributeLimiter::create_search(size_t want_hits, size_t max_group_size, double FieldSpecList field; // single field API is protected field.add(FieldSpec(_attribute_name, my_field_id, my_handle)); _blueprint = _searchable_attributes.createBlueprint(_requestContext, field, node); - _blueprint->fetchPostings(ExecuteInfo::create(strictSearch, strictSearch ? 1.0F : hit_rate, &_requestContext.getDoom())); + auto execInfo = ExecuteInfo::create(strictSearch, strictSearch ? 1.0F : hit_rate, &_requestContext.getDoom()); + _blueprint->fetchPostings(execInfo); _estimatedHits.store(_blueprint->getState().estimate().estHits, std::memory_order_relaxed); _blueprint->freeze(); } _match_datas.push_back(layout.createMatchData()); - return _blueprint->createSearch(*_match_datas.back(), strictSearch); + return *_match_datas.back(); +} + +std::unique_ptr +AttributeLimiter::create_search(size_t want_hits, size_t max_group_size, double hit_rate, bool strictSearch) { + search::fef::MatchData & matchData = create_match_data(want_hits, max_group_size, hit_rate, strictSearch); + return _blueprint->createSearch(matchData, strictSearch); } } diff --git a/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.h b/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.h index df0acccbd7a..1f9a1cebd8b 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.h +++ b/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.h @@ -46,6 +46,7 @@ public: ssize_t getEstimatedHits() const; static DiversityCutoffStrategy toDiversityCutoffStrategy(vespalib::stringref strategy); private: + search::fef::MatchData & create_match_data(size_t want_hits, size_t max_group_size, double hit_rate, bool strictSearch); search::queryeval::Searchable & _searchable_attributes; const search::queryeval::IRequestContext & _requestContext; const RangeQueryLocator & _rangeQueryLocator; -- cgit v1.2.3 From d8cd6277df664aadb35f84dc3d78d6038b244539 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 17 Nov 2023 14:22:16 +0000 Subject: Use a pair as suggested in code review --- .../src/vespa/searchcore/proton/matching/attribute_limiter.cpp | 8 ++++---- .../src/vespa/searchcore/proton/matching/attribute_limiter.h | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) (limited to 'searchcore') diff --git a/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp b/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp index 497e0b36fc7..5d2269b87cb 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp @@ -76,7 +76,7 @@ toString(AttributeLimiter::DiversityCutoffStrategy strategy) return (strategy == AttributeLimiter::DiversityCutoffStrategy::STRICT) ? STRICT_STR : LOOSE_STR; } -search::fef::MatchData & +AttributeLimiter::BlueprintAndMatchData AttributeLimiter::create_match_data(size_t want_hits, size_t max_group_size, double hit_rate, bool strictSearch) { std::lock_guard guard(_lock); const uint32_t my_field_id = 0; @@ -105,13 +105,13 @@ AttributeLimiter::create_match_data(size_t want_hits, size_t max_group_size, dou _blueprint->freeze(); } _match_datas.push_back(layout.createMatchData()); - return *_match_datas.back(); + return {*_blueprint, *_match_datas.back()}; } std::unique_ptr AttributeLimiter::create_search(size_t want_hits, size_t max_group_size, double hit_rate, bool strictSearch) { - search::fef::MatchData & matchData = create_match_data(want_hits, max_group_size, hit_rate, strictSearch); - return _blueprint->createSearch(matchData, strictSearch); + auto bp_and_md = create_match_data(want_hits, max_group_size, hit_rate, strictSearch); + return bp_and_md.first.createSearch(bp_and_md.second, strictSearch); } } diff --git a/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.h b/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.h index 1f9a1cebd8b..a2c73b63bee 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.h +++ b/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.h @@ -46,7 +46,8 @@ public: ssize_t getEstimatedHits() const; static DiversityCutoffStrategy toDiversityCutoffStrategy(vespalib::stringref strategy); private: - search::fef::MatchData & create_match_data(size_t want_hits, size_t max_group_size, double hit_rate, bool strictSearch); + using BlueprintAndMatchData = std::pair; + BlueprintAndMatchData create_match_data(size_t want_hits, size_t max_group_size, double hit_rate, bool strictSearch); search::queryeval::Searchable & _searchable_attributes; const search::queryeval::IRequestContext & _requestContext; const RangeQueryLocator & _rangeQueryLocator; -- cgit v1.2.3 From e60132d7f00ecd6d4514a3cb736e2f799ef0a9da Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 17 Nov 2023 14:49:32 +0000 Subject: Use structured capture --- searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'searchcore') diff --git a/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp b/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp index 5d2269b87cb..d07169a0d63 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp @@ -110,8 +110,8 @@ AttributeLimiter::create_match_data(size_t want_hits, size_t max_group_size, dou std::unique_ptr AttributeLimiter::create_search(size_t want_hits, size_t max_group_size, double hit_rate, bool strictSearch) { - auto bp_and_md = create_match_data(want_hits, max_group_size, hit_rate, strictSearch); - return bp_and_md.first.createSearch(bp_and_md.second, strictSearch); + auto [blueprint, match_data] = create_match_data(want_hits, max_group_size, hit_rate, strictSearch); + return blueprint.createSearch(match_data, strictSearch); } } -- cgit v1.2.3