Merge pull request #29680 from vespa-engine/balder/gc-last-remains-of-obsolete-clock

GC the last remain of obsolete Clock

19 files changed, 16 insertions, 288 deletions

diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_thread.h b/searchcore/src/vespa/searchcore/proton/matching/match_thread.h
index 7467e6c50a6..c6b233f2fcd 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/match_thread.h
+++ b/searchcore/src/vespa/searchcore/proton/matching/match_thread.h
@@ -88,7 +88,7 @@ private:
         LazyValue       _score_feature;
         double          _rankDropLimit;
         HitCollector   &_hits;
-        const Doom     &_doom;
+        const Doom      _doom;
     public:
         std::vector<uint32_t> dropped;
     };
diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.h b/searchcore/src/vespa/searchcore/proton/matching/match_tools.h
index 5dcd874f82b..759fe68eea2 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.h
+++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.h
@@ -41,7 +41,7 @@ private:
     using RankSetup = search::fef::RankSetup;
     using ExecutionProfiler = vespalib::ExecutionProfiler;
     QueryLimiter                    &_queryLimiter;
-    const vespalib::Doom            &_doom;
+    const vespalib::Doom             _doom;
     const Query                     &_query;
     MaybeMatchPhaseLimiter          &_match_limiter;
     const QueryEnvironment          &_queryEnv;
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
index 6682214927b..d47bed8936f 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
@@ -37,7 +37,6 @@
 #include <vespa/searchlib/engine/searchreply.h>
 #include <vespa/vespalib/util/destructor_callbacks.h>
 #include <vespa/vespalib/util/exceptions.h>
-#include <vespa/vespalib/util/clock.h>
 
 #include <vespa/log/log.h>
 #include <vespa/searchcorespi/index/warmupconfig.h>
@@ -219,7 +218,7 @@ DocumentDB::DocumentDB(const vespalib::string &baseDir,
       _feedHandler(std::make_unique<FeedHandler>(_writeService, tlsSpec, docTypeName, *this, _writeFilter, *this, tlsWriterFactory)),
       _subDBs(*this, *this, *_feedHandler, _docTypeName,
               _writeService, shared_service.shared(), fileHeaderContext, std::move(attribute_interlock),
-              metricsWireService, getMetrics(), queryLimiter, shared_service.clock().nowRef(),
+              metricsWireService, getMetrics(), queryLimiter, shared_service.nowRef(),
               _configMutex, _baseDir, hwInfo),
       _maintenanceController(shared_service.transport(), _writeService.master(), _refCount, _docTypeName),
       _jobTrackers(),
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.h b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.h
index 60a99a7d98a..5f32b9184a7 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.h
+++ b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.h
@@ -12,7 +12,6 @@
 #include <optional>
 
 namespace vespalib {
-    class Clock;
     class Executor;
     class HwInfo;
     class ThreadStackExecutorBase;
diff --git a/searchcore/src/vespa/searchcore/proton/server/i_shared_threading_service.h b/searchcore/src/vespa/searchcore/proton/server/i_shared_threading_service.h
index c09e7d51afc..d2d557878d6 100644
--- a/searchcore/src/vespa/searchcore/proton/server/i_shared_threading_service.h
+++ b/searchcore/src/vespa/searchcore/proton/server/i_shared_threading_service.h
@@ -1,6 +1,9 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
+#include <atomic>
+#include <vespa/vespalib/util/time.h>
+
 class FNET_Transport;
 
 namespace storage::spi { struct BucketExecutor; }
@@ -9,7 +12,6 @@ namespace vespalib {
 class ISequencedTaskExecutor;
 class ThreadExecutor;
 class InvokeService;
-class Clock;
 }
 
 namespace proton {
@@ -56,7 +58,7 @@ public:
     /**
      * Return a very cheap clock.
      */
-    virtual const vespalib::Clock & clock() const = 0;
+    virtual const std::atomic<vespalib::steady_time> & nowRef() const = 0;
 };
 
 }
diff --git a/searchcore/src/vespa/searchcore/proton/server/searchable_doc_subdb_configurer.h b/searchcore/src/vespa/searchcore/proton/server/searchable_doc_subdb_configurer.h
index 740f312bfb5..b5c87b8dc9b 100644
--- a/searchcore/src/vespa/searchcore/proton/server/searchable_doc_subdb_configurer.h
+++ b/searchcore/src/vespa/searchcore/proton/server/searchable_doc_subdb_configurer.h
@@ -17,7 +17,6 @@ namespace searchcorespi { class IndexSearchable; }
 namespace proton::matching { class QueryLimiter; }
 
 namespace vespalib::eval { struct ConstantValueFactory; }
-namespace vespalib { class Clock; }
 
 namespace proton {
 
diff --git a/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.cpp b/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.cpp
index 45b5ec2ef48..de55213f7ea 100644
--- a/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.cpp
@@ -26,8 +26,7 @@ SharedThreadingService::SharedThreadingService(const SharedThreadingServiceConfi
       _invokeService(std::make_unique<vespalib::InvokeServiceImpl>(std::max(vespalib::adjustTimeoutByDetectedHz(1ms),
                                                                             cfg.field_writer_config().reactionTime()))),
       _invokeRegistrations(),
-      _bucket_executor(bucket_executor),
-      _clock(dynamic_cast<const vespalib::InvokeServiceImpl &>(*_invokeService).nowRef())
+      _bucket_executor(bucket_executor)
 {
     const auto& fw_cfg = cfg.field_writer_config();
     _field_writer = vespalib::SequencedTaskExecutor::create(vespalib::be_nice(CpuUsage::wrap(proton_field_writer_executor, CpuUsage::Category::WRITE), cfg.feeding_niceness()),
@@ -53,4 +52,9 @@ SharedThreadingService::sync_all_executors() {
     }
 }
 
+const std::atomic<vespalib::steady_time> &
+SharedThreadingService::nowRef() const {
+    return dynamic_cast<const vespalib::InvokeServiceImpl &>(*_invokeService).nowRef();
+}
+
 }
diff --git a/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.h b/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.h
index c2fffcc4a6c..a436c3a8006 100644
--- a/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.h
+++ b/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.h
@@ -5,7 +5,6 @@
 #include "shared_threading_service_config.h"
 #include <vespa/vespalib/util/threadexecutor.h>
 #include <vespa/vespalib/util/syncable.h>
-#include <vespa/vespalib/util/clock.h>
 #include <memory>
 
 namespace vespalib {
@@ -25,7 +24,6 @@ private:
     std::unique_ptr<vespalib::InvokeService>          _invokeService;
     std::vector<Registration>                         _invokeRegistrations;
     storage::spi::BucketExecutor&                     _bucket_executor;
-    vespalib::Clock                                   _clock;
 public:
     SharedThreadingService(const SharedThreadingServiceConfig& cfg,
                            FNET_Transport& transport,
@@ -40,7 +38,7 @@ public:
     vespalib::InvokeService & invokeService() override { return *_invokeService; }
     FNET_Transport & transport() override { return _transport; }
     storage::spi::BucketExecutor& bucket_executor() override { return _bucket_executor; }
-    const vespalib::Clock & clock() const override { return _clock; }
+    const std::atomic<vespalib::steady_time> & nowRef() const override;
 };
 
 }
diff --git a/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.cpp b/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.cpp
index 95e87247366..f635a406d6c 100644
--- a/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.cpp
+++ b/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.cpp
@@ -12,8 +12,7 @@ MockSharedThreadingService::MockSharedThreadingService(ThreadExecutor& shared_in
       _field_writer(vespalib::SequencedTaskExecutor::create(mock_field_writer_executor, 1)),
       _invokeService(10ms),
       _transport(),
-      _bucket_executor(num_bucket_executors),
-      _clock(_invokeService.nowRef())
+      _bucket_executor(num_bucket_executors)
 {
 }
 
diff --git a/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.h b/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.h
index b48d2e0df79..622b2b5fed3 100644
--- a/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.h
+++ b/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.h
@@ -4,7 +4,6 @@
 #include "transport_helper.h"
 #include <vespa/persistence/dummyimpl/dummy_bucket_executor.h>
 #include <vespa/searchcore/proton/server/i_shared_threading_service.h>
-#include <vespa/vespalib/util/clock.h>
 #include <vespa/vespalib/util/invokeserviceimpl.h>
 
 namespace proton {
@@ -17,7 +16,6 @@ private:
     vespalib::InvokeServiceImpl   _invokeService;
     Transport                     _transport;
     storage::spi::dummy::DummyBucketExecutor _bucket_executor;
-    vespalib::Clock               _clock;
 public:
     MockSharedThreadingService(ThreadExecutor& shared_in, size_t num_bucket_executors = 2);
     ~MockSharedThreadingService() override;
@@ -26,7 +24,7 @@ public:
     vespalib::InvokeService & invokeService() override { return _invokeService; }
     FNET_Transport & transport() override { return _transport.transport(); }
     storage::spi::BucketExecutor& bucket_executor() override { return _bucket_executor; }
-    const vespalib::Clock & clock() const override { return _clock; }
+    const std::atomic<vespalib::steady_time> & nowRef() const override { return _invokeService.nowRef(); }
 };
 
 }
diff --git a/searchcore/src/vespa/searchcorespi/index/ithreadingservice.h b/searchcore/src/vespa/searchcorespi/index/ithreadingservice.h
index 52c586a6bb9..444a5cde1a0 100644
--- a/searchcore/src/vespa/searchcorespi/index/ithreadingservice.h
+++ b/searchcore/src/vespa/searchcorespi/index/ithreadingservice.h
@@ -7,7 +7,6 @@ class FNET_Transport;
 
 namespace vespalib {
     class ISequencedTaskExecutor;
-    class Clock;
 }
 namespace searchcorespi::index {
 
diff --git a/vespalib/CMakeLists.txt b/vespalib/CMakeLists.txt
index 83711ab8200..49a4cca1e1c 100644
--- a/vespalib/CMakeLists.txt
+++ b/vespalib/CMakeLists.txt
@@ -42,7 +42,6 @@ vespa_define_module(
     src/tests/btree/btree-scan-speed
     src/tests/btree/btree-stress
     src/tests/btree/btree_store
-    src/tests/clock
     src/tests/component
     src/tests/compress
     src/tests/compression
diff --git a/vespalib/src/tests/clock/.gitignore b/vespalib/src/tests/clock/.gitignore
deleted file mode 100644
index 96861fcc5d3..00000000000
--- a/vespalib/src/tests/clock/.gitignore
+++ /dev/null
@@ -1,5 +0,0 @@
-.depend
-Makefile
-clock_test
-vespalib_clock_test_app
-vespalib_clock_benchmark_app
diff --git a/vespalib/src/tests/clock/CMakeLists.txt b/vespalib/src/tests/clock/CMakeLists.txt
deleted file mode 100644
index 55c4ca55299..00000000000
--- a/vespalib/src/tests/clock/CMakeLists.txt
+++ /dev/null
@@ -1,14 +0,0 @@
-# Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-vespa_add_executable(vespalib_clock_benchmark_app TEST
-    SOURCES
-    clock_benchmark.cpp
-    DEPENDS
-    vespalib
-)
-vespa_add_executable(vespalib_clock_test_app TEST
-    SOURCES
-    clock_test.cpp
-    DEPENDS
-    vespalib
-)
-vespa_add_test(NAME vespalib_clock_test_app COMMAND vespalib_clock_test_app)
diff --git a/vespalib/src/tests/clock/clock_benchmark.cpp b/vespalib/src/tests/clock/clock_benchmark.cpp
deleted file mode 100644
index 81a228f820f..00000000000
--- a/vespalib/src/tests/clock/clock_benchmark.cpp
+++ /dev/null
@@ -1,170 +0,0 @@
-// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#include <vespa/vespalib/util/clock.h>
-#include <vespa/vespalib/util/invokeserviceimpl.h>
-#include <cassert>
-#include <vector>
-#include <atomic>
-#include <cinttypes>
-#include <cstring>
-#include <condition_variable>
-#include <mutex>
-#include <thread>
-
-using vespalib::Clock;
-using vespalib::steady_time;
-using vespalib::steady_clock;
-using vespalib::duration;
-using vespalib::to_s;
-
-struct UpdateClock {
-    virtual ~UpdateClock() {}
-    virtual void update() = 0;
-};
-
-struct NSValue : public UpdateClock {
-    void update() override { _value = std::chrono::steady_clock::now().time_since_epoch().count(); }
-    int64_t _value;
-};
-
-struct NSVolatile : public UpdateClock {
-    void update() override { _value = std::chrono::steady_clock::now().time_since_epoch().count(); }
-    volatile int64_t _value;
-};
-struct NSAtomic : public UpdateClock {
-    void update() override { _value.store(std::chrono::steady_clock::now().time_since_epoch().count()); }
-    std::atomic<int64_t> _value;
-};
-
-class TestClock
-{
-private:
-    int                       _timePeriodMS;
-    std::mutex                _lock;
-    std::condition_variable   _cond;
-    UpdateClock              &_clock;
-    bool                      _stop;
-    std::thread               _thread;
-
-    void run();
-
-public:
-    TestClock(UpdateClock & clock, double timePeriod)
-        : _timePeriodMS(static_cast<uint32_t>(timePeriod*1000)),
-          _lock(),
-          _cond(),
-          _clock(clock),
-          _stop(false),
-          _thread()
-    {
-        _thread = std::thread([this](){run();});
-    }
-    ~TestClock() {
-        {
-            std::lock_guard<std::mutex> guard(_lock);
-            _stop = true;
-            _cond.notify_all();
-        }
-        _thread.join();
-    }
-};
-
-void TestClock::run()
-{
-    std::unique_lock<std::mutex> guard(_lock);
-    while (!_stop) {
-        _clock.update();
-        _cond.wait_for(guard, std::chrono::milliseconds(_timePeriodMS));
-    }
-}
-
-template<typename Func>
-struct Sampler {
-    Sampler(Func func, uint64_t samples)
-      : _samples(samples),
-        _count(),
-        _func(func),
-        _thread()
-    {
-        memset(_count, 0, sizeof(_count));
-        _thread = std::thread([this](){run();});
-    }
-    void run() {
-        steady_time prev = _func();
-        for (uint64_t samples = 0; samples < _samples; ++samples) {
-            steady_time now = _func();
-            duration diff = now - prev;
-            if (diff > duration::zero()) prev = now;
-            _count[1 + ((diff == duration::zero()) ? 0 : (diff > duration::zero()) ? 1 : -1)]++;
-        }
-    }
-    uint64_t    _samples;
-    uint64_t    _count[3];
-    Func        _func;
-    std::thread _thread;
-};
-
-template<typename Func>
-void benchmark(const char * desc, uint64_t samples, uint32_t numThreads, Func func) {
-    std::vector<std::unique_ptr<Sampler<Func>>> threads;
-    threads.reserve(numThreads);
-    steady_time start = steady_clock::now();
-    for (uint32_t i(0); i < numThreads; i++) {
-        threads.push_back(std::make_unique<Sampler<Func>>(func, samples));
-    }
-    uint64_t count[3];
-    memset(count, 0, sizeof(count));
-    for (const auto & sampler : threads) {
-        sampler->_thread.join();
-        for (uint32_t i(0); i < 3; i++) {
-            count[i] += sampler->_count[i];
-        }
-    }
-    printf("%s: Took %" PRId64 " clock samples in %2.3f with [%" PRId64 ", %" PRId64 ", %" PRId64 "] counts\n", desc, samples, to_s(steady_clock::now() - start), count[0], count[1], count[2]);
-}
-
-int
-main(int argc, char *argv[])
-{
-    if (argc != 4) {
-        fprintf(stderr, "usage: %s <frequency> <numThreads> <samples>\n", argv[0]);
-        return 1;
-    }
-    uint64_t frequency = atoll(argv[1]);
-    uint32_t numThreads = atoi(argv[2]);
-    uint64_t samples = atoll(argv[3]);
-    NSValue nsValue;
-    NSVolatile nsVolatile;
-    NSAtomic nsAtomic;
-    vespalib::InvokeServiceImpl invoker(vespalib::from_s(1.0/frequency));
-    Clock clock(invoker.nowRef());
-    TestClock nsClock(nsValue, 1.0/frequency);
-    TestClock nsVolatileClock(nsVolatile, 1.0/frequency);
-    TestClock nsAtomicClock(nsAtomic, 1.0/frequency);
-
-    benchmark("vespalib::Clock", samples, numThreads, [&clock]() {
-        return clock.getTimeNS();
-    });
-    benchmark("uint64_t", samples, numThreads, [&nsValue]() {
-        return steady_time (duration(nsValue._value));
-    });
-    benchmark("volatile uint64_t", samples, numThreads, [&nsVolatile]() {
-        return steady_time(duration(nsVolatile._value));
-    });
-    benchmark("memory_order_relaxed", samples, numThreads, [&nsAtomic]() {
-        return steady_time(duration(nsAtomic._value.load(std::memory_order_relaxed)));
-    });
-    benchmark("memory_order_consume", samples, numThreads, [&nsAtomic]() {
-        return steady_time(duration(nsAtomic._value.load(std::memory_order_consume)));
-    });
-    benchmark("memory_order_acquire", samples, numThreads, [&nsAtomic]() {
-        return steady_time(duration(nsAtomic._value.load(std::memory_order_acquire)));
-    });
-    benchmark("memory_order_seq_cst", samples, numThreads, [&nsAtomic]() {
-        return steady_time(duration(nsAtomic._value.load(std::memory_order_seq_cst)));
-    });
-    benchmark("vespalib::steady_time::now()", samples, numThreads, []() {
-        return steady_clock::now();
-    });
-    return 0;
-}
diff --git a/vespalib/src/tests/clock/clock_test.cpp b/vespalib/src/tests/clock/clock_test.cpp
deleted file mode 100644
index 667035bdbde..00000000000
--- a/vespalib/src/tests/clock/clock_test.cpp
+++ /dev/null
@@ -1,29 +0,0 @@
-// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#include <vespa/vespalib/testkit/testapp.h>
-#include <vespa/vespalib/util/clock.h>
-#include <vespa/vespalib/util/invokeserviceimpl.h>
-#include <thread>
-
-using vespalib::Clock;
-using vespalib::duration;
-using vespalib::steady_time;
-using vespalib::steady_clock;
-
-void waitForMovement(steady_time start, Clock & clock, vespalib::duration timeout) {
-    steady_time startOsClock = steady_clock::now();
-    while ((clock.getTimeNS() <= start) && ((steady_clock::now() - startOsClock) < timeout)) {
-        std::this_thread::sleep_for(1ms);
-    }
-}
-
-TEST("Test that clock is ticking forward") {
-    vespalib::InvokeServiceImpl invoker(50ms);
-    Clock clock(invoker.nowRef());
-    steady_time start = clock.getTimeNS();
-    waitForMovement(start, clock, 10s);
-    steady_time stop = clock.getTimeNS();
-    EXPECT_TRUE(stop > start);
-}
-
-TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/vespalib/src/vespa/vespalib/util/CMakeLists.txt b/vespalib/src/vespa/vespalib/util/CMakeLists.txt
index e69152a443c..518cbe337fa 100644
--- a/vespalib/src/vespa/vespalib/util/CMakeLists.txt
+++ b/vespalib/src/vespa/vespalib/util/CMakeLists.txt
@@ -17,7 +17,6 @@ vespa_add_library(vespalib_vespalib_util OBJECT
     box.cpp
     cgroup_resource_limits.cpp
     classname.cpp
-    clock.cpp
     compress.cpp
     compressor.cpp
     count_down_latch.cpp
diff --git a/vespalib/src/vespa/vespalib/util/clock.cpp b/vespalib/src/vespa/vespalib/util/clock.cpp
deleted file mode 100644
index 7bfefa9906f..00000000000
--- a/vespalib/src/vespa/vespalib/util/clock.cpp
+++ /dev/null
@@ -1,15 +0,0 @@
-// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#include "clock.h"
-#include <cassert>
-namespace vespalib {
-
-Clock::Clock(const std::atomic<steady_time> & source) noexcept
-    : _timeNS(source)
-{
-    static_assert(std::atomic<steady_time>::is_always_lock_free);
-}
-
-Clock::~Clock() = default;
-
-}
diff --git a/vespalib/src/vespa/vespalib/util/clock.h b/vespalib/src/vespa/vespalib/util/clock.h
deleted file mode 100644
index ab4671f7bfb..00000000000
--- a/vespalib/src/vespa/vespalib/util/clock.h
+++ /dev/null
@@ -1,34 +0,0 @@
-// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-#pragma once
-
-#include <vespa/vespalib/util/time.h>
-#include <atomic>
-#include <memory>
-
-namespace vespalib {
-
-/**
- * TODO Remove abstraction. Can use atomic ref directly
- * It is intended used where you want to check the time with low cost, but where
- * resolution is not that important.
- */
-
-class Clock
-{
-private:
-    const std::atomic<steady_time> &_timeNS;
-public:
-    Clock(const std::atomic<steady_time> & source) noexcept;
-    Clock(const Clock &) = delete;
-    Clock & operator =(const Clock &) = delete;
-    Clock(Clock &&) = delete;
-    Clock & operator =(Clock &&) = delete;
-    ~Clock();
-
-    vespalib::steady_time getTimeNS() const noexcept {
-        return vespalib::steady_time(_timeNS.load(std::memory_order_relaxed));
-    }
-    const std::atomic<steady_time> & nowRef() const { return _timeNS; }
-};
-
-}