From 013f47959b88282e5ec51c267f6b4c5f48115242 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 26 May 2023 07:38:51 +0000 Subject: - Make the MatchContext value object movable. - Reduce code visibility. --- .../src/tests/proton/matching/matching_test.cpp | 7 +++--- .../searchcore/proton/matching/CMakeLists.txt | 1 + .../searchcore/proton/matching/blueprintbuilder.h | 6 ++--- .../searchcore/proton/matching/isearchcontext.h | 7 +----- .../searchcore/proton/matching/match_context.cpp | 19 +++++++++++++++ .../searchcore/proton/matching/match_context.h | 17 +++++--------- .../proton/matching/same_element_builder.h | 13 +++++++---- .../searchcore/proton/matching/search_session.cpp | 12 ++++++++-- .../searchcore/proton/matching/search_session.h | 13 ++++++----- .../vespa/searchcore/proton/server/matchview.cpp | 16 ++++++------- .../src/vespa/searchcore/proton/server/matchview.h | 27 ++++++++++------------ .../vespa/searchcore/proton/server/searchview.cpp | 2 +- 12 files changed, 78 insertions(+), 62 deletions(-) create mode 100644 searchcore/src/vespa/searchcore/proton/matching/match_context.cpp (limited to 'searchcore/src') diff --git a/searchcore/src/tests/proton/matching/matching_test.cpp b/searchcore/src/tests/proton/matching/matching_test.cpp index 130fb29c289..b59384f1493 100644 --- a/searchcore/src/tests/proton/matching/matching_test.cpp +++ b/searchcore/src/tests/proton/matching/matching_test.cpp @@ -394,10 +394,9 @@ struct MyWorld { SearchReply::UP performSearch(const SearchRequest & req, size_t threads) { Matcher::SP matcher = createMatcher(); - SearchSession::OwnershipBundle owned_objects; - owned_objects.search_handler = std::make_shared(matcher); - owned_objects.context = std::make_unique(std::make_unique(), - std::make_unique()); + SearchSession::OwnershipBundle owned_objects({std::make_unique(), + std::make_unique()}, + std::make_shared(matcher)); vespalib::SimpleThreadBundle threadBundle(threads); SearchReply::UP reply = matcher->match(req, threadBundle, searchContext, attributeContext, *sessionManager, metaStore, metaStore.getBucketDB(), diff --git a/searchcore/src/vespa/searchcore/proton/matching/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/matching/CMakeLists.txt index 7960d1d51b5..92c8ec8f441 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/matching/CMakeLists.txt @@ -12,6 +12,7 @@ vespa_add_library(searchcore_matching STATIC i_match_loop_communicator.cpp indexenvironment.cpp match_loop_communicator.cpp + match_context.cpp match_master.cpp match_params.cpp match_phase_limit_calculator.cpp diff --git a/searchcore/src/vespa/searchcore/proton/matching/blueprintbuilder.h b/searchcore/src/vespa/searchcore/proton/matching/blueprintbuilder.h index 6c9203ef52c..f1db05b814f 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/blueprintbuilder.h +++ b/searchcore/src/vespa/searchcore/proton/matching/blueprintbuilder.h @@ -6,6 +6,8 @@ #include #include +namespace search::queryeval { class IRequestContext; } + namespace proton::matching { struct BlueprintBuilder { @@ -14,9 +16,7 @@ struct BlueprintBuilder { * blueprint meta-data back into corresponding query tree nodes. */ static search::queryeval::Blueprint::UP - build(const search::queryeval::IRequestContext & requestContext, - search::query::Node &node, - ISearchContext &context); + build(const search::queryeval::IRequestContext & requestContext, search::query::Node &node, ISearchContext &context); }; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/isearchcontext.h b/searchcore/src/vespa/searchcore/proton/matching/isearchcontext.h index 51fca51d398..d3f17b10aac 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/isearchcontext.h +++ b/searchcore/src/vespa/searchcore/proton/matching/isearchcontext.h @@ -2,10 +2,9 @@ #pragma once -#include - #include +namespace search::queryeval { class Searchable; } namespace searchcorespi { class IndexSearchable; } namespace proton::matching { @@ -25,10 +24,6 @@ class ISearchContext protected: ISearchContext() = default; public: - /** - * Convenience typedef for an auto pointer to this interface. - **/ - using UP = std::unique_ptr; ISearchContext(const ISearchContext &) = delete; ISearchContext & operator = (const ISearchContext &) = delete; diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_context.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_context.cpp new file mode 100644 index 00000000000..2d8be596380 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/matching/match_context.cpp @@ -0,0 +1,19 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "match_context.h" +#include + +namespace proton::matching { + +MatchContext::MatchContext(std::unique_ptr attrCtx, std::unique_ptr searchCtx) noexcept + : _attrCtx(std::move(attrCtx)), + _searchCtx(std::move(searchCtx)) +{ + assert(_attrCtx); + assert(_searchCtx); +} + +MatchContext::MatchContext() noexcept = default; +MatchContext::~MatchContext() = default; + +} diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_context.h b/searchcore/src/vespa/searchcore/proton/matching/match_context.h index d3e5a87d34c..c12c1c5731d 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_context.h +++ b/searchcore/src/vespa/searchcore/proton/matching/match_context.h @@ -4,25 +4,20 @@ #include "isearchcontext.h" #include -#include namespace proton::matching { class MatchContext { using IAttributeContext = search::attribute::IAttributeContext; - IAttributeContext::UP _attrCtx; - ISearchContext::UP _searchCtx; - + std::unique_ptr _attrCtx; + std::unique_ptr _searchCtx; public: using UP = std::unique_ptr; - MatchContext(IAttributeContext::UP attrCtx, ISearchContext::UP searchCtx) - : _attrCtx(std::move(attrCtx)), - _searchCtx(std::move(searchCtx)) - { - assert(_attrCtx); - assert(_searchCtx); - } + MatchContext() noexcept; + MatchContext(IAttributeContext::UP attrCtx, std::unique_ptr searchCtx) noexcept; + MatchContext(MatchContext &&) noexcept = default; + ~MatchContext(); IAttributeContext &getAttributeContext() const { return *_attrCtx; } ISearchContext &getSearchContext() const { return *_searchCtx; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/same_element_builder.h b/searchcore/src/vespa/searchcore/proton/matching/same_element_builder.h index dd89e952a34..9928c9a6ae8 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/same_element_builder.h +++ b/searchcore/src/vespa/searchcore/proton/matching/same_element_builder.h @@ -7,21 +7,24 @@ #include #include -namespace search::queryeval { class FieldSpec; } +namespace search::queryeval { + class IRequestContext; + class FieldSpec; +} namespace proton::matching { class SameElementBuilder { -private: - const search::queryeval::IRequestContext &_requestContext; - ISearchContext &_context; - std::unique_ptr _result; public: SameElementBuilder(const search::queryeval::IRequestContext &requestContext, ISearchContext &context, const search::queryeval::FieldSpec &field, bool expensive); void add_child(search::query::Node &node); search::queryeval::Blueprint::UP build(); +private: + const search::queryeval::IRequestContext &_requestContext; + ISearchContext &_context; + std::unique_ptr _result; }; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/search_session.cpp b/searchcore/src/vespa/searchcore/proton/matching/search_session.cpp index c9b6ddb897f..a2e0cfd5d8e 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/search_session.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/search_session.cpp @@ -18,12 +18,20 @@ SearchSession::SearchSession(const SessionId &id, vespalib::steady_time create_t void SearchSession::releaseEnumGuards() { - _owned_objects.context->releaseEnumGuards(); + _owned_objects.context.releaseEnumGuards(); } SearchSession::~SearchSession() = default; -SearchSession::OwnershipBundle::OwnershipBundle() = default; +SearchSession::OwnershipBundle::OwnershipBundle(MatchContext && match_context, + std::shared_ptr searchHandler) noexcept + : context(std::move(match_context)), + search_handler(std::move(searchHandler)), + feature_overrides(), + readGuard() +{} + +SearchSession::OwnershipBundle::OwnershipBundle() noexcept = default; SearchSession::OwnershipBundle::~OwnershipBundle() = default; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/search_session.h b/searchcore/src/vespa/searchcore/proton/matching/search_session.h index 2cc37a07564..d219682baf2 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/search_session.h +++ b/searchcore/src/vespa/searchcore/proton/matching/search_session.h @@ -2,6 +2,7 @@ #pragma once +#include "match_context.h" #include #include #include @@ -22,13 +23,14 @@ class MatchContext; class SearchSession { public: struct OwnershipBundle { - OwnershipBundle(); - OwnershipBundle(OwnershipBundle &&) = default; - OwnershipBundle & operator = (OwnershipBundle &&) = default; + OwnershipBundle() noexcept; + OwnershipBundle(MatchContext && matchContext, std::shared_ptr searchHandler) noexcept; + OwnershipBundle(OwnershipBundle &&) noexcept = default; + OwnershipBundle & operator = (OwnershipBundle &&) noexcept = delete; ~OwnershipBundle(); + MatchContext context; std::shared_ptr search_handler; std::unique_ptr feature_overrides; - std::unique_ptr context; IDocumentMetaStoreContext::IReadGuard::SP readGuard; }; private: @@ -44,8 +46,7 @@ public: using SP = std::shared_ptr; SearchSession(const SessionId &id, vespalib::steady_time create_time, vespalib::steady_time time_of_doom, - std::unique_ptr match_tools_factory, - OwnershipBundle &&owned_objects); + std::unique_ptr match_tools_factory, OwnershipBundle &&owned_objects); ~SearchSession(); const SessionId &getSessionId() const { return _session_id; } diff --git a/searchcore/src/vespa/searchcore/proton/server/matchview.cpp b/searchcore/src/vespa/searchcore/proton/server/matchview.cpp index f865d533d85..d7a95ae1102 100644 --- a/searchcore/src/vespa/searchcore/proton/server/matchview.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/matchview.cpp @@ -50,17 +50,16 @@ MatchView::MatchView(Matchers::SP matchers, MatchView::~MatchView() = default; -Matcher::SP +std::shared_ptr MatchView::getMatcher(const vespalib::string & rankProfile) const { return _matchers->lookup(rankProfile); } -MatchContext::UP +MatchContext MatchView::createContext() const { - IAttributeContext::UP attrCtx = _attrMgr->createContext(); auto searchCtx = std::make_unique(_indexSearchable, _docIdLimit.get()); - return std::make_unique(std::move(attrCtx), std::move(searchCtx)); + return {_attrMgr->createContext(), std::move(searchCtx)}; } std::unique_ptr @@ -68,14 +67,13 @@ MatchView::match(std::shared_ptr searchHandler, const Sear vespalib::ThreadBundle &threadBundle) const { Matcher::SP matcher = getMatcher(req.ranking); - SearchSession::OwnershipBundle owned_objects; - owned_objects.search_handler = std::move(searchHandler); + SearchSession::OwnershipBundle owned_objects(createContext(), std::move(searchHandler)); owned_objects.readGuard = _metaStore->getReadGuard(); - owned_objects.context = createContext(); - MatchContext *ctx = owned_objects.context.get(); + ISearchContext & search_ctx = owned_objects.context.getSearchContext(); + IAttributeContext & attribute_ctx = owned_objects.context.getAttributeContext(); const search::IDocumentMetaStore & dms = owned_objects.readGuard->get(); const bucketdb::BucketDBOwner & bucketDB = _metaStore->get().getBucketDB(); - return matcher->match(req, threadBundle, ctx->getSearchContext(), ctx->getAttributeContext(), + return matcher->match(req, threadBundle, search_ctx, attribute_ctx, _sessionMgr, dms, bucketDB, std::move(owned_objects)); } diff --git a/searchcore/src/vespa/searchcore/proton/server/matchview.h b/searchcore/src/vespa/searchcore/proton/server/matchview.h index 20db5665832..f01442b9bc3 100644 --- a/searchcore/src/vespa/searchcore/proton/server/matchview.h +++ b/searchcore/src/vespa/searchcore/proton/server/matchview.h @@ -10,26 +10,23 @@ namespace searchcorespi { class IndexSearchable; } -namespace proton { - -namespace matching { - -class MatchContext; -class Matcher; -class SessionManager; - +namespace proton::matching { + class MatchContext; + class Matcher; + class SessionManager; } +namespace proton { struct IAttributeManager; class MatchView { using SessionManager = matching::SessionManager; - std::shared_ptr _matchers; - std::shared_ptr _indexSearchable; - std::shared_ptr _attrMgr; - SessionManager & _sessionMgr; - std::shared_ptr _metaStore; - DocIdLimit &_docIdLimit; + std::shared_ptr _matchers; + std::shared_ptr _indexSearchable; + std::shared_ptr _attrMgr; + SessionManager & _sessionMgr; + std::shared_ptr _metaStore; + DocIdLimit &_docIdLimit; size_t getNumDocs() const { return _metaStore->get().getNumActiveLids(); @@ -63,7 +60,7 @@ public: return _matchers->getStats(rankProfile); } - std::unique_ptr createContext() const; + matching::MatchContext createContext() const; std::unique_ptr match(std::shared_ptr searchHandler, diff --git a/searchcore/src/vespa/searchcore/proton/server/searchview.cpp b/searchcore/src/vespa/searchcore/proton/server/searchview.cpp index 0c96b43a727..cace2478245 100644 --- a/searchcore/src/vespa/searchcore/proton/server/searchview.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/searchview.cpp @@ -127,7 +127,7 @@ SearchView::getDocsumsInternal(const DocsumRequest & req) auto store(_summarySetup->createDocsumStore()); auto mctx = _matchView->createContext(); auto ctx = std::make_unique(req, _summarySetup->getDocsumWriter(), *store, _matchView->getMatcher(req.ranking), - mctx->getSearchContext(), mctx->getAttributeContext(), + mctx.getSearchContext(), mctx.getAttributeContext(), *_summarySetup->getAttributeManager(), getSessionManager()); SearchView::InternalDocsumReply reply(ctx->getDocsums(), true); uint64_t endGeneration = readGuard->get().getCurrentGeneration(); -- cgit v1.2.3