diff options
Diffstat (limited to 'searchcore/src')
13 files changed, 61 insertions, 118 deletions
diff --git a/searchcore/src/tests/proton/matching/unpacking_iterators_optimizer/unpacking_iterators_optimizer_test.cpp b/searchcore/src/tests/proton/matching/unpacking_iterators_optimizer/unpacking_iterators_optimizer_test.cpp index 78486f54704..bd26323deb4 100644 --- a/searchcore/src/tests/proton/matching/unpacking_iterators_optimizer/unpacking_iterators_optimizer_test.cpp +++ b/searchcore/src/tests/proton/matching/unpacking_iterators_optimizer/unpacking_iterators_optimizer_test.cpp @@ -269,100 +269,60 @@ std::string delayed_split_query_tree_dump = //----------------------------------------------------------------------------- -Node::UP optimize(Node::UP root, bool white_list, bool split, bool delay) { - return UnpackingIteratorsOptimizer::optimize(std::move(root), white_list, split, delay); +Node::UP optimize(Node::UP root, bool white_list, bool split) { + return UnpackingIteratorsOptimizer::optimize(std::move(root), white_list, split); } TEST(UnpackingIteratorsOptimizerTest, require_that_root_phrase_node_can_be_left_alone) { - std::string actual1 = dump_query(*optimize(make_phrase(), false, false, false)); - std::string actual2 = dump_query(*optimize(make_phrase(), false, true, false)); - std::string actual3 = dump_query(*optimize(make_phrase(), true, false, false)); + std::string actual1 = dump_query(*optimize(make_phrase(), false, false)); + std::string actual2 = dump_query(*optimize(make_phrase(), false, true)); + std::string actual3 = dump_query(*optimize(make_phrase(), true, false)); std::string expect = plain_phrase_dump; EXPECT_EQ(actual1, expect); EXPECT_EQ(actual2, expect); EXPECT_EQ(actual3, expect); } -TEST(UnpackingIteratorsOptimizerTest, require_that_root_phrase_node_can_be_delayed) { - std::string actual1 = dump_query(*optimize(make_phrase(), false, false, true)); - std::string actual2 = dump_query(*optimize(make_phrase(), false, true, true)); - std::string actual3 = dump_query(*optimize(make_phrase(), true, false, true)); - std::string expect = delayed_phrase_dump; - EXPECT_EQ(actual1, expect); - EXPECT_EQ(actual2, expect); - EXPECT_EQ(actual3, expect); -} - TEST(UnpackingIteratorsOptimizerTest, require_that_root_phrase_node_can_be_split) { - std::string actual1 = dump_query(*optimize(make_phrase(), true, true, true)); - std::string actual2 = dump_query(*optimize(make_phrase(), true, true, false)); + std::string actual1 = dump_query(*optimize(make_phrase(), true, true)); std::string expect = split_phrase_dump; EXPECT_EQ(actual1, expect); - EXPECT_EQ(actual2, expect); } //----------------------------------------------------------------------------- TEST(UnpackingIteratorsOptimizerTest, require_that_root_same_element_node_can_be_left_alone) { - std::string actual1 = dump_query(*optimize(make_same_element(), false, false, false)); - std::string actual2 = dump_query(*optimize(make_same_element(), false, true, false)); - std::string actual3 = dump_query(*optimize(make_same_element(), true, false, false)); + std::string actual1 = dump_query(*optimize(make_same_element(), false, false)); + std::string actual2 = dump_query(*optimize(make_same_element(), false, true)); + std::string actual3 = dump_query(*optimize(make_same_element(), true, false)); std::string expect = plain_same_element_dump; EXPECT_EQ(actual1, expect); EXPECT_EQ(actual2, expect); EXPECT_EQ(actual3, expect); } -TEST(UnpackingIteratorsOptimizerTest, require_that_root_same_element_node_can_be_delayed) { - std::string actual1 = dump_query(*optimize(make_same_element(), false, false, true)); - std::string actual2 = dump_query(*optimize(make_same_element(), false, true, true)); - std::string actual3 = dump_query(*optimize(make_same_element(), true, false, true)); - std::string expect = delayed_same_element_dump; - EXPECT_EQ(actual1, expect); - EXPECT_EQ(actual2, expect); - EXPECT_EQ(actual3, expect); -} - TEST(UnpackingIteratorsOptimizerTest, require_that_root_same_element_node_can_be_split) { - std::string actual1 = dump_query(*optimize(make_same_element(), true, true, true)); - std::string actual2 = dump_query(*optimize(make_same_element(), true, true, false)); + std::string actual1 = dump_query(*optimize(make_same_element(), true, true)); std::string expect = split_same_element_dump; EXPECT_EQ(actual1, expect); - EXPECT_EQ(actual2, expect); } //----------------------------------------------------------------------------- TEST(UnpackingIteratorsOptimizerTest, require_that_query_tree_can_be_left_alone) { - std::string actual1 = dump_query(*optimize(make_query_tree(), false, false, false)); - std::string actual2 = dump_query(*optimize(make_query_tree(), true, false, false)); + std::string actual1 = dump_query(*optimize(make_query_tree(), false, false)); + std::string actual2 = dump_query(*optimize(make_query_tree(), true, false)); std::string expect = plain_query_tree_dump; EXPECT_EQ(actual1, expect); EXPECT_EQ(actual2, expect); } -TEST(UnpackingIteratorsOptimizerTest, require_that_query_tree_can_be_delayed) { - std::string actual1 = dump_query(*optimize(make_query_tree(), false, false, true)); - std::string actual2 = dump_query(*optimize(make_query_tree(), true, false, true)); - std::string expect = delayed_query_tree_dump; - EXPECT_EQ(actual1, expect); - EXPECT_EQ(actual2, expect); -} - TEST(UnpackingIteratorsOptimizerTest, require_that_query_tree_can_be_split) { - std::string actual1 = dump_query(*optimize(make_query_tree(), false, true, false)); - std::string actual2 = dump_query(*optimize(make_query_tree(), true, true, false)); + std::string actual1 = dump_query(*optimize(make_query_tree(), false, true)); + std::string actual2 = dump_query(*optimize(make_query_tree(), true, true)); std::string expect = split_query_tree_dump; EXPECT_EQ(actual1, expect); EXPECT_EQ(actual2, expect); } -TEST(UnpackingIteratorsOptimizerTest, require_that_query_tree_can_be_delayed_and_split) { - std::string actual1 = dump_query(*optimize(make_query_tree(), false, true, true)); - std::string actual2 = dump_query(*optimize(make_query_tree(), true, true, true)); - std::string expect = delayed_split_query_tree_dump; - EXPECT_EQ(actual1, expect); - EXPECT_EQ(actual2, expect); -} - GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchcore/src/tests/proton/summaryengine/summaryengine.cpp b/searchcore/src/tests/proton/summaryengine/summaryengine.cpp index f82b4c9243f..f1183c2556a 100644 --- a/searchcore/src/tests/proton/summaryengine/summaryengine.cpp +++ b/searchcore/src/tests/proton/summaryengine/summaryengine.cpp @@ -3,13 +3,9 @@ #include <vespa/vespalib/data/slime/slime.h> #include <vespa/searchcore/proton/summaryengine/summaryengine.h> #include <vespa/searchlib/engine/searchreply.h> -#include <vespa/searchlib/util/rawbuf.h> -#include <vespa/searchlib/util/slime_output_raw_buf_adapter.h> #include <vespa/vespalib/data/databuffer.h> #include <vespa/vespalib/data/simple_buffer.h> #include <vespa/vespalib/util/compressor.h> -#include <vespa/searchsummary/docsummary/docsumwriter.h> -#include <vespa/metrics/metricset.h> #include <vespa/fnet/frt/rpcrequest.h> #include <vespa/log/log.h> diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp index 7b4db47f585..942dac63955 100644 --- a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp +++ b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp @@ -84,7 +84,7 @@ DocsumContext::createSlimeReply() if (_request.expired() ) { break; } Cursor &docSumC = array.addObject(); ObjectSymbolInserter inserter(docSumC, docsumSym); - if ((docId != search::endDocId) && !rci.mustSkip) { + if ((docId != search::endDocId) && rci.outputClass != nullptr) { _docsumWriter.insertDocsum(rci, docId, &_docsumState, &_docsumStore, inserter); } num_ok++; @@ -103,7 +103,7 @@ DocsumContext::createSlimeReply() DocsumContext::DocsumContext(const DocsumRequest & request, IDocsumWriter & docsumWriter, IDocsumStore & docsumStore, std::shared_ptr<Matcher> matcher, ISearchContext & searchCtx, IAttributeContext & attrCtx, - IAttributeManager & attrMgr, SessionManager & sessionMgr) : + const IAttributeManager & attrMgr, SessionManager & sessionMgr) : _request(request), _docsumWriter(docsumWriter), _docsumStore(docsumStore), @@ -124,24 +124,24 @@ DocsumContext::getDocsums() } void -DocsumContext::FillSummaryFeatures(search::docsummary::GetDocsumsState * state, search::docsummary::IDocsumEnvironment *) +DocsumContext::FillSummaryFeatures(search::docsummary::GetDocsumsState& state) { - assert(&_docsumState == state); + assert(&_docsumState == &state); if (_matcher->canProduceSummaryFeatures()) { - state->_summaryFeatures = _matcher->getSummaryFeatures(_request, _searchCtx, _attrCtx, _sessionMgr); + state._summaryFeatures = _matcher->getSummaryFeatures(_request, _searchCtx, _attrCtx, _sessionMgr); } - state->_summaryFeaturesCached = false; + state._summaryFeaturesCached = false; } void -DocsumContext::FillRankFeatures(search::docsummary::GetDocsumsState * state, search::docsummary::IDocsumEnvironment *) +DocsumContext::FillRankFeatures(search::docsummary::GetDocsumsState& state) { - assert(&_docsumState == state); + assert(&_docsumState == &state); // check if we are allowed to run - if ( ! state->_args.dumpFeatures()) { + if ( ! state._args.dumpFeatures()) { return; } - state->_rankFeatures = _matcher->getRankFeatures(_request, _searchCtx, _attrCtx, _sessionMgr); + state._rankFeatures = _matcher->getRankFeatures(_request, _searchCtx, _attrCtx, _sessionMgr); } std::unique_ptr<MatchingElements> diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.h b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.h index 958e19f9bed..5c1db91f05d 100644 --- a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.h +++ b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.h @@ -27,7 +27,7 @@ private: std::shared_ptr<matching::Matcher> _matcher; matching::ISearchContext & _searchCtx; search::attribute::IAttributeContext & _attrCtx; - search::IAttributeManager & _attrMgr; + const search::IAttributeManager & _attrMgr; search::docsummary::GetDocsumsState _docsumState; matching::SessionManager & _sessionMgr; @@ -43,14 +43,14 @@ public: std::shared_ptr<matching::Matcher> matcher, matching::ISearchContext & searchCtx, search::attribute::IAttributeContext & attrCtx, - search::IAttributeManager & attrMgr, + const search::IAttributeManager & attrMgr, matching::SessionManager & sessionMgr); search::engine::DocsumReply::UP getDocsums(); // Implements GetDocsumsStateCallback - void FillSummaryFeatures(search::docsummary::GetDocsumsState * state, search::docsummary::IDocsumEnvironment * env) override; - void FillRankFeatures(search::docsummary::GetDocsumsState * state, search::docsummary::IDocsumEnvironment * env) override; + void FillSummaryFeatures(search::docsummary::GetDocsumsState& state) override; + void FillRankFeatures(search::docsummary::GetDocsumsState& state) override; std::unique_ptr<search::MatchingElements> fill_matching_elements(const search::MatchingElementsFields &fields) override; }; diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/isummarymanager.h b/searchcore/src/vespa/searchcore/proton/docsummary/isummarymanager.h index e3c4705104f..946c45feb4b 100644 --- a/searchcore/src/vespa/searchcore/proton/docsummary/isummarymanager.h +++ b/searchcore/src/vespa/searchcore/proton/docsummary/isummarymanager.h @@ -31,16 +31,11 @@ public: typedef std::unique_ptr<ISummarySetup> UP; typedef std::shared_ptr<ISummarySetup> SP; - virtual ~ISummarySetup() {} + ~ISummarySetup() override = default; virtual search::docsummary::IDocsumWriter &getDocsumWriter() const = 0; virtual const search::docsummary::ResultConfig &getResultConfig() = 0; virtual search::docsummary::IDocsumStore::UP createDocsumStore() = 0; - - // Inherit doc from IDocsumEnvironment - virtual search::IAttributeManager *getAttributeManager() override = 0; - virtual vespalib::string lookupIndex(const vespalib::string & s) const override = 0; - virtual juniper::Juniper *getJuniper() override = 0; }; typedef std::unique_ptr<ISummaryManager> UP; diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/summarymanager.cpp b/searchcore/src/vespa/searchcore/proton/docsummary/summarymanager.cpp index 18c84038f9b..3e3a3529e46 100644 --- a/searchcore/src/vespa/searchcore/proton/docsummary/summarymanager.cpp +++ b/searchcore/src/vespa/searchcore/proton/docsummary/summarymanager.cpp @@ -10,6 +10,7 @@ #include <vespa/searchcore/proton/flushengine/shrink_lid_space_flush_target.h> #include <vespa/vespalib/util/lambdatask.h> #include <vespa/searchsummary/docsummary/docsumconfig.h> +#include <vespa/searchsummary/docsummary/keywordextractor.h> #include <vespa/vespalib/util/exceptions.h> #include <vespa/fastlib/text/normwordfolder.h> @@ -89,7 +90,7 @@ SummarySetup(const vespalib::string & baseDir, const SummaryConfig & summaryCfg, _juniperConfig(), _attributeMgr(std::move(attributeMgr)), _docStore(std::move(docStore)), - _repo(repo) + _repo(std::move(repo)) { auto resultConfig = std::make_unique<ResultConfig>(); if (!resultConfig->ReadConfig(summaryCfg, make_string("SummaryManager(%s)", baseDir.c_str()).c_str())) { @@ -103,7 +104,7 @@ SummarySetup(const vespalib::string & baseDir, const SummaryConfig & summaryCfg, _juniperConfig = std::make_unique<juniper::Juniper>(&_juniperProps, _wordFolder.get()); _docsumWriter = std::make_unique<DynamicDocsumWriter>(std::move(resultConfig), std::unique_ptr<KeywordExtractor>()); - DynamicDocsumConfig dynCfg(this, _docsumWriter.get()); + DynamicDocsumConfig dynCfg(*this, _docsumWriter.get()); dynCfg.configure(summarymapCfg); } diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/summarymanager.h b/searchcore/src/vespa/searchcore/proton/docsummary/summarymanager.h index 0fd45bb28fb..8be949a7351 100644 --- a/searchcore/src/vespa/searchcore/proton/docsummary/summarymanager.h +++ b/searchcore/src/vespa/searchcore/proton/docsummary/summarymanager.h @@ -43,9 +43,9 @@ public: search::docsummary::IDocsumStore::UP createDocsumStore() override; - search::IAttributeManager * getAttributeManager() override { return _attributeMgr.get(); } + const search::IAttributeManager * getAttributeManager() const override { return _attributeMgr.get(); } vespalib::string lookupIndex(const vespalib::string & s) const override { (void) s; return ""; } - juniper::Juniper * getJuniper() override { return _juniperConfig.get(); } + const juniper::Juniper * getJuniper() const override { return _juniperConfig.get(); } }; private: diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp index 427d4507c43..eefb8411df2 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp @@ -24,6 +24,21 @@ using vespalib::Issue; namespace { +struct LazyThreadTraceInserter { + search::engine::Trace &root_trace; + std::unique_ptr<vespalib::slime::Inserter> inserter; + LazyThreadTraceInserter(search::engine::Trace &root_trace_in) + : root_trace(root_trace_in), inserter() {} + void handle(const search::engine::Trace &thread_trace) { + if (thread_trace.hasTrace()) { + if (!inserter) { + inserter = std::make_unique<vespalib::slime::ArrayInserter>(root_trace.createCursor("query_execution").setArray("threads")); + } + vespalib::slime::inject(thread_trace.getRoot(), *inserter); + } + } +}; + struct TimedMatchLoopCommunicator final : IMatchLoopCommunicator { IMatchLoopCommunicator &communicator; vespalib::Timer timer; @@ -104,17 +119,12 @@ MatchMaster::match(search::engine::Trace & trace, double query_time_s = vespalib::to_s(query_latency_time.elapsed()); double rerank_time_s = vespalib::to_s(timedCommunicator.elapsed); double match_time_s = 0.0; - std::unique_ptr<vespalib::slime::Inserter> inserter; - if (trace.shouldTrace(4)) { - inserter = std::make_unique<vespalib::slime::ArrayInserter>(trace.createCursor("query_execution").setArray("threads")); - } + LazyThreadTraceInserter inserter(trace); for (size_t i = 0; i < threadState.size(); ++i) { const MatchThread & matchThread = *threadState[i]; match_time_s = std::max(match_time_s, matchThread.get_match_time()); _stats.merge_partition(matchThread.get_thread_stats(), i); - if (inserter && matchThread.getTrace().hasTrace()) { - vespalib::slime::inject(matchThread.getTrace().getRoot(), *inserter); - } + inserter.handle(matchThread.getTrace()); matchThread.get_issues().for_each_message([](const auto &msg){ Issue::report(Issue(msg)); }); } _stats.queryLatency(query_time_s); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp index a2af40aae96..6fecdbf611c 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp @@ -187,8 +187,7 @@ MatchToolsFactory(QueryLimiter & queryLimiter, _query.setWhiteListBlueprint(metaStore.createWhiteListBlueprint()); trace.addEvent(5, "Deserialize and build query tree"); _valid = _query.buildTree(queryStack, location, viewResolver, indexEnv, - rankSetup.split_unpacking_iterators(), - rankSetup.delay_unpacking_iterators()); + SplitUnpackingIterators::check(_queryEnv.getProperties(), rankSetup.split_unpacking_iterators())); if (_valid) { _query.extractTerms(_queryEnv.terms()); _query.extractLocations(_queryEnv.locations()); diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp index 5802433ae02..aaacb971ee0 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp @@ -164,7 +164,7 @@ Query::~Query() = default; bool Query::buildTree(vespalib::stringref stack, const string &location, const ViewResolver &resolver, const IIndexEnvironment &indexEnv, - bool split_unpacking_iterators, bool delay_unpacking_iterators) + bool split_unpacking_iterators) { SimpleQueryStackDumpIterator stack_dump_iterator(stack); _query_tree = QueryTreeCreator<ProtonNodeTypes>::create(stack_dump_iterator); @@ -173,7 +173,7 @@ Query::buildTree(vespalib::stringref stack, const string &location, _query_tree->accept(prefixSameElementSubIndexes); exchange_location_nodes(location, _query_tree, _locations); _query_tree = UnpackingIteratorsOptimizer::optimize(std::move(_query_tree), - bool(_whiteListBlueprint), split_unpacking_iterators, delay_unpacking_iterators); + bool(_whiteListBlueprint), split_unpacking_iterators); ResolveViewVisitor resolve_visitor(resolver, indexEnv); _query_tree->accept(resolve_visitor); return true; diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.h b/searchcore/src/vespa/searchcore/proton/matching/query.h index 09eaed5c4a9..a666e382485 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.h +++ b/searchcore/src/vespa/searchcore/proton/matching/query.h @@ -53,8 +53,7 @@ public: const vespalib::string &location, const ViewResolver &resolver, const search::fef::IIndexEnvironment &idxEnv, - bool split_unpacking_iterators = false, - bool delay_unpacking_iterators = false); + bool split_unpacking_iterators = false); /** * Extract query terms from the query tree; to be used to build diff --git a/searchcore/src/vespa/searchcore/proton/matching/unpacking_iterators_optimizer.cpp b/searchcore/src/vespa/searchcore/proton/matching/unpacking_iterators_optimizer.cpp index 72e0153b5c2..e3ab2c9faa8 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/unpacking_iterators_optimizer.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/unpacking_iterators_optimizer.cpp @@ -72,13 +72,9 @@ struct TermExpander : QueryVisitor { struct NodeTraverser : TemplateTermVisitor<NodeTraverser, ProtonNodeTypes> { bool split_unpacking_iterators; - bool delay_unpacking_iterators; - - NodeTraverser(bool split_unpacking_iterators_in, - bool delay_unpacking_iterators_in) - : split_unpacking_iterators(split_unpacking_iterators_in), - delay_unpacking_iterators(delay_unpacking_iterators_in) {} + NodeTraverser(bool split_unpacking_iterators_in) + : split_unpacking_iterators(split_unpacking_iterators_in) {} template <class TermNode> void visitTerm(TermNode &) {} void visit(ProtonNodeTypes::And &n) override { for (Node *child: n.getChildren()) { @@ -92,16 +88,6 @@ struct NodeTraverser : TemplateTermVisitor<NodeTraverser, ProtonNodeTypes> expander.flush(n); } } - void visit(ProtonNodeTypes::Phrase &n) override { - if (delay_unpacking_iterators) { - n.set_expensive(true); - } - } - void visit(ProtonNodeTypes::SameElement &n) override { - if (delay_unpacking_iterators) { - n.set_expensive(true); - } - } }; } // namespace proton::matching::<unnamed> @@ -109,12 +95,10 @@ struct NodeTraverser : TemplateTermVisitor<NodeTraverser, ProtonNodeTypes> search::query::Node::UP UnpackingIteratorsOptimizer::optimize(search::query::Node::UP root, bool has_white_list, - bool split_unpacking_iterators, - bool delay_unpacking_iterators) + bool split_unpacking_iterators) { - if (split_unpacking_iterators || delay_unpacking_iterators) { - NodeTraverser traverser(split_unpacking_iterators, - delay_unpacking_iterators); + if (split_unpacking_iterators) { + NodeTraverser traverser(split_unpacking_iterators); root->accept(traverser); } if (has_white_list && split_unpacking_iterators) { diff --git a/searchcore/src/vespa/searchcore/proton/matching/unpacking_iterators_optimizer.h b/searchcore/src/vespa/searchcore/proton/matching/unpacking_iterators_optimizer.h index 3955c3cbc64..d4bfadf6c31 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/unpacking_iterators_optimizer.h +++ b/searchcore/src/vespa/searchcore/proton/matching/unpacking_iterators_optimizer.h @@ -14,8 +14,7 @@ namespace proton::matching { struct UnpackingIteratorsOptimizer { static search::query::Node::UP optimize(search::query::Node::UP root, bool has_white_list, - bool split_unpacking_iterators, - bool delay_unpacking_iterators); + bool split_unpacking_iterators); }; } |