Merge pull request #10620 from vespa-engine/havardpe/unpacking-iterators-optimizer

Havardpe/unpacking iterators optimizer 

14 files changed, 92 insertions, 15 deletions

diff --git a/searchlib/src/tests/fef/properties/properties_test.cpp b/searchlib/src/tests/fef/properties/properties_test.cpp
index 380bbcfce5f..df868de3a97 100644
--- a/searchlib/src/tests/fef/properties/properties_test.cpp
+++ b/searchlib/src/tests/fef/properties/properties_test.cpp
@@ -262,6 +262,22 @@ TEST("test stuff") {
             p.add("vespa.dump.ignoredefaultfeatures", "true");
             EXPECT_TRUE(dump::IgnoreDefaultFeatures::check(p));
         }
+        { // vespa.matching.split_unpacking_iterators
+            EXPECT_EQUAL(matching::SplitUnpackingIterators::NAME, vespalib::string("vespa.matching.split_unpacking_iterators"));
+            EXPECT_EQUAL(matching::SplitUnpackingIterators::DEFAULT_VALUE, false);
+            Properties p;
+            EXPECT_EQUAL(matching::SplitUnpackingIterators::check(p), false);
+            p.add("vespa.matching.split_unpacking_iterators", "true");
+            EXPECT_EQUAL(matching::SplitUnpackingIterators::check(p), true);
+        }
+        { // vespa.matching.delay_unpacking_iterators
+            EXPECT_EQUAL(matching::DelayUnpackingIterators::NAME, vespalib::string("vespa.matching.delay_unpacking_iterators"));
+            EXPECT_EQUAL(matching::DelayUnpackingIterators::DEFAULT_VALUE, false);
+            Properties p;
+            EXPECT_EQUAL(matching::DelayUnpackingIterators::check(p), false);
+            p.add("vespa.matching.delay_unpacking_iterators", "true");
+            EXPECT_EQUAL(matching::DelayUnpackingIterators::check(p), true);
+        }
         { // vespa.matching.termwise_limit
             EXPECT_EQUAL(matching::TermwiseLimit::NAME, vespalib::string("vespa.matching.termwise_limit"));
             EXPECT_EQUAL(matching::TermwiseLimit::DEFAULT_VALUE, 1.0);
diff --git a/searchlib/src/tests/queryeval/same_element/same_element_test.cpp b/searchlib/src/tests/queryeval/same_element/same_element_test.cpp
index 45ebdd78fb3..6ec379140c0 100644
--- a/searchlib/src/tests/queryeval/same_element/same_element_test.cpp
+++ b/searchlib/src/tests/queryeval/same_element/same_element_test.cpp
@@ -15,7 +15,7 @@ using namespace search::queryeval;
 using search::attribute::ElementIterator;
 
 std::unique_ptr<SameElementBlueprint> make_blueprint(const std::vector<FakeResult> &children, bool fake_attr = false) {
-    auto result = std::make_unique<SameElementBlueprint>();
+    auto result = std::make_unique<SameElementBlueprint>(false);
     for (size_t i = 0; i < children.size(); ++i) {
         uint32_t field_id = i;
         vespalib::string field_name = vespalib::make_string("f%u", field_id);
diff --git a/searchlib/src/tests/queryeval/simple_phrase/simple_phrase_test.cpp b/searchlib/src/tests/queryeval/simple_phrase/simple_phrase_test.cpp
index aad448f7fe1..f5ce8a9608b 100644
--- a/searchlib/src/tests/queryeval/simple_phrase/simple_phrase_test.cpp
+++ b/searchlib/src/tests/queryeval/simple_phrase/simple_phrase_test.cpp
@@ -173,7 +173,7 @@ PhraseSearchTest::PhraseSearchTest(bool expiredDoom)
     : _requestContext(nullptr, expiredDoom ? 0 : std::numeric_limits<int64_t>::max()),
       _index(),
       _phrase_fs(field, fieldId, phrase_handle),
-      _phrase(_phrase_fs, _requestContext),
+      _phrase(_phrase_fs, _requestContext, false),
       _children(),
       _md(MatchData::makeTestInstance(100, 10)),
       _order(),
@@ -301,7 +301,7 @@ Test::requireThatBlueprintExposesFieldWithEstimate()
 {
     FakeRequestContext requestContext;
     FieldSpec f("foo", 1, 1);
-    SimplePhraseBlueprint phrase(f, requestContext);
+    SimplePhraseBlueprint phrase(f, requestContext, false);
     ASSERT_TRUE(phrase.getState().numFields() == 1);
     EXPECT_EQUAL(f.getFieldId(), phrase.getState().field(0).getFieldId());
     EXPECT_EQUAL(f.getHandle(), phrase.getState().field(0).getHandle());
@@ -327,7 +327,7 @@ Test::requireThatBlueprintForcesPositionDataOnChildren()
 {
     FakeRequestContext requestContext;
     FieldSpec f("foo", 1, 1, true);
-    SimplePhraseBlueprint phrase(f, requestContext);
+    SimplePhraseBlueprint phrase(f, requestContext, false);
     EXPECT_TRUE(f.isFilter());
     EXPECT_TRUE(!phrase.getNextChildField(f).isFilter());
 }
diff --git a/searchlib/src/vespa/searchlib/fef/indexproperties.cpp b/searchlib/src/vespa/searchlib/fef/indexproperties.cpp
index dfe0e17c33d..a7df39faf2f 100644
--- a/searchlib/src/vespa/searchlib/fef/indexproperties.cpp
+++ b/searchlib/src/vespa/searchlib/fef/indexproperties.cpp
@@ -202,6 +202,14 @@ IgnoreDefaultFeatures::check(const Properties &props)
 
 namespace matching {
 
+const vespalib::string SplitUnpackingIterators::NAME("vespa.matching.split_unpacking_iterators");
+const bool SplitUnpackingIterators::DEFAULT_VALUE(false);
+bool SplitUnpackingIterators::check(const Properties &props) { return lookupBool(props, NAME, DEFAULT_VALUE); }
+
+const vespalib::string DelayUnpackingIterators::NAME("vespa.matching.delay_unpacking_iterators");
+const bool DelayUnpackingIterators::DEFAULT_VALUE(false);
+bool DelayUnpackingIterators::check(const Properties &props) { return lookupBool(props, NAME, DEFAULT_VALUE); }
+
 const vespalib::string TermwiseLimit::NAME("vespa.matching.termwise_limit");
 const double TermwiseLimit::DEFAULT_VALUE(1.0);
 
diff --git a/searchlib/src/vespa/searchlib/fef/indexproperties.h b/searchlib/src/vespa/searchlib/fef/indexproperties.h
index 23efe514d68..9adf4487ec5 100644
--- a/searchlib/src/vespa/searchlib/fef/indexproperties.h
+++ b/searchlib/src/vespa/searchlib/fef/indexproperties.h
@@ -135,6 +135,28 @@ namespace execute::onsummary {
 namespace matching {
 
     /**
+     * When enabled, iterators that unpack posting information as part
+     * of matching may be split into multiple parts (some cheap, some
+     * expensive).
+     **/
+    struct SplitUnpackingIterators {
+        static const vespalib::string NAME;
+        static const bool DEFAULT_VALUE;
+        static bool check(const Properties &props);
+    };
+
+    /**
+     * When enabled, iterators that unpack posting information as part
+     * of matching will be tagged as expensive, in order to delay
+     * their execution within the iterator tree.
+     **/
+    struct DelayUnpackingIterators {
+        static const vespalib::string NAME;
+        static const bool DEFAULT_VALUE;
+        static bool check(const Properties &props);
+    };
+
+    /**
      * A number in the range [0,1] indicating how much of the corpus
      * the query must match for termwise evaluation to be enabled. 1
      * means never allowed. 0 means always allowed. The default value
diff --git a/searchlib/src/vespa/searchlib/fef/ranksetup.cpp b/searchlib/src/vespa/searchlib/fef/ranksetup.cpp
index eea9188632d..440ea3e0ea1 100644
--- a/searchlib/src/vespa/searchlib/fef/ranksetup.cpp
+++ b/searchlib/src/vespa/searchlib/fef/ranksetup.cpp
@@ -34,6 +34,9 @@ RankSetup::RankSetup(const BlueprintFactory &factory, const IIndexEnvironment &i
       _firstPhaseRankFeature(),
       _secondPhaseRankFeature(),
       _degradationAttribute(),
+      _split_unpacking_iterators(false),
+      _delay_unpacking_iterators(false),
+      _termwise_limit(1.0),
       _numThreads(0),
       _minHitsPerThread(0),
       _numSearchPartitions(0),
@@ -76,6 +79,8 @@ RankSetup::configure()
     for (uint32_t i = 0; i < dumpFeatures.size(); ++i) {
         addDumpFeature(dumpFeatures[i]);
     }
+    split_unpacking_iterators(matching::SplitUnpackingIterators::check(_indexEnv.getProperties()));
+    delay_unpacking_iterators(matching::DelayUnpackingIterators::check(_indexEnv.getProperties()));
     set_termwise_limit(matching::TermwiseLimit::lookup(_indexEnv.getProperties()));
     setNumThreadsPerSearch(matching::NumThreadsPerSearch::lookup(_indexEnv.getProperties()));
     setMinHitsPerThread(matching::MinHitsPerThread::lookup(_indexEnv.getProperties()));
diff --git a/searchlib/src/vespa/searchlib/fef/ranksetup.h b/searchlib/src/vespa/searchlib/fef/ranksetup.h
index ef869c87740..d543794b347 100644
--- a/searchlib/src/vespa/searchlib/fef/ranksetup.h
+++ b/searchlib/src/vespa/searchlib/fef/ranksetup.h
@@ -31,6 +31,8 @@ private:
     vespalib::string         _firstPhaseRankFeature;
     vespalib::string         _secondPhaseRankFeature;
     vespalib::string         _degradationAttribute;
+    bool                     _split_unpacking_iterators;
+    bool                     _delay_unpacking_iterators;
     double                   _termwise_limit;
     uint32_t                 _numThreads;
     uint32_t                 _minHitsPerThread;
@@ -115,6 +117,12 @@ public:
      **/
     const vespalib::string &getSecondPhaseRank() const { return _secondPhaseRankFeature; }
 
+    bool split_unpacking_iterators() const { return _split_unpacking_iterators; }
+    void split_unpacking_iterators(bool value) { _split_unpacking_iterators = value; }
+
+    bool delay_unpacking_iterators() const { return _delay_unpacking_iterators; }
+    void delay_unpacking_iterators(bool value) { _delay_unpacking_iterators = value; }
+
     /**
      * Set the termwise limit
      *
diff --git a/searchlib/src/vespa/searchlib/query/tree/intermediatenodes.h b/searchlib/src/vespa/searchlib/query/tree/intermediatenodes.h
index 6d643d951f0..3bca4f041b3 100644
--- a/searchlib/src/vespa/searchlib/query/tree/intermediatenodes.h
+++ b/searchlib/src/vespa/searchlib/query/tree/intermediatenodes.h
@@ -99,17 +99,30 @@ class ONear : public QueryNodeMixin<ONear, Intermediate>
 class Phrase : public QueryNodeMixin<Phrase, Intermediate>, public Term {
 public:
     Phrase(const vespalib::string &view, int32_t id, Weight weight)
-        : Term(view, id, weight) {}
+        : Term(view, id, weight), _expensive(false) {}
     virtual ~Phrase() = 0;
+    Phrase &set_expensive(bool value) {
+        _expensive = value;
+        return *this;
+    }
+    bool is_expensive() const { return _expensive; }
+private:
+    bool _expensive;
 };
 
 class SameElement : public QueryNodeMixin<SameElement, Intermediate> {
 public:
-    SameElement(const vespalib::string &view) : _view(view) {}
+    SameElement(const vespalib::string &view) : _view(view), _expensive(false) {}
     virtual ~SameElement() = 0;
     const vespalib::string & getView() const { return _view; }
+    SameElement &set_expensive(bool value) {
+        _expensive = value;
+        return *this;
+    }
+    bool is_expensive() const { return _expensive; }
 private:
     vespalib::string _view;
+    bool _expensive;
 };
 
 class WeightedSetTerm : public QueryNodeMixin<WeightedSetTerm, Intermediate>, public Term {
diff --git a/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h b/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h
index 7bf6c17f136..e7c3fd8c73b 100644
--- a/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h
+++ b/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h
@@ -70,12 +70,12 @@ private:
 
     void visit(Phrase &node) override {
         replicate(node, _builder.addPhrase(node.getChildren().size(), node.getView(),
-                                           node.getId(), node.getWeight()));
+                                           node.getId(), node.getWeight()).set_expensive(node.is_expensive()));
         visitNodes(node.getChildren());
     }
 
     void visit(SameElement &node) override {
-        _builder.addSameElement(node.getChildren().size(), node.getView());
+        _builder.addSameElement(node.getChildren().size(), node.getView()).set_expensive(node.is_expensive());
         visitNodes(node.getChildren());
     }
 
@@ -107,8 +107,7 @@ private:
     }
 
     void replicate(const Term &original, Term &replica) {
-        replica.setTermIndex(original.getTermIndex());
-        replica.setRanked(original.isRanked());
+        replica.setStateFrom(original);
     }
 
     void visit(NumberTerm &node) override {
diff --git a/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp b/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp
index e3588d88ccb..3731b2ff6a8 100644
--- a/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp
@@ -31,7 +31,7 @@ CreateBlueprintVisitorHelper::getResult()
 
 void
 CreateBlueprintVisitorHelper::visitPhrase(query::Phrase &n) {
-    auto phrase = std::make_unique<SimplePhraseBlueprint>(_field, _requestContext);
+    auto phrase = std::make_unique<SimplePhraseBlueprint>(_field, _requestContext, n.is_expensive());
     for (const query::Node * child : n.getChildren()) {
         FieldSpecList fields;
         fields.add(phrase->getNextChildField(_field));
diff --git a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp
index fa9cc82975f..2376b129fc1 100644
--- a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp
@@ -10,12 +10,15 @@
 
 namespace search::queryeval {
 
-SameElementBlueprint::SameElementBlueprint()
+SameElementBlueprint::SameElementBlueprint(bool expensive)
     : ComplexLeafBlueprint(FieldSpecBaseList()),
       _estimate(),
       _layout(),
       _terms()
 {
+    if (expensive) {
+        set_cost_tier(State::COST_TIER_EXPENSIVE);
+    }
 }
 
 SameElementBlueprint::~SameElementBlueprint() = default;
diff --git a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h
index decb518a7c9..3b29e518aa1 100644
--- a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h
+++ b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h
@@ -17,7 +17,7 @@ private:
     std::vector<Blueprint::UP> _terms;
 
 public:
-    SameElementBlueprint();
+    SameElementBlueprint(bool expensive);
     SameElementBlueprint(const SameElementBlueprint &) = delete;
     SameElementBlueprint &operator=(const SameElementBlueprint &) = delete;
     ~SameElementBlueprint();
diff --git a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp
index ed7690f605f..7429553d889 100644
--- a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp
@@ -9,7 +9,7 @@
 
 namespace search::queryeval {
 
-SimplePhraseBlueprint::SimplePhraseBlueprint(const FieldSpec &field, const IRequestContext & requestContext)
+SimplePhraseBlueprint::SimplePhraseBlueprint(const FieldSpec &field, const IRequestContext & requestContext, bool expensive)
     : ComplexLeafBlueprint(field),
       _doom(requestContext.getSoftDoom()),
       _field(field),
@@ -17,6 +17,9 @@ SimplePhraseBlueprint::SimplePhraseBlueprint(const FieldSpec &field, const IRequ
       _layout(),
       _terms()
 {
+    if (expensive) {
+        set_cost_tier(State::COST_TIER_EXPENSIVE);
+    }
 }
 
 SimplePhraseBlueprint::~SimplePhraseBlueprint()
diff --git a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.h b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.h
index 26668f85cb7..c313c0b38ad 100644
--- a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.h
+++ b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.h
@@ -23,7 +23,7 @@ private:
     SimplePhraseBlueprint &operator=(const SimplePhraseBlueprint &); // disabled
 
 public:
-    SimplePhraseBlueprint(const FieldSpec &field, const IRequestContext & requestContext);
+    SimplePhraseBlueprint(const FieldSpec &field, const IRequestContext & requestContext, bool expensive);
     ~SimplePhraseBlueprint();
 
     // used by create visitor