From 28912325d52e239e1563cc5004723de8d650ac25 Mon Sep 17 00:00:00 2001
From: Henning Baldersheim <balder@yahoo-inc.com>
Date: Thu, 10 Aug 2023 19:49:36 +0000
Subject: - 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.

---
 vdslib/src/tests/distribution/CMakeLists.txt       |  1 -
 vdslib/src/tests/distribution/distributiontest.cpp | 46 +++++++++++++++++++++-
 .../distribution/idealnodecalculatorimpltest.cpp   | 35 ----------------
 3 files changed, 45 insertions(+), 37 deletions(-)
 delete mode 100644 vdslib/src/tests/distribution/idealnodecalculatorimpltest.cpp

(limited to 'vdslib/src/tests')

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());
-}
-
-}
-- 
cgit v1.2.3