- Avoid going via a temporary IdealNodesList.

- Use ConstArrayRef to hide implementation.
- Store all 3 node categories in a single vector.
- Use a small_vector that can handle redundancy up to 5 without requiring extra memory allocation.
- Build a hash_map if redundancy/groups > 32 for constant lookup time.

3 files changed, 45 insertions, 37 deletions

diff --git a/vdslib/src/tests/distribution/CMakeLists.txt b/vdslib/src/tests/distribution/CMakeLists.txt
index c4ae8b0291c..3f3be1e1cad 100644
--- a/vdslib/src/tests/distribution/CMakeLists.txt
+++ b/vdslib/src/tests/distribution/CMakeLists.txt
@@ -3,7 +3,6 @@ vespa_add_library(vdslib_testdistribution
     SOURCES
     distributiontest.cpp
     grouptest.cpp
-    idealnodecalculatorimpltest.cpp
     DEPENDS
     vdslib
     GTest::GTest
diff --git a/vdslib/src/tests/distribution/distributiontest.cpp b/vdslib/src/tests/distribution/distributiontest.cpp
index ec7c05fa7a2..ce07711a069 100644
--- a/vdslib/src/tests/distribution/distributiontest.cpp
+++ b/vdslib/src/tests/distribution/distributiontest.cpp
@@ -5,7 +5,6 @@
 #include <vespa/config/subscription/configuri.h>
 #include <vespa/fastos/file.h>
 #include <vespa/vdslib/distribution/distribution.h>
-#include <vespa/vdslib/distribution/idealnodecalculator.h>
 #include <vespa/vdslib/state/clusterstate.h>
 #include <vespa/vdslib/state/random.h>
 #include <vespa/vespalib/data/slime/slime.h>
@@ -84,6 +83,51 @@ TEST(DistributionTest, test_verify_java_distributions)
 
 namespace {
 
+/**
+* A list of ideal nodes, sorted in preferred order. Wraps a vector to hide
+* unneeded details, and make it easily printable.
+*/
+class IdealNodeList : public document::Printable {
+public:
+    IdealNodeList() noexcept;
+    ~IdealNodeList();
+
+    void push_back(const Node& node) {
+        _idealNodes.push_back(node);
+    }
+
+    const Node& operator[](uint32_t i) const noexcept { return _idealNodes[i]; }
+    uint32_t size() const noexcept { return _idealNodes.size(); }
+    bool contains(const Node& n) const noexcept {
+        return indexOf(n) != 0xffff;
+    }
+    uint16_t indexOf(const Node& n) const noexcept {
+        for (uint16_t i=0; i<_idealNodes.size(); ++i) {
+            if (n == _idealNodes[i]) return i;
+        }
+        return 0xffff;
+    }
+
+    void print(std::ostream& out, bool, const std::string &) const override;
+private:
+    std::vector<Node> _idealNodes;
+};
+
+IdealNodeList::IdealNodeList() noexcept = default;
+IdealNodeList::~IdealNodeList() = default;
+
+void
+IdealNodeList::print(std::ostream& out, bool , const std::string &) const
+{
+    out << "[";
+    for (uint32_t i=0; i<_idealNodes.size(); ++i) {
+        if (i != 0) out << ", ";
+        out << _idealNodes[i];
+    }
+    out << "]";
+}
+
+
 struct ExpectedResult {
     ExpectedResult() { }
     ExpectedResult(const ExpectedResult &) = default;
diff --git a/vdslib/src/tests/distribution/idealnodecalculatorimpltest.cpp b/vdslib/src/tests/distribution/idealnodecalculatorimpltest.cpp
deleted file mode 100644
index 4159491097c..00000000000
--- a/vdslib/src/tests/distribution/idealnodecalculatorimpltest.cpp
+++ /dev/null
@@ -1,35 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#include <vespa/config-stor-distribution.h>
-#include <vespa/vdslib/distribution/idealnodecalculatorimpl.h>
-#include <vespa/vdslib/distribution/distribution.h>
-#include <vespa/vdslib/state/clusterstate.h>
-#include <vespa/vespalib/gtest/gtest.h>
-
-namespace storage::lib {
-
-/**
- * Class is just a wrapper for distribution, so little needs to be tested. Just
- * that:
- *
- *   - get ideal nodes calls gets propagated correctly.
- *   - Changes in distribution/cluster state is picked up.
- */
-
-TEST(IdealNodeCalculatorImplTest, test_normal_usage)
-{
-    ClusterState state("storage:10");
-    Distribution distr(Distribution::getDefaultDistributionConfig(3, 10));
-    IdealNodeCalculatorImpl impl;
-    IdealNodeCalculatorConfigurable& configurable(impl);
-    IdealNodeCalculator& calc(impl);
-    configurable.setDistribution(distr);
-    configurable.setClusterState(state);
-
-    std::string expected("[storage.8, storage.9, storage.6]");
-    EXPECT_EQ(
-            expected,
-            calc.getIdealStorageNodes(document::BucketId(16, 5)).toString());
-}
-
-}