Remove option to delay phrases and same-element. The gain is smaller than the risk and too hard to use.

6 files changed, 24 insertions, 83 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/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<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);
 };
 
 }