Merge pull request #11640 from vespa-engine/balder/simpler-and-safer-hash-rebased

Balder/simpler and safer hash rebased

9 files changed, 24 insertions, 68 deletions

diff --git a/searchcore/src/tests/proton/attribute/attribute_test.cpp b/searchcore/src/tests/proton/attribute/attribute_test.cpp
index 3da27cde10e..2d75bb8906b 100644
--- a/searchcore/src/tests/proton/attribute/attribute_test.cpp
+++ b/searchcore/src/tests/proton/attribute/attribute_test.cpp
@@ -9,8 +9,6 @@
 #include <vespa/document/update/documentupdate.h>
 #include <vespa/eval/tensor/tensor.h>
 #include <vespa/eval/tensor/default_tensor_engine.h>
-#include <vespa/eval/tensor/types.h>
-#include <vespa/fastos/file.h>
 #include <vespa/searchcommon/attribute/attributecontent.h>
 #include <vespa/searchcore/proton/attribute/attribute_collection_spec_factory.h>
 #include <vespa/searchcore/proton/attribute/attribute_writer.h>
@@ -22,13 +20,11 @@
 #include <vespa/searchcore/proton/test/attribute_utils.h>
 #include <vespa/searchcorespi/flush/iflushtarget.h>
 #include <vespa/searchlib/attribute/attributefactory.h>
-#include <vespa/searchlib/attribute/attributevector.hpp>
 #include <vespa/searchlib/attribute/bitvector_search_cache.h>
 #include <vespa/searchlib/attribute/imported_attribute_vector.h>
 #include <vespa/searchlib/attribute/imported_attribute_vector_factory.h>
 #include <vespa/searchlib/attribute/integerbase.h>
 #include <vespa/searchlib/attribute/predicate_attribute.h>
-#include <vespa/searchlib/attribute/singlenumericattribute.hpp>
 #include <vespa/searchlib/common/foregroundtaskexecutor.h>
 #include <vespa/searchlib/common/idestructorcallback.h>
 #include <vespa/searchlib/common/sequencedtaskexecutorobserver.h>
@@ -38,13 +34,15 @@
 #include <vespa/searchlib/predicate/predicate_index.h>
 #include <vespa/searchlib/tensor/tensor_attribute.h>
 #include <vespa/searchlib/test/directory_handler.h>
-#include <vespa/searchlib/util/filekit.h>
 #include <vespa/vespalib/io/fileutil.h>
 #include <vespa/vespalib/util/exceptions.h>
 #include <vespa/vespalib/test/insertion_operators.h>
 #include <vespa/vespalib/testkit/testapp.h>
 #include <vespa/searchcommon/attribute/iattributevector.h>
 #include <vespa/vespalib/btree/btreeroot.hpp>
+#include <vespa/searchlib/attribute/singlenumericattribute.hpp>
+
+
 
 #include <vespa/log/log.h>
 LOG_SETUP("attribute_test");
@@ -751,7 +749,7 @@ TEST_F("require that attribute writer spreads write over 2 write contexts", Fixt
     TEST_DO(putAttributes(f, {0, 1}));
 }
 
-TEST_F("require that attribute writer spreads write over 3 write contexts", Fixture(8))
+TEST_F("require that attribute writer spreads write over 3 write contexts", Fixture(3))
 {
     TEST_DO(putAttributes(f, {0, 1, 2}));
 }
diff --git a/searchlib/src/tests/common/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp b/searchlib/src/tests/common/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp
index f8ab03d7710..77caf535405 100644
--- a/searchlib/src/tests/common/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp
+++ b/searchlib/src/tests/common/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp
@@ -101,7 +101,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(2, [=]() { tv->modify(14, 42); });
+        f._threads.execute(1, [=]() { tv->modify(14, 42); });
         tv->wait(2);
         if (tv->_fail != 1) {
              continue;
@@ -173,16 +173,14 @@ vespalib::string makeAltComponentId(Fixture &f)
 
 }
 
-TEST_F("require that task with different string component ids are not serialized",
-       Fixture)
+TEST_F("require that task with different string component ids are not serialized", Fixture)
 {
-    int tryCnt = detectSerializeFailure(f, "2", 100);
+    int tryCnt = detectSerializeFailure(f, "1", 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/foregroundtaskexecutor.cpp b/searchlib/src/vespa/searchlib/common/foregroundtaskexecutor.cpp
index 91ca91be4cd..513684d3fd5 100644
--- a/searchlib/src/vespa/searchlib/common/foregroundtaskexecutor.cpp
+++ b/searchlib/src/vespa/searchlib/common/foregroundtaskexecutor.cpp
@@ -14,32 +14,16 @@ ForegroundTaskExecutor::ForegroundTaskExecutor()
 }
 
 ForegroundTaskExecutor::ForegroundTaskExecutor(uint32_t threads)
-    : _threads(threads),
-      _ids()
+    : ISequencedTaskExecutor(threads)
 {
 }
 
-ForegroundTaskExecutor::~ForegroundTaskExecutor()
-{
-}
-
-ISequencedTaskExecutor::ExecutorId
-ForegroundTaskExecutor::getExecutorId(uint64_t componentId)
-{
-    auto itr = _ids.find(componentId);
-    if (itr == _ids.end()) {
-        auto insarg = std::make_pair(componentId, ExecutorId(_ids.size() % _threads));
-        auto insres = _ids.insert(insarg);
-        assert(insres.second);
-        itr = insres.first;
-    }
-    return itr->second;
-}
+ForegroundTaskExecutor::~ForegroundTaskExecutor() = default;
 
 void
 ForegroundTaskExecutor::executeTask(ExecutorId id, vespalib::Executor::Task::UP task)
 {
-    assert(id.getId() < _threads);
+    assert(id.getId() < getNumExecutors());
     task->run();
 }
 
@@ -48,5 +32,4 @@ ForegroundTaskExecutor::sync()
 {
 }
 
-
 } // namespace search
diff --git a/searchlib/src/vespa/searchlib/common/foregroundtaskexecutor.h b/searchlib/src/vespa/searchlib/common/foregroundtaskexecutor.h
index cfd135d3fa0..2074eda009b 100644
--- a/searchlib/src/vespa/searchlib/common/foregroundtaskexecutor.h
+++ b/searchlib/src/vespa/searchlib/common/foregroundtaskexecutor.h
@@ -16,8 +16,6 @@ namespace search {
  */
 class ForegroundTaskExecutor : public ISequencedTaskExecutor
 {
-    const uint32_t                       _threads;
-    vespalib::hash_map<size_t, ExecutorId> _ids;
 public:
     using ISequencedTaskExecutor::getExecutorId;
 
@@ -25,8 +23,6 @@ public:
     ForegroundTaskExecutor(uint32_t threads);
     ~ForegroundTaskExecutor() override;
 
-    uint32_t getNumExecutors() const override { return _threads; }
-    ExecutorId getExecutorId(uint64_t componentId) override;
     void executeTask(ExecutorId id, vespalib::Executor::Task::UP task) override;
     void sync() override;
 };
diff --git a/searchlib/src/vespa/searchlib/common/isequencedtaskexecutor.h b/searchlib/src/vespa/searchlib/common/isequencedtaskexecutor.h
index a8b2a722c01..866f9f60423 100644
--- a/searchlib/src/vespa/searchlib/common/isequencedtaskexecutor.h
+++ b/searchlib/src/vespa/searchlib/common/isequencedtaskexecutor.h
@@ -25,6 +25,7 @@ public:
     private:
         uint32_t _id;
     };
+    ISequencedTaskExecutor(uint32_t numExecutors) : _numExecutors(numExecutors) { }
     virtual ~ISequencedTaskExecutor() { }
 
     /**
@@ -34,10 +35,12 @@ public:
      * @param componentId   component id
      * @return              executor id
      */
-    virtual ExecutorId getExecutorId(uint64_t componentId) = 0;
-    virtual uint32_t getNumExecutors() const = 0;
+    ExecutorId getExecutorId(uint64_t componentId) const {
+        return ExecutorId((componentId * 1099511628211ULL ) % _numExecutors);
+    }
+    uint32_t getNumExecutors() const { return _numExecutors; }
 
-    ExecutorId getExecutorId(vespalib::stringref componentId) {
+    ExecutorId getExecutorId(vespalib::stringref componentId) const {
         vespalib::hash<vespalib::stringref> hashfun;
         return getExecutorId(hashfun(componentId));
     }
@@ -96,7 +99,8 @@ public:
     void execute(ExecutorId id, FunctionType &&function) {
         executeTask(id, vespalib::makeLambdaTask(std::forward<FunctionType>(function)));
     }
-
+private:
+    uint32_t _numExecutors;
 };
 
 } // namespace search
diff --git a/searchlib/src/vespa/searchlib/common/sequencedtaskexecutor.cpp b/searchlib/src/vespa/searchlib/common/sequencedtaskexecutor.cpp
index 5306cabba8c..bb779b659ab 100644
--- a/searchlib/src/vespa/searchlib/common/sequencedtaskexecutor.cpp
+++ b/searchlib/src/vespa/searchlib/common/sequencedtaskexecutor.cpp
@@ -16,7 +16,8 @@ constexpr uint32_t stackSize = 128 * 1024;
 
 
 SequencedTaskExecutor::SequencedTaskExecutor(uint32_t threads, uint32_t taskLimit)
-    : _executors()
+    : ISequencedTaskExecutor(threads),
+      _executors()
 {
     for (uint32_t id = 0; id < threads; ++id) {
         auto executor = std::make_unique<BlockingThreadStackExecutor>(1, stackSize, taskLimit);
@@ -37,19 +38,6 @@ SequencedTaskExecutor::setTaskLimit(uint32_t taskLimit)
     }
 }
 
-ISequencedTaskExecutor::ExecutorId
-SequencedTaskExecutor::getExecutorId(uint64_t componentId)
-{
-    auto itr = _ids.find(componentId);
-    if (itr == _ids.end()) {
-        auto insarg = std::make_pair(componentId, ExecutorId(_ids.size() % _executors.size()));
-        auto insres = _ids.insert(insarg);
-        assert(insres.second);
-        itr = insres.first;
-    }
-    return itr->second;
-}
-
 void
 SequencedTaskExecutor::executeTask(ExecutorId id, vespalib::Executor::Task::UP task)
 {
@@ -59,7 +47,6 @@ SequencedTaskExecutor::executeTask(ExecutorId id, vespalib::Executor::Task::UP t
     assert(!rejectedTask);
 }
 
-
 void
 SequencedTaskExecutor::sync()
 {
diff --git a/searchlib/src/vespa/searchlib/common/sequencedtaskexecutor.h b/searchlib/src/vespa/searchlib/common/sequencedtaskexecutor.h
index 7551e82e489..2b7e70d69c7 100644
--- a/searchlib/src/vespa/searchlib/common/sequencedtaskexecutor.h
+++ b/searchlib/src/vespa/searchlib/common/sequencedtaskexecutor.h
@@ -20,7 +20,6 @@ class SequencedTaskExecutor : public ISequencedTaskExecutor
 {
     using Stats = vespalib::ExecutorStats;
     std::vector<std::shared_ptr<vespalib::BlockingThreadStackExecutor>> _executors;
-    vespalib::hash_map<size_t, ExecutorId> _ids;
 public:
     using ISequencedTaskExecutor::getExecutorId;
 
@@ -28,8 +27,6 @@ public:
     ~SequencedTaskExecutor();
 
     void setTaskLimit(uint32_t taskLimit);
-    uint32_t getNumExecutors() const override { return _executors.size(); }
-    ExecutorId getExecutorId(uint64_t componentId) override;
     void executeTask(ExecutorId id, vespalib::Executor::Task::UP task) override;
     void sync() override;
     Stats getStats();
diff --git a/searchlib/src/vespa/searchlib/common/sequencedtaskexecutorobserver.cpp b/searchlib/src/vespa/searchlib/common/sequencedtaskexecutorobserver.cpp
index b693c976ebe..04504086520 100644
--- a/searchlib/src/vespa/searchlib/common/sequencedtaskexecutorobserver.cpp
+++ b/searchlib/src/vespa/searchlib/common/sequencedtaskexecutorobserver.cpp
@@ -5,7 +5,8 @@
 namespace search {
 
 SequencedTaskExecutorObserver::SequencedTaskExecutorObserver(ISequencedTaskExecutor &executor)
-    : _executor(executor),
+    : ISequencedTaskExecutor(executor.getNumExecutors()),
+      _executor(executor),
       _executeCnt(0u),
       _syncCnt(0u),
       _executeHistory(),
@@ -15,12 +16,6 @@ SequencedTaskExecutorObserver::SequencedTaskExecutorObserver(ISequencedTaskExecu
 
 SequencedTaskExecutorObserver::~SequencedTaskExecutorObserver() = default;
 
-ISequencedTaskExecutor::ExecutorId
-SequencedTaskExecutorObserver::getExecutorId(uint64_t componentId)
-{
-    return _executor.getExecutorId(componentId);
-}
-
 void
 SequencedTaskExecutorObserver::executeTask(ExecutorId id, vespalib::Executor::Task::UP task)
 {
diff --git a/searchlib/src/vespa/searchlib/common/sequencedtaskexecutorobserver.h b/searchlib/src/vespa/searchlib/common/sequencedtaskexecutorobserver.h
index b4561148bca..b2de71f06b3 100644
--- a/searchlib/src/vespa/searchlib/common/sequencedtaskexecutorobserver.h
+++ b/searchlib/src/vespa/searchlib/common/sequencedtaskexecutorobserver.h
@@ -23,10 +23,8 @@ public:
     using ISequencedTaskExecutor::getExecutorId;
 
     SequencedTaskExecutorObserver(ISequencedTaskExecutor &executor);
-    virtual ~SequencedTaskExecutorObserver() override;
+    ~SequencedTaskExecutorObserver() override;
 
-    uint32_t getNumExecutors() const override { return _executor.getNumExecutors(); }
-    ExecutorId getExecutorId(uint64_t componentId) override;
     void executeTask(ExecutorId id, vespalib::Executor::Task::UP task) override;
     void sync() override;