auto-enumerate blueprints using guard

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

@@ -49,18 +49,18 @@ class AttributeLimiter {
     using BlueprintAndMatchData = std::pair<search::queryeval::Blueprint&, search::fef::MatchData&>;
     BlueprintAndMatchData create_match_data(size_t want_hits, size_t max_group_size, double hit_rate,
                                             bool strictSearch);
-    search::queryeval::Searchable&                       _searchable_attributes;
-    const search::queryeval::IRequestContext&            _requestContext;
-    const RangeQueryLocator&                             _rangeQueryLocator;
-    std::string                                          _attribute_name;
-    bool                                                 _descending;
-    std::string                                          _diversity_attribute;
-    std::mutex                                           _lock;
-    std::vector<search::fef::MatchData::UP>              _match_datas;
-    std::unique_ptr<search::queryeval::Blueprint>        _blueprint;
-    std::atomic<ssize_t>                                 _estimatedHits;
-    double                                               _diversityCutoffFactor;
-    DiversityCutoffStrategy                              _diversityCutoffStrategy;
+    search::queryeval::Searchable&                _searchable_attributes;
+    const search::queryeval::IRequestContext&     _requestContext;
+    const RangeQueryLocator&                      _rangeQueryLocator;
+    std::string                                   _attribute_name;
+    bool                                          _descending;
+    std::string                                   _diversity_attribute;
+    std::mutex                                    _lock;
+    std::vector<search::fef::MatchData::UP>       _match_datas;
+    std::unique_ptr<search::queryeval::Blueprint> _blueprint;
+    std::atomic<ssize_t>                          _estimatedHits;
+    double                                        _diversityCutoffFactor;
+    DiversityCutoffStrategy                       _diversityCutoffStrategy;
 };
 
 } // namespace proton::matching

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

@@ -32,9 +32,9 @@ using search::queryeval::AndNotBlueprint;
 using search::queryeval::Blueprint;
 using search::queryeval::EquivBlueprint;
 using search::queryeval::IntermediateBlueprint;
-using search::queryeval::MatchSpan;
 using search::queryeval::MatchingElementsSearch;
 using search::queryeval::MatchingPhase;
+using search::queryeval::MatchSpan;
 using search::queryeval::NearBlueprint;
 using search::queryeval::NearSearchBase;
 using search::queryeval::ONearBlueprint;
@@ -122,7 +122,8 @@ void find_matching_elements(const std::vector<uint32_t>& docs, const std::string
     }
 }
 
-bool has_matching_elements_field(const IntermediateBlueprint& bp, const MatchingElementsFields& fields, FieldIdToNameMapper id_to_name) {
+bool has_matching_elements_field(const IntermediateBlueprint& bp, const MatchingElementsFields& fields,
+                                 FieldIdToNameMapper id_to_name) {
     for (size_t i = 0; i < bp.childCnt(); ++i) {
         const auto& cs = bp.getChild(i).getState();
         for (size_t j = 0; j < cs.numFields(); ++j) {
@@ -163,11 +164,9 @@ void extract_matching_elements(uint32_t doc, std::span<const MatchSpan> match_sp
     }
 }
 
-void find_matching_elements(const std::vector<uint32_t> &docs, const IntermediateBlueprint& bp,
-                            const MatchingElementsFields& fields,
-                            FieldIdToNameMapper id_to_name,
-                            MatchData& match_data,
-                            MatchingElements &result) {
+void find_matching_elements(const std::vector<uint32_t>& docs, const IntermediateBlueprint& bp,
+                            const MatchingElementsFields& fields, FieldIdToNameMapper id_to_name,
+                            MatchData& match_data, MatchingElements& result) {
     if (!has_matching_elements_field(bp, fields, id_to_name)) {
         return;
     }
@@ -181,7 +180,7 @@ void find_matching_elements(const std::vector<uint32_t> &docs, const Intermediat
     }
     search->initRange(docs.front(), docs.back() + 1);
     std::vector<MatchSpan> match_spans;
-    std::vector<uint32_t> matching_elements;
+    std::vector<uint32_t>  matching_elements;
 
     for (uint32_t doc : docs) {
         if (search->seek(doc)) {

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

@@ -170,8 +170,10 @@ MatchToolsFactory::MatchToolsFactory(
       _object_store(nullptr),
       _metaStore(metaStore),
       _needed_handles() {
-    if (doom.soft_doom())
+    if (doom.soft_doom()) {
         return;
+    }
+    auto enum_guard = search::queryeval::Blueprint::auto_enum();
     auto trace = root_trace.make_trace();
     trace.addEvent(4, "Start query setup");
     _query.setWhiteListBlueprint(metaStore.createWhiteListBlueprint());
@@ -200,14 +202,10 @@ MatchToolsFactory::MatchToolsFactory(
                 setup_profiler = std::make_unique<vespalib::ExecutionProfiler>(depth);
             }
         }
-        if (trace.getLevel() >= 6 || setup_profiler) {
-            // need stable ids in the blueprint tree for profiler / dump output
-            _query.enumerate_blueprint_nodes();
-        }
         trace.addEvent(4, "Perform dictionary lookups and posting lists initialization");
         {
-            auto info = ExecuteInfo::create(in_flow.rate(), _requestContext.getDoom(), thread_bundle,
-                                            setup_profiler.get());
+            auto info =
+                ExecuteInfo::create(in_flow.rate(), _requestContext.getDoom(), thread_bundle, setup_profiler.get());
             FetchPostingsProfilerGuard guard(setup_profiler.get(), *_query.peekRoot());
             _query.fetchPostings(info);
         }

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

@@ -230,10 +230,6 @@ void Query::tag_needed_handles(HandleRecorder& handle_recorder, const search::fe
     proton::matching::tag_needed_handles(*_query_tree, handle_recorder, index_env);
 }
 
-void Query::enumerate_blueprint_nodes() noexcept {
-    _blueprint->enumerate(1);
-}
-
 void Query::optimize(InFlow in_flow, bool sort_by_cost) {
     _in_flow = in_flow;
     bool allow_force_strict = sort_by_cost && in_flow.strict();

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

@@ -96,8 +96,6 @@ class Query {
 
     void tag_needed_handles(HandleRecorder& handle_recorder, const search::fef::IIndexEnvironment& index_env);
 
-    void enumerate_blueprint_nodes() noexcept;
-
     /**
      * Optimize the query to be executed. This function should be
      * called after the reserveHandles function and before the

searchlib/src/tests/queryeval/blueprint/blueprint_test.cpp

@@ -809,18 +809,17 @@ void check_ids(Blueprint& bp, const std::vector<uint32_t>& expect, const std::st
 }
 
 TEST(BlueprintTest, blueprint_node_enumeration) {
-    auto a = std::make_unique<AndBlueprint>();
-    a->addChild(std::make_unique<MyLeaf>());
-    a->addChild(std::make_unique<MyLeaf>());
-    auto b = std::make_unique<AndBlueprint>();
-    b->addChild(std::make_unique<MyLeaf>());
-    b->addChild(std::make_unique<MyLeaf>());
-    auto root = std::make_unique<OrBlueprint>();
+    auto guard = Blueprint::auto_enum(10);
+    auto a = std::make_unique<AndBlueprint>();   // 10
+    a->addChild(std::make_unique<MyLeaf>());     // 11
+    a->addChild(std::make_unique<MyLeaf>());     // 12
+    auto b = std::make_unique<AndBlueprint>();   // 13
+    b->addChild(std::make_unique<MyLeaf>());     // 14
+    b->addChild(std::make_unique<MyLeaf>());     // 15
+    auto root = std::make_unique<OrBlueprint>(); // 16
     root->addChild(std::move(a));
     root->addChild(std::move(b));
-    check_ids(*root, {0, 0, 0, 0, 0, 0, 0}, "before enumerate");
-    root->enumerate(1);
-    check_ids(*root, {3, 4, 2, 6, 7, 5, 1}, "after enumerate");
+    check_ids(*root, {11, 12, 10, 14, 15, 13, 16}, "auto enumerated");
 }
 
 GTEST_MAIN_RUN_ALL_TESTS()

searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp

@@ -241,16 +241,15 @@ void collect_fetch_postings_counts(std::map<std::string, size_t>& counts, const
 } // namespace
 
 TEST(IntermediateBlueprintsTest, test_fetch_postings_can_be_profiled) {
+    auto enum_guard = Blueprint::auto_enum();
     auto bp = std::make_unique<AndBlueprint>();
     bp->addChild(ap(MyLeafSpec(20).create()));
     bp->addChild(ap(MyLeafSpec(200).create()));
     bp->addChild(ap(MyLeafSpec(2000).create()));
     bp->setDocIdLimit(5000);
     optimize(bp, true);
-    bp->enumerate(1);
     vespalib::ExecutionProfiler profiler(64);
-    auto info = ExecuteInfo::create(1.0, vespalib::Doom::never(),
-                                    vespalib::ThreadBundle::trivial(), &profiler);
+    auto info = ExecuteInfo::create(1.0, vespalib::Doom::never(), vespalib::ThreadBundle::trivial(), &profiler);
     {
         FetchPostingsProfilerGuard guard(&profiler, *bp);
         bp->fetchPostings(info);
@@ -267,7 +266,7 @@ TEST(IntermediateBlueprintsTest, test_fetch_postings_can_be_profiled) {
 }
 
 TEST(IntermediateBlueprintsTest, test_fetch_postings_profile_omits_id_when_unset) {
-    auto leaf = ap(MyLeafSpec(20).create());
+    auto                        leaf = ap(MyLeafSpec(20).create());
     vespalib::ExecutionProfiler profiler(64);
     {
         FetchPostingsProfilerGuard guard(&profiler, *leaf);

searchlib/src/vespa/searchlib/queryeval/blueprint.cpp

@@ -58,6 +58,35 @@ void maybe_eliminate_self(Blueprint*& self, Blueprint::UP replacement) {
 
 //-----------------------------------------------------------------------------
 
+namespace {
+
+uint32_t& next_enum_ref() {
+    thread_local uint32_t next_enum = 0;
+    return next_enum;
+}
+
+uint32_t make_enum_value() {
+    uint32_t& next_enum = next_enum_ref();
+    if (next_enum == 0) {
+        return 0;
+    }
+    return next_enum++;
+}
+
+} // namespace
+
+Blueprint::AutoEnumGuard::AutoEnumGuard(uint32_t first_id) noexcept {
+    // enable auto enum
+    next_enum_ref() = first_id;
+}
+
+Blueprint::AutoEnumGuard::~AutoEnumGuard() {
+    // disable auto enum
+    next_enum_ref() = 0;
+}
+
+//-----------------------------------------------------------------------------
+
 thread_local Blueprint::Options Blueprint::_opts;
 
 Blueprint::HitEstimate Blueprint::max(const std::vector<HitEstimate>& data) {
@@ -121,7 +150,7 @@ Blueprint::Blueprint() noexcept
       _flow_stats(0.0, 0.0, 0.0),
       _sourceId(0xffffffff),
       _docid_limit(0),
-      _id(0),
+      _id(make_enum_value()),
       _strict(false),
       _frozen(false) {
 }
@@ -138,11 +167,6 @@ void Blueprint::resolve_strict(InFlow& in_flow) noexcept {
     _strict = in_flow.strict();
 }
 
-uint32_t Blueprint::enumerate(uint32_t next_id) noexcept {
-    set_id(next_id++);
-    return next_id;
-}
-
 void Blueprint::each_node_post_order(const std::function<void(Blueprint&)>& f) {
     f(*this);
 }
@@ -426,14 +450,6 @@ void IntermediateBlueprint::setDocIdLimit(uint32_t limit) noexcept {
     }
 }
 
-uint32_t IntermediateBlueprint::enumerate(uint32_t next_id) noexcept {
-    set_id(next_id++);
-    for (Blueprint::UP& child : _children) {
-        next_id = child->enumerate(next_id);
-    }
-    return next_id;
-}
-
 void IntermediateBlueprint::each_node_post_order(const std::function<void(Blueprint&)>& f) {
     for (Blueprint::UP& child : _children) {
         child->each_node_post_order(f);
@@ -642,11 +658,11 @@ void IntermediateBlueprint::visitMembers(vespalib::ObjectVisitor& visitor) const
 }
 
 void IntermediateBlueprint::fetchPostings(const ExecuteInfo& execInfo) {
-    auto flow = my_flow(InFlow(strict(), execInfo.hit_rate()));
+    auto  flow = my_flow(InFlow(strict(), execInfo.hit_rate()));
     auto* profiler = execInfo.profiler();
     for (const auto& child : _children) {
-        double nextHitRate = flow.flow();
-        auto childInfo = ExecuteInfo::create(nextHitRate, execInfo);
+        double                     nextHitRate = flow.flow();
+        auto                       childInfo = ExecuteInfo::create(nextHitRate, execInfo);
         FetchPostingsProfilerGuard guard(profiler, *child);
         child->fetchPostings(childInfo);
         flow.add(child->estimate());

searchlib/src/vespa/searchlib/queryeval/blueprint.h

@@ -115,7 +115,20 @@ class Blueprint {
         BindOpts& operator=(const BindOpts&) = delete;
     };
 
+    // guard used to enable auto-enumeration of blueprints
+    struct AutoEnumGuard {
+        explicit AutoEnumGuard(uint32_t first_id) noexcept;
+        ~AutoEnumGuard();
+        AutoEnumGuard(AutoEnumGuard&&) = delete;
+        AutoEnumGuard(const AutoEnumGuard&) = delete;
+        AutoEnumGuard& operator=(AutoEnumGuard&&) = delete;
+        AutoEnumGuard& operator=(const AutoEnumGuard&) = delete;
+    };
+
 public:
+    // enable auto enumeration of blueprints (need to keep guard alive)
+    static AutoEnumGuard auto_enum(uint32_t next_id = 1) noexcept { return AutoEnumGuard(next_id); }
+
     // thread local Options are used during query planning (calculate_flow_stats/sort)
     //
     // The optimize_and_sort function will handle this for you by
@@ -313,9 +326,7 @@ class Blueprint {
     virtual void setDocIdLimit(uint32_t limit) noexcept { _docid_limit = limit; }
     uint32_t get_docid_limit() const noexcept { return _docid_limit; }
 
-    void set_id(uint32_t value) noexcept { _id = value; }
     uint32_t id() const noexcept { return _id; }
-    virtual uint32_t enumerate(uint32_t next_id) noexcept;
 
     bool strict() const noexcept { return _strict; }
 
@@ -535,7 +546,6 @@ class IntermediateBlueprint : public blueprint::StateCache {
     ~IntermediateBlueprint() override;
 
     void setDocIdLimit(uint32_t limit) noexcept final;
-    uint32_t enumerate(uint32_t next_id) noexcept override;
     void each_node_post_order(const std::function<void(Blueprint&)>& f) override;
 
     void optimize(Blueprint*& self, OptimizePass pass) override;
@@ -639,7 +649,7 @@ struct ComplexLeafBlueprint : LeafBlueprint {
  **/
 struct FetchPostingsProfilerGuard : vespalib::ProfilerNameGuard {
     FetchPostingsProfilerGuard(vespalib::ExecutionProfiler* profiler_in, const Blueprint& bp) noexcept
-        : ProfilerNameGuard(profiler_in, [&]{
+        : ProfilerNameGuard(profiler_in, [&] {
               if (bp.id() == 0) {
                   return vespalib::make_string("[]%s::fetchPostings", bp.getClassName().c_str());
               }

searchlib/src/vespa/searchlib/queryeval/executeinfo.h

@@ -5,7 +5,9 @@
 #include <vespa/vespalib/util/doom.h>
 #include <vespa/vespalib/util/thread_bundle.h>
 
-namespace vespalib { class ExecutionProfiler; }
+namespace vespalib {
+class ExecutionProfiler;
+}
 
 namespace search::queryeval {
 
@@ -31,8 +33,7 @@ class ExecuteInfo {
                               vespalib::ThreadBundle& thread_bundle_in) noexcept {
         return {hitRate, doom, thread_bundle_in, nullptr};
     }
-    static ExecuteInfo create(double hitRate, const vespalib::Doom& doom,
-                              vespalib::ThreadBundle& thread_bundle_in,
+    static ExecuteInfo create(double hitRate, const vespalib::Doom& doom, vespalib::ThreadBundle& thread_bundle_in,
                               vespalib::ExecutionProfiler* profiler_in) noexcept {
         return {hitRate, doom, thread_bundle_in, profiler_in};
     }