Merge pull request #36593 from vespa-engine/johsol/wire-first-phase-max

Wire firstPhaseMax into mingle

Author: arnej27959

searchcore/src/tests/proton/matching/match_loop_communicator/match_loop_communicator_test.cpp

@@ -1,5 +1,6 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #include <vespa/searchcore/proton/matching/match_loop_communicator.h>
+#include <vespa/searchlib/features/first_phase_max_feature.h>
 #include <vespa/searchlib/features/first_phase_rank_lookup.h>
 #include <vespa/searchlib/fef/objectstore.h>
 #include <vespa/vespalib/gtest/gtest.h>
@@ -17,6 +18,7 @@ using Hits = MatchLoopCommunicator::Hits;
 using TaggedHit = MatchLoopCommunicator::TaggedHit;
 using TaggedHits = MatchLoopCommunicator::TaggedHits;
 using ObjectStore = search::fef::ObjectStore;
+using search::features::FirstPhaseMaxBlueprint;
 using search::features::FirstPhaseRankLookup;
 using search::queryeval::SortedHitSequence;
 using vespalib::test::Nexus;
@@ -321,6 +323,19 @@ TEST(MatchLoopCommunicatorTest, require_that_first_phase_rank_lookup_is_populate
     EXPECT_EQ(FeatureVec({1, unranked, 3, unranked, 5}), extract_ranks(l2));
 }
 
+TEST(MatchLoopCommunicatorTest, require_that_first_phase_max_is_populated)
+{
+    constexpr size_t num_threads = 1;
+    constexpr size_t thread_id = 0;
+    ObjectStore store;
+    FirstPhaseMaxBlueprint::make_shared_state(store);
+    MatchLoopCommunicator f1(num_threads, 3, {}, &store, do_nothing);
+    const auto& first_phase_max = *FirstPhaseMaxBlueprint::get_shared_state(store);
+    auto hits_in = hit_vec({{21, 5}, {22, 4}, {23, 3}, {24, 2}, {25, 1}});
+    auto result = second_phase(f1, hits_in, thread_id, 10);
+    EXPECT_EQ(5, first_phase_max);
+}
+
 TEST(MatchLoopCommunicatorTest, require_that_before_second_phase_is_called_once)
 {
     constexpr size_t num_threads = 5;

searchcore/src/vespa/searchcore/proton/matching/match_loop_communicator.cpp

@@ -1,10 +1,12 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "match_loop_communicator.h"
+#include <vespa/searchlib/features/first_phase_max_feature.h>
 #include <vespa/searchlib/features/first_phase_rank_lookup.h>
 #include <vespa/vespalib/util/priority_queue.h>
 #include <vespa/vespalib/util/rendezvous.hpp>
 
+using search::features::FirstPhaseMaxBlueprint;
 using search::features::FirstPhaseRankLookup;
 
 namespace proton:: matching {
@@ -89,9 +91,11 @@ MatchLoopCommunicator::GetSecondPhaseWork::GetSecondPhaseWork(size_t n, size_t t
       _diversifier(std::move(diversifier)),
       _object_store(object_store),
       _first_phase_rank_lookup(nullptr),
+      _first_phase_max(nullptr),
       _before_second_phase(std::move(before_second_phase)) {
     if (_object_store) {
         _first_phase_rank_lookup = FirstPhaseRankLookup::get_mutable_shared_state(*_object_store);
+        _first_phase_max = FirstPhaseMaxBlueprint::get_mutable_shared_state(*_object_store);
     }
 }
 
@@ -163,6 +167,10 @@ MatchLoopCommunicator::GetSecondPhaseWork::mingle()
     } else {
         mingle(queue, NoRegisterFirstPhaseRank());
     }
+
+    if (_first_phase_max != nullptr) {
+        *_first_phase_max = best_scores.high;
+    }
 }
 
 void

searchcore/src/vespa/searchcore/proton/matching/match_loop_communicator.h

@@ -33,6 +33,7 @@ class MatchLoopCommunicator final : public IMatchLoopCommunicator
         std::unique_ptr<IDiversifier> _diversifier;
         IObjectStore* _object_store;
         FirstPhaseRankLookup* _first_phase_rank_lookup;
+        double* _first_phase_max;
         std::function<void()> _before_second_phase;
         GetSecondPhaseWork(size_t n, size_t topN_in, Range &best_scores_in, BestDropped &best_dropped_in, std::unique_ptr<IDiversifier> diversifier, IObjectStore* object_store, std::function<void()> before_second_phase);
         ~GetSecondPhaseWork() override;

searchlib/src/vespa/searchlib/features/first_phase_max_feature.cpp

@@ -42,12 +42,6 @@ namespace {
 
 const std::string key = "firstPhaseMax";
 
-static void make_shared_state(IObjectStore& store) {
-    if (store.get(key) == nullptr) {
-        store.add(key, std::make_unique<AnyWrapper<feature_t>>(-std::numeric_limits<feature_t>::infinity()));
-    }
-}
-
 }
 
 FirstPhaseMaxBlueprint::FirstPhaseMaxBlueprint()
@@ -61,6 +55,12 @@ bool FirstPhaseMaxBlueprint::setup(const IIndexEnvironment&, const ParameterList
     return true;
 }
 
+void FirstPhaseMaxBlueprint::make_shared_state(IObjectStore& store) {
+    if (store.get(key) == nullptr) {
+        store.add(key, std::make_unique<AnyWrapper<feature_t>>(-std::numeric_limits<feature_t>::infinity()));
+    }
+}
+
 const feature_t* FirstPhaseMaxBlueprint::get_shared_state(const IObjectStore& store) {
     auto* wrapper = dynamic_cast<const AnyWrapper<feature_t>*>(store.get(key));
     if (wrapper != nullptr) {

searchlib/src/vespa/searchlib/features/first_phase_max_feature.h

@@ -30,6 +30,7 @@ class FirstPhaseMaxBlueprint : public fef::Blueprint {
     ~FirstPhaseMaxBlueprint() override;
 
     // for tests
+    static void make_shared_state(fef::IObjectStore& store);
     static const feature_t* get_shared_state(const fef::IObjectStore& store);
     static feature_t* get_mutable_shared_state(fef::IObjectStore& store);