Merge pull request #28021 from vespa-engine/balder/generate-fast-idealstate-lookup-once

Balder/generate fast idealstate lookup once

14 files changed, 61 insertions, 75 deletions

diff --git a/storage/src/tests/distributor/distributor_bucket_space_test.cpp b/storage/src/tests/distributor/distributor_bucket_space_test.cpp
index 6e5863298ce..3ea4c1ca3c2 100644
--- a/storage/src/tests/distributor/distributor_bucket_space_test.cpp
+++ b/storage/src/tests/distributor/distributor_bucket_space_test.cpp
@@ -106,13 +106,13 @@ DistributorBucketSpaceTest::count_service_layer_buckets(const std::vector<Bucket
         for (uint32_t i = 0; i < 3; ++i) {
             switch (i) {
             case 0:
-                ideal_nodes = ideal_nodes_bundle.get_available_nodes();
+                ideal_nodes = ideal_nodes_bundle.available_nodes();
                 break;
             case 1:
-                ideal_nodes = ideal_nodes_bundle.get_available_nonretired_nodes();
+                ideal_nodes = ideal_nodes_bundle.available_nonretired_nodes();
                 break;
             case 2:
-                ideal_nodes = ideal_nodes_bundle.get_available_nonretired_or_maintenance_nodes();
+                ideal_nodes = ideal_nodes_bundle.available_nonretired_or_maintenance_nodes();
                 break;
             default:
                 ;
diff --git a/storage/src/tests/distributor/statecheckerstest.cpp b/storage/src/tests/distributor/statecheckerstest.cpp
index cd64d81774e..4ca4d70a816 100644
--- a/storage/src/tests/distributor/statecheckerstest.cpp
+++ b/storage/src/tests/distributor/statecheckerstest.cpp
@@ -1588,10 +1588,9 @@ TEST_F(StateCheckersTest, context_populates_ideal_state_containers) {
                             getDistributorBucketSpace(), statsTracker, makeDocumentBucket({17, 0}));
 
     ASSERT_THAT(c.idealState(), ElementsAre(1, 3));
-    // TODO replace with UnorderedElementsAre once we can build gmock without issues
-    std::vector<uint16_t> ideal_state(c.unorderedIdealState.begin(), c.unorderedIdealState.end());
-    std::sort(ideal_state.begin(), ideal_state.end());
-    ASSERT_THAT(ideal_state, ElementsAre(1, 3));
+    for (uint16_t node : c.idealState()) {
+        ASSERT_TRUE(c.idealStateBundle.is_nonretired_or_maintenance(node));
+    }
 }
 
 namespace {
@@ -1606,8 +1605,7 @@ public:
     explicit StateCheckerRunner(StateCheckersTest& fixture);
     ~StateCheckerRunner();
 
-    StateCheckerRunner& addToDb(const document::BucketId& bid,
-                                const std::string& bucketInfo)
+    StateCheckerRunner& addToDb(const document::BucketId& bid, const std::string& bucketInfo)
     {
         _fixture.addNodesToBucketDB(bid, bucketInfo);
         return *this;
@@ -1642,8 +1640,7 @@ public:
         Checker checker;
         StateChecker::Context c(_fixture.node_context(), _fixture.operation_context(),
                                 _fixture.getDistributorBucketSpace(), _statsTracker, makeDocumentBucket(bid));
-        _result = _fixture.testStateChecker(
-                checker, c, false, StateCheckersTest::PendingMessage(), false);
+        _result = _fixture.testStateChecker(checker, c, false, StateCheckersTest::PendingMessage(), false);
     }
 
     const std::string& result() const { return _result; }
diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp b/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp
index 713133667c4..299aaffb569 100644
--- a/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp
+++ b/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp
@@ -108,9 +108,7 @@ DistributorBucketSpace::owns_bucket_in_state(
 }
 
 bool
-DistributorBucketSpace::owns_bucket_in_state(
-        const lib::ClusterState& clusterState,
-        document::BucketId bucket) const
+DistributorBucketSpace::owns_bucket_in_state(const lib::ClusterState& clusterState, document::BucketId bucket) const
 {
     return owns_bucket_in_state(*_distribution, clusterState, bucket);
 }
diff --git a/storage/src/vespa/storage/distributor/distributor_stripe_component.cpp b/storage/src/vespa/storage/distributor/distributor_stripe_component.cpp
index 9a5fd595b1d..1121c5071c0 100644
--- a/storage/src/vespa/storage/distributor/distributor_stripe_component.cpp
+++ b/storage/src/vespa/storage/distributor/distributor_stripe_component.cpp
@@ -168,7 +168,7 @@ DistributorStripeComponent::update_bucket_database(
 
     UpdateBucketDatabaseProcessor processor(getClock(),
                                             found_down_node ? up_nodes : changed_nodes,
-                                            bucketSpace.get_ideal_service_layer_nodes_bundle(bucket.getBucketId()).get_available_nodes(),
+                                            bucketSpace.get_ideal_service_layer_nodes_bundle(bucket.getBucketId()).available_nodes(),
                                             (update_flags & DatabaseUpdate::RESET_TRUSTED) != 0);
 
     bucketSpace.getBucketDatabase().process_update(bucket.getBucketId(), processor, (update_flags & DatabaseUpdate::CREATE_IF_NONEXISTING) != 0);
diff --git a/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.cpp b/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.cpp
index 0d37219356e..cc4eedd2a35 100644
--- a/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.cpp
+++ b/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.cpp
@@ -2,16 +2,27 @@
 
 #include "ideal_service_layer_nodes_bundle.h"
 #include <vespa/vdslib/distribution/idealnodecalculator.h>
+#include <vespa/vespalib/stllike/hash_set_insert.hpp>
+
 
 namespace storage::distributor {
 
 IdealServiceLayerNodesBundle::IdealServiceLayerNodesBundle() noexcept
   : _available_nodes(),
     _available_nonretired_nodes(),
-    _available_nonretired_or_maintenance_nodes()
+    _available_nonretired_or_maintenance_nodes(),
+    _unordered_nonretired_or_maintenance_nodes()
 {
 }
 
+void
+IdealServiceLayerNodesBundle::set_available_nonretired_or_maintenance_nodes(std::vector<uint16_t> available_nonretired_or_maintenance_nodes) {
+    _available_nonretired_or_maintenance_nodes = std::move(available_nonretired_or_maintenance_nodes);
+    _unordered_nonretired_or_maintenance_nodes.clear();
+    _unordered_nonretired_or_maintenance_nodes.insert(_available_nonretired_or_maintenance_nodes.begin(),
+                                                      _available_nonretired_or_maintenance_nodes.end());
+}
+
 IdealServiceLayerNodesBundle::IdealServiceLayerNodesBundle(IdealServiceLayerNodesBundle &&) noexcept = default;
 
 IdealServiceLayerNodesBundle::~IdealServiceLayerNodesBundle() = default;
diff --git a/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h b/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h
index bb2e2c15996..9577ec09208 100644
--- a/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h
+++ b/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h
@@ -1,8 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
-#include <vector>
-#include <cstdint>
+#include <vespa/vespalib/stllike/hash_set.h>
 
 namespace storage::distributor {
 
@@ -13,6 +12,7 @@ class IdealServiceLayerNodesBundle {
     std::vector<uint16_t> _available_nodes;
     std::vector<uint16_t> _available_nonretired_nodes;
     std::vector<uint16_t> _available_nonretired_or_maintenance_nodes;
+    vespalib::hash_set<uint16_t> _unordered_nonretired_or_maintenance_nodes;
 public:
     IdealServiceLayerNodesBundle() noexcept;
     IdealServiceLayerNodesBundle(IdealServiceLayerNodesBundle &&) noexcept;
@@ -24,14 +24,15 @@ public:
     void set_available_nonretired_nodes(std::vector<uint16_t> available_nonretired_nodes) {
         _available_nonretired_nodes = std::move(available_nonretired_nodes);
     }
-    void set_available_nonretired_or_maintenance_nodes(std::vector<uint16_t> available_nonretired_or_maintenance_nodes) {
-        _available_nonretired_or_maintenance_nodes = std::move(available_nonretired_or_maintenance_nodes);
-    }
-    const std::vector<uint16_t> & get_available_nodes() const noexcept { return _available_nodes; }
-    const std::vector<uint16_t> & get_available_nonretired_nodes() const noexcept { return _available_nonretired_nodes; }
-    const std::vector<uint16_t> & get_available_nonretired_or_maintenance_nodes() const noexcept {
+    void set_available_nonretired_or_maintenance_nodes(std::vector<uint16_t> available_nonretired_or_maintenance_nodes);
+    const std::vector<uint16_t> & available_nodes() const noexcept { return _available_nodes; }
+    const std::vector<uint16_t> & available_nonretired_nodes() const noexcept { return _available_nonretired_nodes; }
+    const std::vector<uint16_t> & available_nonretired_or_maintenance_nodes() const noexcept {
         return _available_nonretired_or_maintenance_nodes;
     }
+    bool is_nonretired_or_maintenance(uint16_t node) const noexcept {
+        return _unordered_nonretired_or_maintenance_nodes.contains(node);
+    }
 };
 
 }
diff --git a/storage/src/vespa/storage/distributor/messagetracker.h b/storage/src/vespa/storage/distributor/messagetracker.h
index 51d29551de0..a0234f425a0 100644
--- a/storage/src/vespa/storage/distributor/messagetracker.h
+++ b/storage/src/vespa/storage/distributor/messagetracker.h
@@ -26,8 +26,8 @@ public:
     };
 
     MessageTracker(const ClusterContext& cluster_context);
-    MessageTracker(MessageTracker&&) = default;
-    MessageTracker& operator=(MessageTracker&&) = delete;
+    MessageTracker(MessageTracker&&) noexcept = default;
+    MessageTracker& operator=(MessageTracker&&) noexcept = delete;
     MessageTracker(const MessageTracker &) = delete;
     MessageTracker& operator=(const MessageTracker&) = delete;
     ~MessageTracker();
diff --git a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp
index f76b6495443..86ea9a559f5 100644
--- a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp
+++ b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp
@@ -67,7 +67,8 @@ PutOperation::insertDatabaseEntryAndScheduleCreateBucket(const OperationTargetLi
         assert(!multipleBuckets);
         (void) multipleBuckets;
         BucketDatabase::Entry entry(_bucket_space.getBucketDatabase().get(lastBucket));
-        const std::vector<uint16_t> & idealState = _bucket_space.get_ideal_service_layer_nodes_bundle(lastBucket).get_available_nodes();
+        const std::vector<uint16_t> & idealState = _bucket_space.get_ideal_service_layer_nodes_bundle(
+                lastBucket).available_nodes();
         active = ActiveCopy::calculate(idealState, _bucket_space.getDistribution(), entry,
                                        _op_ctx.distributor_config().max_activation_inhibited_out_of_sync_groups());
         LOG(debug, "Active copies for bucket %s: %s", entry.getBucketId().toString().c_str(), active.toString().c_str());
diff --git a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp
index e3aa7e1ef6e..fcd27fdab74 100644
--- a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp
+++ b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp
@@ -71,7 +71,8 @@ BucketInstanceList::populate(const document::BucketId& specificId, const Distrib
     std::vector<BucketDatabase::Entry> entries;
     db.getParents(specificId, entries);
     for (const auto & entry : entries) {
-        lib::IdealNodeList idealNodes(make_node_list(distributor_bucket_space.get_ideal_service_layer_nodes_bundle(entry.getBucketId()).get_available_nonretired_or_maintenance_nodes()));
+        lib::IdealNodeList idealNodes(make_node_list(distributor_bucket_space.get_ideal_service_layer_nodes_bundle(
+                entry.getBucketId()).available_nonretired_or_maintenance_nodes()));
         add(entry, idealNodes);
     }
 }
@@ -127,7 +128,8 @@ BucketInstanceList::extendToEnoughCopies(
                                                     : _instances[0]._bucket);
     newTarget = leastSpecificLeafBucketInSubtree(newTarget, mostSpecificId, db);
 
-    lib::IdealNodeList idealNodes(make_node_list(distributor_bucket_space.get_ideal_service_layer_nodes_bundle(newTarget).get_available_nonretired_nodes()));
+    lib::IdealNodeList idealNodes(make_node_list(
+            distributor_bucket_space.get_ideal_service_layer_nodes_bundle(newTarget).available_nonretired_nodes()));
     for (uint32_t i=0; i<idealNodes.size(); ++i) {
         if (!contains(idealNodes[i])) {
             _instances.push_back(BucketInstance(
diff --git a/storage/src/vespa/storage/distributor/statechecker.cpp b/storage/src/vespa/storage/distributor/statechecker.cpp
index 96ad3b46e5d..cd8b6e934d4 100644
--- a/storage/src/vespa/storage/distributor/statechecker.cpp
+++ b/storage/src/vespa/storage/distributor/statechecker.cpp
@@ -78,9 +78,7 @@ StateChecker::Context::Context(const DistributorNodeContext& node_ctx_in,
       db(distributorBucketSpace.getBucketDatabase()),
       stats(statsTracker),
       merges_inhibited_in_bucket_space(distributorBucketSpace.merges_inhibited())
-{
-    unorderedIdealState.insert(idealState().begin(), idealState().end());
-}
+{ }
 
 StateChecker::Context::~Context() = default;
 
diff --git a/storage/src/vespa/storage/distributor/statechecker.h b/storage/src/vespa/storage/distributor/statechecker.h
index ef50c78f068..25918e7a047 100644
--- a/storage/src/vespa/storage/distributor/statechecker.h
+++ b/storage/src/vespa/storage/distributor/statechecker.h
@@ -64,27 +64,20 @@ public:
         std::vector<BucketDatabase::Entry> entries;
 
         // Common
-        const lib::ClusterState&        systemState;
-        const lib::ClusterState*        pending_cluster_state; // nullptr if no state is pending.
-        const DistributorConfiguration& distributorConfig;
-        const lib::Distribution&        distribution;
-        BucketGcTimeCalculator          gcTimeCalculator;
-
-        // Separate ideal state into ordered sequence and unordered set, as we
-        // need to both know the actual order (activation prioritization etc) as
-        // well as have the ability to quickly check if a node is in an ideal
-        // location.
-        const IdealServiceLayerNodesBundle & idealStateBundle;
-        vespalib::hash_set<uint16_t> unorderedIdealState;
-
-        const DistributorNodeContext&            node_ctx;
-        const DistributorStripeOperationContext& op_ctx;
-        const BucketDatabase&                    db;
-        NodeMaintenanceStatsTracker&             stats;
-        const bool                               merges_inhibited_in_bucket_space;
+        const lib::ClusterState                 & systemState;
+        const lib::ClusterState                 * pending_cluster_state; // nullptr if no state is pending.
+        const DistributorConfiguration          & distributorConfig;
+        const lib::Distribution                 & distribution;
+        BucketGcTimeCalculator                    gcTimeCalculator;
+        const IdealServiceLayerNodesBundle      & idealStateBundle;
+        const DistributorNodeContext            & node_ctx;
+        const DistributorStripeOperationContext & op_ctx;
+        const BucketDatabase                    & db;
+        NodeMaintenanceStatsTracker             & stats;
+        const bool                                merges_inhibited_in_bucket_space;
 
         const BucketDatabase::Entry& getSiblingEntry() const noexcept { return siblingEntry; }
-        const std::vector<uint16_t> & idealState() const noexcept { return idealStateBundle.get_available_nonretired_or_maintenance_nodes(); }
+        const std::vector<uint16_t> & idealState() const noexcept { return idealStateBundle.available_nonretired_or_maintenance_nodes(); }
 
         document::Bucket getBucket() const noexcept { return bucket; }
         document::BucketId getBucketId() const noexcept { return bucket.getBucketId(); }
@@ -107,28 +100,19 @@ public:
         std::unique_ptr<ResultImpl> _impl;
     public:
         IdealStateOperation::UP createOperation() {
-            return (_impl
-                    ? _impl->createOperation()
-                    : IdealStateOperation::UP());
+            return (_impl ? _impl->createOperation() : IdealStateOperation::UP());
         }
 
         MaintenancePriority getPriority() const {
-            return (_impl
-                    ? _impl->getPriority()
-                    : MaintenancePriority());
+            return (_impl ? _impl->getPriority() : MaintenancePriority());
         }
 
         MaintenanceOperation::Type getType() const {
-            return (_impl
-                    ? _impl->getType()
-                    : MaintenanceOperation::OPERATION_COUNT);
-            
+            return (_impl ? _impl->getType() : MaintenanceOperation::OPERATION_COUNT);
         }
 
         static Result noMaintenanceNeeded();
-        static Result createStoredResult(
-                IdealStateOperation::UP operation,
-                MaintenancePriority::Priority priority);
+        static Result createStoredResult(IdealStateOperation::UP operation, MaintenancePriority::Priority priority);
     private:
         explicit Result(std::unique_ptr<ResultImpl> impl)
             : _impl(std::move(impl))
diff --git a/storage/src/vespa/storage/distributor/statecheckers.cpp b/storage/src/vespa/storage/distributor/statecheckers.cpp
index 56247d744bf..43766225155 100644
--- a/storage/src/vespa/storage/distributor/statecheckers.cpp
+++ b/storage/src/vespa/storage/distributor/statecheckers.cpp
@@ -656,9 +656,9 @@ private:
 MergeNodes::~MergeNodes() = default;
 
 bool
-presentInIdealState(const StateChecker::Context& c, uint16_t node)
+presentInIdealState(const StateChecker::Context& c, uint16_t node) noexcept
 {
-    return c.unorderedIdealState.find(node) != c.unorderedIdealState.end();
+    return c.idealStateBundle.is_nonretired_or_maintenance(node);
 }
 
 void
diff --git a/vdslib/src/vespa/vdslib/distribution/distribution.cpp b/vdslib/src/vespa/vdslib/distribution/distribution.cpp
index 637a5089822..9dda360eea5 100644
--- a/vdslib/src/vespa/vdslib/distribution/distribution.cpp
+++ b/vdslib/src/vespa/vdslib/distribution/distribution.cpp
@@ -459,9 +459,7 @@ Distribution::getDefaultDistributionConfig(uint16_t redundancy, uint16_t nodeCou
 }
 
 std::vector<uint16_t>
-Distribution::getIdealStorageNodes(
-            const ClusterState& state, const document::BucketId& bucket,
-            const char* upStates) const
+Distribution::getIdealStorageNodes(const ClusterState& state, const document::BucketId& bucket, const char* upStates) const
 {
     std::vector<uint16_t> nodes;
     getIdealNodes(NodeType::STORAGE, state, bucket, nodes, upStates);
@@ -469,10 +467,7 @@ Distribution::getIdealStorageNodes(
 }
 
 uint16_t
-Distribution::getIdealDistributorNode(
-            const ClusterState& state,
-            const document::BucketId& bucket,
-            const char* upStates) const
+Distribution::getIdealDistributorNode(const ClusterState& state, const document::BucketId& bucket, const char* upStates) const
 {
     std::vector<uint16_t> nodes;
     getIdealNodes(NodeType::DISTRIBUTOR, state, bucket, nodes, upStates);
diff --git a/vespalib/src/vespa/vespalib/stllike/hash_set_insert.hpp b/vespalib/src/vespa/vespalib/stllike/hash_set_insert.hpp
index 6d5b7ed8b05..77e46bbf9e8 100644
--- a/vespalib/src/vespa/vespalib/stllike/hash_set_insert.hpp
+++ b/vespalib/src/vespa/vespalib/stllike/hash_set_insert.hpp
@@ -9,7 +9,7 @@ namespace vespalib {
 template<typename K, typename H, typename EQ, typename M>
 template<typename InputIterator>
 hash_set<K, H, EQ, M>::hash_set(InputIterator first, InputIterator last)
-    : _ht(0)
+    : _ht(last - first)
 {
     insert(first, last);
 }
@@ -18,7 +18,6 @@ template<typename K, typename H, typename EQ, typename M>
 template<typename InputIt>
 void
 hash_set<K, H, EQ, M>::insert(InputIt first, InputIt last) {
-    _ht.resize(last - first + capacity());
     for (; first < last; first++) {
         insert(*first);
     }