Improve the distribution by using a indirect mapping that should improve

chance for better loaddistribution.

3 files changed, 40 insertions, 15 deletions

diff --git a/searchlib/src/tests/common/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp b/searchlib/src/tests/common/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp
index 77caf535405..558d24551a8 100644
--- a/searchlib/src/tests/common/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp
+++ b/searchlib/src/tests/common/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp
@@ -19,10 +19,7 @@ class Fixture
 public:
     SequencedTaskExecutor _threads;
 
-    Fixture()
-        : _threads(2)
-    {
-    }
+    Fixture() : _threads(2) { }
 };
 
 
@@ -101,7 +98,7 @@ TEST_F("require that task with different component ids are not serialized", Fixt
         std::shared_ptr<TestObj> tv(std::make_shared<TestObj>());
         EXPECT_EQUAL(0, tv->_val);
         f._threads.execute(0, [=]() { usleep(2000); tv->modify(0, 14); });
-        f._threads.execute(1, [=]() { tv->modify(14, 42); });
+        f._threads.execute(2, [=]() { tv->modify(14, 42); });
         tv->wait(2);
         if (tv->_fail != 1) {
              continue;
@@ -175,12 +172,13 @@ vespalib::string makeAltComponentId(Fixture &f)
 
 TEST_F("require that task with different string component ids are not serialized", Fixture)
 {
-    int tryCnt = detectSerializeFailure(f, "1", 100);
+    int tryCnt = detectSerializeFailure(f, "2", 100);
     EXPECT_TRUE(tryCnt < 100);
 }
 
 
-TEST_F("require that task with different string component ids mapping to the same executor id are serialized", Fixture)
+TEST_F("require that task with different string component ids mapping to the same executor id are serialized",
+       Fixture)
 {
     vespalib::string altComponentId = makeAltComponentId(f);
     LOG(info, "second string component id is \"%s\"", altComponentId.c_str());
diff --git a/searchlib/src/vespa/searchlib/common/isequencedtaskexecutor.cpp b/searchlib/src/vespa/searchlib/common/isequencedtaskexecutor.cpp
index ea167924a2d..9d1bc99bebb 100644
--- a/searchlib/src/vespa/searchlib/common/isequencedtaskexecutor.cpp
+++ b/searchlib/src/vespa/searchlib/common/isequencedtaskexecutor.cpp
@@ -2,12 +2,22 @@
 
 #include "isequencedtaskexecutor.h"
 #include <vespa/vespalib/stllike/hash_fun.h>
+#include <vespa/vespalib/stllike/hashtable.h>
+#include <cassert>
 
 namespace search {
+    namespace {
+        constexpr uint8_t MAGIC = 255;
+    }
 
 ISequencedTaskExecutor::ISequencedTaskExecutor(uint32_t numExecutors)
-    : _numExecutors(numExecutors)
-{ }
+    : _component2Id(vespalib::hashtable_base::getModuloStl(numExecutors*8), MAGIC),
+      _mutex(),
+      _numExecutors(numExecutors),
+      _nextId(0)
+{
+    assert(numExecutors < 256);
+}
 
 ISequencedTaskExecutor::~ISequencedTaskExecutor() = default;
 
@@ -17,4 +27,19 @@ ISequencedTaskExecutor::getExecutorId(vespalib::stringref componentId) const {
     return getExecutorId(hashfun(componentId));
 }
 
+ISequencedTaskExecutor::ExecutorId
+ISequencedTaskExecutor::getExecutorId(uint64_t componentId) const {
+    uint32_t shrunkId = componentId % _component2Id.size();
+    uint8_t executorId = _component2Id[shrunkId];
+    if (executorId == MAGIC) {
+        std::lock_guard guard(_mutex);
+        if (_component2Id[shrunkId] == MAGIC) {
+            _component2Id[shrunkId] = _nextId % getNumExecutors();
+            _nextId++;
+        }
+        executorId = _component2Id[shrunkId];
+    }
+    return ExecutorId(executorId);
+}
+
 }
diff --git a/searchlib/src/vespa/searchlib/common/isequencedtaskexecutor.h b/searchlib/src/vespa/searchlib/common/isequencedtaskexecutor.h
index 00674221b0d..d1318a2b71c 100644
--- a/searchlib/src/vespa/searchlib/common/isequencedtaskexecutor.h
+++ b/searchlib/src/vespa/searchlib/common/isequencedtaskexecutor.h
@@ -4,6 +4,8 @@
 #include <vespa/vespalib/util/executor.h>
 #include <vespa/vespalib/stllike/string.h>
 #include <vespa/vespalib/util/lambdatask.h>
+#include <vector>
+#include <mutex>
 
 namespace search {
 
@@ -29,15 +31,12 @@ public:
     virtual ~ISequencedTaskExecutor();
 
     /**
-     * Calculate which executor will handle an component. All callers
-     * must be in the same thread.
+     * Calculate which executor will handle an component.
      *
      * @param componentId   component id
      * @return              executor id
      */
-    ExecutorId getExecutorId(uint64_t componentId) const {
-        return ExecutorId((componentId * 1099511628211ULL ) % _numExecutors);
-    }
+    ExecutorId getExecutorId(uint64_t componentId) const;
     uint32_t getNumExecutors() const { return _numExecutors; }
 
     ExecutorId getExecutorId(vespalib::stringref componentId) const;
@@ -97,7 +96,10 @@ public:
         executeTask(id, vespalib::makeLambdaTask(std::forward<FunctionType>(function)));
     }
 private:
-    uint32_t _numExecutors;
+    mutable std::vector<uint8_t> _component2Id;
+    mutable std::mutex           _mutex;
+    uint32_t                     _numExecutors;
+    mutable uint32_t             _nextId;
 };
 
 } // namespace search