From 4d96730428ba8655601f189e00d0348b8454e7e1 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 15 Dec 2023 19:06:51 +0000 Subject: GC the last remain of obsolete Clock --- .../searchcore/proton/matching/match_thread.h | 2 +- .../vespa/searchcore/proton/matching/match_tools.h | 2 +- .../vespa/searchcore/proton/server/documentdb.cpp | 3 +- .../proton/server/documentsubdbcollection.h | 1 - .../proton/server/i_shared_threading_service.h | 6 +- .../server/searchable_doc_subdb_configurer.h | 1 - .../proton/server/shared_threading_service.cpp | 8 +- .../proton/server/shared_threading_service.h | 4 +- .../proton/test/mock_shared_threading_service.cpp | 3 +- .../proton/test/mock_shared_threading_service.h | 4 +- .../vespa/searchcorespi/index/ithreadingservice.h | 1 - vespalib/CMakeLists.txt | 1 - vespalib/src/tests/clock/.gitignore | 5 - vespalib/src/tests/clock/CMakeLists.txt | 14 -- vespalib/src/tests/clock/clock_benchmark.cpp | 170 --------------------- vespalib/src/tests/clock/clock_test.cpp | 29 ---- vespalib/src/vespa/vespalib/util/CMakeLists.txt | 1 - vespalib/src/vespa/vespalib/util/clock.cpp | 15 -- vespalib/src/vespa/vespalib/util/clock.h | 34 ----- 19 files changed, 16 insertions(+), 288 deletions(-) delete mode 100644 vespalib/src/tests/clock/.gitignore delete mode 100644 vespalib/src/tests/clock/CMakeLists.txt delete mode 100644 vespalib/src/tests/clock/clock_benchmark.cpp delete mode 100644 vespalib/src/tests/clock/clock_test.cpp delete mode 100644 vespalib/src/vespa/vespalib/util/clock.cpp delete mode 100644 vespalib/src/vespa/vespalib/util/clock.h 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 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 #include #include -#include #include #include @@ -219,7 +218,7 @@ DocumentDB::DocumentDB(const vespalib::string &baseDir, _feedHandler(std::make_unique(_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 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 +#include + 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 & 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(std::max(vespalib::adjustTimeoutByDetectedHz(1ms), cfg.field_writer_config().reactionTime()))), _invokeRegistrations(), - _bucket_executor(bucket_executor), - _clock(dynamic_cast(*_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 & +SharedThreadingService::nowRef() const { + return dynamic_cast(*_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 #include -#include #include namespace vespalib { @@ -25,7 +24,6 @@ private: std::unique_ptr _invokeService; std::vector _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 & 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 #include -#include #include 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 & 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 -#include -#include -#include -#include -#include -#include -#include -#include -#include - -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 _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(timePeriod*1000)), - _lock(), - _cond(), - _clock(clock), - _stop(false), - _thread() - { - _thread = std::thread([this](){run();}); - } - ~TestClock() { - { - std::lock_guard guard(_lock); - _stop = true; - _cond.notify_all(); - } - _thread.join(); - } -}; - -void TestClock::run() -{ - std::unique_lock guard(_lock); - while (!_stop) { - _clock.update(); - _cond.wait_for(guard, std::chrono::milliseconds(_timePeriodMS)); - } -} - -template -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 -void benchmark(const char * desc, uint64_t samples, uint32_t numThreads, Func func) { - std::vector>> threads; - threads.reserve(numThreads); - steady_time start = steady_clock::now(); - for (uint32_t i(0); i < numThreads; i++) { - threads.push_back(std::make_unique>(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 \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 -#include -#include -#include - -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 -namespace vespalib { - -Clock::Clock(const std::atomic & source) noexcept - : _timeNS(source) -{ - static_assert(std::atomic::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 -#include -#include - -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 &_timeNS; -public: - Clock(const std::atomic & 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 & nowRef() const { return _timeNS; } -}; - -} -- cgit v1.2.3