Merge pull request #29372 from vespa-engine/balder/inherit-cost-tier

- Or and SourceBlender inherits max cost tier of children.

4 files changed, 78 insertions, 15 deletions

diff --git a/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp b/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp
index b150f8db7c5..2c3b2f3e8aa 100644
--- a/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp
+++ b/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp
@@ -1415,22 +1415,57 @@ TEST("require that highest cost tier sorts last for AND") {
     EXPECT_EQUAL(expect_up->asString(), top_up->asString());
 }
 
-TEST("require that intermediate cost tier is minimum cost tier of children") {
-    Blueprint::UP bp1(
-            ap((new AndBlueprint())->
-               addChild(ap(MyLeafSpec(10).cost_tier(1).create())).
-               addChild(ap(MyLeafSpec(20).cost_tier(2).create())).
-               addChild(ap(MyLeafSpec(30).cost_tier(3).create()))));
-    Blueprint::UP bp2(
-            ap((new AndBlueprint())->
-               addChild(ap(MyLeafSpec(10).cost_tier(3).create())).
-               addChild(ap(MyLeafSpec(20).cost_tier(2).create())).
-               addChild(ap(MyLeafSpec(30).cost_tier(2).create()))));
-    EXPECT_EQUAL(bp1->getState().cost_tier(), 1u);
-    EXPECT_EQUAL(bp2->getState().cost_tier(), 2u);
+template<typename BP>
+void
+verifyCostTierInheritance(uint8_t expected, uint8_t expected_reverse) {
+    auto bp1 = std::make_unique<BP>();
+    bp1->addChild(ap(MyLeafSpec(10).cost_tier(1).create())).
+         addChild(ap(MyLeafSpec(20).cost_tier(2).create())).
+         addChild(ap(MyLeafSpec(30).cost_tier(3).create()));
+    auto bp2 = std::make_unique<BP>();
+    bp2->addChild(ap(MyLeafSpec(10).cost_tier(3).create())).
+         addChild(ap(MyLeafSpec(20).cost_tier(2).create())).
+         addChild(ap(MyLeafSpec(30).cost_tier(1).create()));
+    EXPECT_EQUAL(bp1->getState().cost_tier(), expected);
+    EXPECT_EQUAL(bp2->getState().cost_tier(), expected_reverse);
+}
+
+TEST("require that AND cost tier is minimum cost tier of children") {
+    verifyCostTierInheritance<AndBlueprint>(1, 1);
+}
+
+TEST("require that OR cost tier is maximum cost tier of children") {
+    verifyCostTierInheritance<OrBlueprint>(3, 3);
+}
+
+TEST("require that Rank cost tier is first childs cost tier") {
+    verifyCostTierInheritance<RankBlueprint>(1, 3);
+}
+
+TEST("require that AndNot cost tier is first childs cost tier") {
+    verifyCostTierInheritance<AndNotBlueprint>(1, 3);
+}
+
+struct MySourceBlender {
+    InvalidSelector selector;
+    SourceBlenderBlueprint sb;
+    MySourceBlender() : selector(), sb(selector) {}
+    IntermediateBlueprint &
+    addChild(Blueprint::UP child) {
+        return sb.addChild(std::move(child));
+    }
+    const Blueprint::State &getState() const {
+        return sb.getState();
+    }
+
+};
+
+TEST("require that SourceBlender cost tier is maximum cost tier of children") {
+    verifyCostTierInheritance<MySourceBlender>(3, 3);
 }
 
-void verify_or_est(const std::vector<Blueprint::HitEstimate> &child_estimates, Blueprint::HitEstimate expect) {
+void
+verify_or_est(const std::vector<Blueprint::HitEstimate> &child_estimates, Blueprint::HitEstimate expect) {
     OrBlueprint my_or;
     my_or.setDocIdLimit(32);
     auto my_est = my_or.combine(child_estimates);
diff --git a/searchlib/src/vespa/searchlib/queryeval/blueprint.h b/searchlib/src/vespa/searchlib/queryeval/blueprint.h
index da6050f075d..993cd124558 100644
--- a/searchlib/src/vespa/searchlib/queryeval/blueprint.h
+++ b/searchlib/src/vespa/searchlib/queryeval/blueprint.h
@@ -298,7 +298,7 @@ class IntermediateBlueprint : public blueprint::StateCache
 private:
     Children _children;
     HitEstimate calculateEstimate() const;
-    uint8_t calculate_cost_tier() const;
+    virtual uint8_t calculate_cost_tier() const;
     uint32_t calculate_tree_size() const;
     bool infer_allow_termwise_eval() const;
     bool infer_want_global_filter() const;
diff --git a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp
index 790d61b3731..f28fe974789 100644
--- a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp
@@ -355,6 +355,16 @@ OrBlueprint::createFilterSearch(bool strict, FilterConstraint constraint) const
     return create_or_filter(get_children(), strict, constraint);
 }
 
+uint8_t
+OrBlueprint::calculate_cost_tier() const
+{
+    uint8_t cost_tier = State::COST_TIER_NORMAL;
+    for (const Blueprint::UP &child : get_children()) {
+        cost_tier = std::max(cost_tier, child->getState().cost_tier());
+    }
+    return cost_tier;
+}
+
 //-----------------------------------------------------------------------------
 WeakAndBlueprint::~WeakAndBlueprint() = default;
 
@@ -663,6 +673,16 @@ SourceBlenderBlueprint::isCompatibleWith(const SourceBlenderBlueprint &other) co
     return (&_selector == &other._selector);
 }
 
+uint8_t
+SourceBlenderBlueprint::calculate_cost_tier() const
+{
+    uint8_t cost_tier = State::COST_TIER_NORMAL;
+    for (const Blueprint::UP &child : get_children()) {
+        cost_tier = std::max(cost_tier, child->getState().cost_tier());
+    }
+    return cost_tier;
+}
+
 //-----------------------------------------------------------------------------
 
 }
diff --git a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h
index 409a9e0fe95..198bd457293 100644
--- a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h
+++ b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h
@@ -28,6 +28,9 @@ public:
     SearchIterator::UP
     createFilterSearch(bool strict, FilterConstraint constraint) const override;
 private:
+    uint8_t calculate_cost_tier() const override {
+        return (childCnt() > 0) ? get_children()[0]->getState().cost_tier() : State::COST_TIER_NORMAL;
+    }
     bool isPositive(size_t index) const override { return index == 0; }
 };
 
@@ -76,6 +79,7 @@ public:
     createFilterSearch(bool strict, FilterConstraint constraint) const override;
 private:
     double computeNextHitRate(const Blueprint & child, double hitRate) const override;
+    uint8_t calculate_cost_tier() const override;
 };
 
 //-----------------------------------------------------------------------------
@@ -168,6 +172,9 @@ public:
                              bool strict, fef::MatchData &md) const override;
     SearchIterator::UP
     createFilterSearch(bool strict, FilterConstraint constraint) const override;
+    uint8_t calculate_cost_tier() const override {
+        return (childCnt() > 0) ? get_children()[0]->getState().cost_tier() : State::COST_TIER_NORMAL;
+    }
 };
 
 //-----------------------------------------------------------------------------
@@ -193,6 +200,7 @@ public:
     /** check if this blueprint has the same source selector as the other */
     bool isCompatibleWith(const SourceBlenderBlueprint &other) const;
     bool isSourceBlender() const override { return true; }
+    uint8_t calculate_cost_tier() const override;
 };
 
 }