From 7451a7b7e16fd8af460238dab7bb5d1ea2cffde8 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 31 Aug 2022 12:27:31 +0000 Subject: Remove option to delay phrases and same-element. The gain is smaller than the risk and too hard to use. --- .../unpacking_iterators_optimizer_test.cpp | 68 +++++----------------- .../searchcore/proton/matching/match_tools.cpp | 3 +- .../src/vespa/searchcore/proton/matching/query.cpp | 4 +- .../src/vespa/searchcore/proton/matching/query.h | 3 +- .../matching/unpacking_iterators_optimizer.cpp | 26 ++------- .../matching/unpacking_iterators_optimizer.h | 3 +- 6 files changed, 24 insertions(+), 83 deletions(-) (limited to 'searchcore') 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/vespa/searchcore/proton/matching/match_tools.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp index a086a719d95..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, - SplitUnpackingIterators::check(_queryEnv.getProperties(), rankSetup.split_unpacking_iterators()), - DelayUnpackingIterators::check(_queryEnv.getProperties(), 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::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 { 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 void visitTerm(TermNode &) {} void visit(ProtonNodeTypes::And &n) override { for (Node *child: n.getChildren()) { @@ -92,16 +88,6 @@ struct NodeTraverser : TemplateTermVisitor 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:: @@ -109,12 +95,10 @@ struct NodeTraverser : TemplateTermVisitor 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); }; } -- cgit v1.2.3