diff options
25 files changed, 47 insertions, 58 deletions
diff --git a/metrics/src/vespa/metrics/valuemetric.h b/metrics/src/vespa/metrics/valuemetric.h index 3470aa067e0..9d5f0a82735 100644 --- a/metrics/src/vespa/metrics/valuemetric.h +++ b/metrics/src/vespa/metrics/valuemetric.h @@ -37,7 +37,6 @@ protected: void logWarning(const char* msg, const char *op) const; void logNonFiniteValueWarning() const; - void sendLogEvent(Metric::String name, double value) const; }; template<typename AvgVal, typename TotVal, bool SumOnAdd> @@ -86,7 +85,7 @@ public: ~ValueMetric(); MetricValueClass::UP getValues() const override { - return MetricValueClass::UP(new Values(_values.getValues())); + return std::make_unique<Values>(_values.getValues()); } void unsetOnZeroValue() { _values.setFlag(UNSET_ON_ZERO_VALUE); } diff --git a/metrics/src/vespa/metrics/valuemetric.hpp b/metrics/src/vespa/metrics/valuemetric.hpp index a9522dcec29..5e0ef95e9e5 100644 --- a/metrics/src/vespa/metrics/valuemetric.hpp +++ b/metrics/src/vespa/metrics/valuemetric.hpp @@ -26,7 +26,7 @@ ValueMetric<AvgVal, TotVal, SumOnAdd>::ValueMetric( {} template<typename AvgVal, typename TotVal, bool SumOnAdd> -ValueMetric<AvgVal, TotVal, SumOnAdd>::~ValueMetric() { } +ValueMetric<AvgVal, TotVal, SumOnAdd>::~ValueMetric() = default; template<typename AvgVal, typename TotVal, bool SumOnAdd> void ValueMetric<AvgVal, TotVal, SumOnAdd>::inc(AvgVal incVal) @@ -239,8 +239,7 @@ ValueMetric<AvgVal, TotVal, SumOnAdd>::getDoubleValue(stringref id) const template<typename AvgVal, typename TotVal, bool SumOnAdd> void -ValueMetric<AvgVal, TotVal, SumOnAdd>::addMemoryUsage( - MemoryConsumption& mc) const +ValueMetric<AvgVal, TotVal, SumOnAdd>::addMemoryUsage(MemoryConsumption& mc) const { ++mc._valueMetricCount; mc._valueMetricValues += _values.getMemoryUsageAllocatedInternally(); diff --git a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp index 71ae26180ad..09c782cef07 100644 --- a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp +++ b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp @@ -15,7 +15,6 @@ #include <vespa/searchcore/proton/feedoperation/removeoperation.h> #include <vespa/searchcore/proton/server/blockable_maintenance_job.h> #include <vespa/searchcore/proton/server/executor_thread_service.h> -#include <vespa/searchcore/proton/server/i_document_scan_iterator.h> #include <vespa/searchcore/proton/server/i_operation_storer.h> #include <vespa/searchcore/proton/server/ibucketmodifiedhandler.h> #include <vespa/searchcore/proton/server/idocumentmovehandler.h> @@ -728,7 +727,7 @@ MyExecutor::isIdle() { (void) getStats(); sync(); - Stats stats(getStats()); + auto stats = getStats(); return stats.acceptedTasks == 0u; } diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.h b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.h index 068d7bd033c..4eaa722e0ba 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.h @@ -114,7 +114,7 @@ public: * * @return executor stats **/ - vespalib::ThreadStackExecutor::Stats getExecutorStats() { return _executor.getStats(); } + vespalib::ExecutorStats getExecutorStats() { return _executor.getStats(); } /** * Returns the underlying executor. Only used for state explorers. diff --git a/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.h b/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.h index 9364fc7b097..74a39a3ec78 100644 --- a/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.h +++ b/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.h @@ -62,7 +62,7 @@ public: * * @return executor stats **/ - vespalib::ThreadStackExecutor::Stats getExecutorStats() { return _executor.getStats(); } + vespalib::ExecutorStats getExecutorStats() { return _executor.getStats(); } /** * Returns the underlying executor. Only used for state explorers. diff --git a/searchcore/src/vespa/searchcore/proton/metrics/executor_metrics.cpp b/searchcore/src/vespa/searchcore/proton/metrics/executor_metrics.cpp index fa825024878..74e0971178c 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/executor_metrics.cpp +++ b/searchcore/src/vespa/searchcore/proton/metrics/executor_metrics.cpp @@ -5,7 +5,7 @@ namespace proton { void -ExecutorMetrics::update(const vespalib::ThreadStackExecutorBase::Stats &stats) +ExecutorMetrics::update(const vespalib::ExecutorStats &stats) { maxPending.set(stats.queueSize.max()); accepted.inc(stats.acceptedTasks); diff --git a/searchcore/src/vespa/searchcore/proton/metrics/executor_metrics.h b/searchcore/src/vespa/searchcore/proton/metrics/executor_metrics.h index 5f7dfdf45b0..273c4ed8979 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/executor_metrics.h +++ b/searchcore/src/vespa/searchcore/proton/metrics/executor_metrics.h @@ -5,7 +5,7 @@ #include <vespa/metrics/metricset.h> #include <vespa/metrics/countmetric.h> #include <vespa/metrics/valuemetric.h> -#include <vespa/vespalib/util/threadstackexecutorbase.h> +#include <vespa/vespalib/util/executor_stats.h> namespace proton { @@ -16,7 +16,7 @@ struct ExecutorMetrics : metrics::MetricSet metrics::LongCountMetric rejected; metrics::LongAverageMetric queueSize; - void update(const vespalib::ThreadStackExecutorBase::Stats &stats); + void update(const vespalib::ExecutorStats &stats); ExecutorMetrics(const std::string &name, metrics::MetricSet *parent); ~ExecutorMetrics(); }; diff --git a/searchcore/src/vespa/searchcore/proton/metrics/executor_threading_service_stats.h b/searchcore/src/vespa/searchcore/proton/metrics/executor_threading_service_stats.h index 0a113207f65..e2c53af11b5 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/executor_threading_service_stats.h +++ b/searchcore/src/vespa/searchcore/proton/metrics/executor_threading_service_stats.h @@ -11,10 +11,8 @@ namespace proton { * document db. */ class ExecutorThreadingServiceStats { -public: - using Stats = vespalib::ExecutorStats; - private: + using Stats = vespalib::ExecutorStats; Stats _masterExecutorStats; Stats _indexExecutorStats; Stats _summaryExecutorStats; diff --git a/searchcore/src/vespa/searchcore/proton/server/executor_thread_service.cpp b/searchcore/src/vespa/searchcore/proton/server/executor_thread_service.cpp index 3b4f12b7c85..684132b34e7 100644 --- a/searchcore/src/vespa/searchcore/proton/server/executor_thread_service.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/executor_thread_service.cpp @@ -73,7 +73,7 @@ ExecutorThreadService::isCurrentThread() const return FastOS_Thread::CompareThreadIds(_threadId->_id, currentThreadId); } -vespalib::ThreadExecutor::Stats ExecutorThreadService::getStats() { +vespalib::ExecutorStats ExecutorThreadService::getStats() { return _executor.getStats(); } diff --git a/searchcore/src/vespa/searchcore/proton/server/executor_thread_service.h b/searchcore/src/vespa/searchcore/proton/server/executor_thread_service.h index 8adb80889e7..44a330ca696 100644 --- a/searchcore/src/vespa/searchcore/proton/server/executor_thread_service.h +++ b/searchcore/src/vespa/searchcore/proton/server/executor_thread_service.h @@ -21,7 +21,7 @@ public: ExecutorThreadService(vespalib::SyncableThreadExecutor &executor); ~ExecutorThreadService(); - Stats getStats() override; + vespalib::ExecutorStats getStats() override; vespalib::Executor::Task::UP execute(vespalib::Executor::Task::UP task) override { return _executor.execute(std::move(task)); diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp index 17c255d506a..edf68633124 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp @@ -737,8 +737,7 @@ Proton::prepareRestart() namespace { void -updateExecutorMetrics(ExecutorMetrics &metrics, - const vespalib::ThreadStackExecutor::Stats &stats) +updateExecutorMetrics(ExecutorMetrics &metrics, const vespalib::ExecutorStats &stats) { metrics.update(stats); } diff --git a/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.h b/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.h index a439306b69b..34eebdc839d 100644 --- a/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.h +++ b/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.h @@ -66,7 +66,7 @@ public: * * @return executor stats **/ - vespalib::ThreadStackExecutor::Stats getExecutorStats() { return _executor.getStats(); } + vespalib::ExecutorStats getExecutorStats() { return _executor.getStats(); } /** * Returns the underlying executor. Only used for state explorers. diff --git a/searchcore/src/vespa/searchcore/proton/test/thread_service_observer.h b/searchcore/src/vespa/searchcore/proton/test/thread_service_observer.h index f2e1e64eeb3..26a92841999 100644 --- a/searchcore/src/vespa/searchcore/proton/test/thread_service_observer.h +++ b/searchcore/src/vespa/searchcore/proton/test/thread_service_observer.h @@ -40,7 +40,7 @@ public: } size_t getNumThreads() const override { return _service.getNumThreads(); } - Stats getStats() override { + vespalib::ExecutorStats getStats() override { return _service.getStats(); } diff --git a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp index e4b64b19739..869ff0456e1 100644 --- a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp @@ -324,12 +324,12 @@ AdaptiveSequencedExecutor::setTaskLimit(uint32_t task_limit) } } -AdaptiveSequencedExecutor::Stats +ExecutorStats AdaptiveSequencedExecutor::getStats() { auto guard = std::lock_guard(_mutex); - Stats stats = _stats; - _stats = Stats(); + ExecutorStats stats = _stats; + _stats = ExecutorStats(); _stats.queueSize.add(_self.pending_tasks); return stats; } diff --git a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h index 4e0388caf8a..fdcdf35fbbb 100644 --- a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h +++ b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h @@ -22,8 +22,7 @@ namespace vespalib { class AdaptiveSequencedExecutor : public ISequencedTaskExecutor { private: - using Stats = vespalib::ExecutorStats; - using Task = vespalib::Executor::Task; + using Task = Executor::Task; struct TaggedTask { Task::UP task; @@ -128,7 +127,7 @@ private: vespalib::ArrayQueue<Worker*> _worker_stack; EventBarrier<BarrierCompletion> _barrier; Self _self; - Stats _stats; + ExecutorStats _stats; Config _cfg; void maybe_block_self(std::unique_lock<std::mutex> &lock); @@ -147,7 +146,7 @@ public: void executeTask(ExecutorId id, Task::UP task) override; void sync() override; void setTaskLimit(uint32_t task_limit) override; - vespalib::ExecutorStats getStats() override; + ExecutorStats getStats() override; Config get_config() const; }; diff --git a/staging_vespalib/src/vespa/vespalib/util/foreground_thread_executor.h b/staging_vespalib/src/vespa/vespalib/util/foreground_thread_executor.h index f3dbd512047..3bd5ca3d49a 100644 --- a/staging_vespalib/src/vespa/vespalib/util/foreground_thread_executor.h +++ b/staging_vespalib/src/vespa/vespalib/util/foreground_thread_executor.h @@ -22,7 +22,7 @@ public: return Task::UP(); } size_t getNumThreads() const override { return 0; } - Stats getStats() override { + ExecutorStats getStats() override { return ExecutorStats(ExecutorStats::QueueSizeT(), _accepted.load(std::memory_order_relaxed), 0); } void setTaskLimit(uint32_t taskLimit) override { (void) taskLimit; } diff --git a/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.cpp b/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.cpp index adb4356c4c8..727894397a7 100644 --- a/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.cpp @@ -99,10 +99,10 @@ SequencedTaskExecutor::wakeup() { } } -SequencedTaskExecutor::Stats +ExecutorStats SequencedTaskExecutor::getStats() { - Stats accumulatedStats; + ExecutorStats accumulatedStats; for (auto &executor :* _executors) { accumulatedStats += executor->getStats(); } diff --git a/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.h b/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.h index 38844535021..7b49f7aac75 100644 --- a/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.h +++ b/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.h @@ -16,7 +16,6 @@ class SyncableThreadExecutor; class SequencedTaskExecutor final : public ISequencedTaskExecutor { public: - using Stats = vespalib::ExecutorStats; using ISequencedTaskExecutor::getExecutorId; using OptimizeFor = vespalib::Executor::OptimizeFor; @@ -26,7 +25,7 @@ public: void executeTask(ExecutorId id, vespalib::Executor::Task::UP task) override; ExecutorId getExecutorId(uint64_t componentId) const override; void sync() override; - Stats getStats() override; + ExecutorStats getStats() override; void wakeup() override; /* diff --git a/staging_vespalib/src/vespa/vespalib/util/singleexecutor.cpp b/staging_vespalib/src/vespa/vespalib/util/singleexecutor.cpp index 7fba68e2092..803ec4f3f7c 100644 --- a/staging_vespalib/src/vespa/vespalib/util/singleexecutor.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/singleexecutor.cpp @@ -158,13 +158,13 @@ SingleExecutor::wait_for_room(Lock & lock) { } } -ThreadExecutor::Stats +ExecutorStats SingleExecutor::getStats() { Lock lock(_mutex); uint64_t accepted = _wp.load(std::memory_order_relaxed); - Stats stats(_queueSize, (accepted - _lastAccepted), 0); + ExecutorStats stats(_queueSize, (accepted - _lastAccepted), 0); _lastAccepted = accepted; - _queueSize = Stats::QueueSizeT() ; + _queueSize = ExecutorStats::QueueSizeT() ; return stats; } diff --git a/staging_vespalib/src/vespa/vespalib/util/singleexecutor.h b/staging_vespalib/src/vespa/vespalib/util/singleexecutor.h index 20e5a560e52..8e9c1ae3fa1 100644 --- a/staging_vespalib/src/vespa/vespalib/util/singleexecutor.h +++ b/staging_vespalib/src/vespa/vespalib/util/singleexecutor.h @@ -29,7 +29,7 @@ public: uint32_t getTaskLimit() const override { return _taskLimit.load(std::memory_order_relaxed); } uint32_t get_watermark() const { return _watermark; } duration get_reaction_time() const { return _reactionTime; } - Stats getStats() override; + ExecutorStats getStats() override; SingleExecutor & shutdown() override; private: using Lock = std::unique_lock<std::mutex>; @@ -55,7 +55,7 @@ private: std::condition_variable _producerCondition; vespalib::Thread _thread; uint64_t _lastAccepted; - Stats::QueueSizeT _queueSize; + ExecutorStats::QueueSizeT _queueSize; std::atomic<uint64_t> _wakeupConsumerAt; std::atomic<uint64_t> _producerNeedWakeupAt; std::atomic<uint64_t> _wp; diff --git a/vespalib/src/tests/executor/threadstackexecutor_test.cpp b/vespalib/src/tests/executor/threadstackexecutor_test.cpp index 63c6856afd2..b55f54f9339 100644 --- a/vespalib/src/tests/executor/threadstackexecutor_test.cpp +++ b/vespalib/src/tests/executor/threadstackexecutor_test.cpp @@ -33,17 +33,18 @@ std::atomic<uint32_t> MyTask::runCnt(0); std::atomic<uint32_t> MyTask::deleteCnt(0); struct MyState { + static constexpr uint32_t NUM_THREADS = 10; Gate gate; // to block workers CountDownLatch latch; // to wait for workers ThreadStackExecutor executor; bool checked; - MyState() : gate(), latch(10), executor(10, 128000, 20), checked(false) + MyState() : gate(), latch(10), executor(NUM_THREADS, 128000, 20), checked(false) { MyTask::resetStats(); } MyState &execute(uint32_t cnt) { for (uint32_t i = 0; i < cnt; ++i) { - executor.execute(Task::UP(new MyTask(gate, latch))); + executor.execute(std::make_unique<MyTask>(gate, latch)); } return *this; } @@ -70,7 +71,7 @@ struct MyState { { ASSERT_TRUE(!checked); checked = true; - ThreadStackExecutor::Stats stats = executor.getStats(); + ExecutorStats stats = executor.getStats(); EXPECT_EQUAL(expect_running + expect_deleted, MyTask::runCnt); EXPECT_EQUAL(expect_rejected + expect_deleted, MyTask::deleteCnt); EXPECT_EQUAL(expect_queue + expect_running + expect_deleted, @@ -187,11 +188,11 @@ TEST_F("require that executor thread stack tag can be set", ThreadStackExecutor( } TEST("require that stats can be accumulated") { - ThreadStackExecutor::Stats stats(ThreadExecutor::Stats::QueueSizeT(1) ,2,3); + ExecutorStats stats(ExecutorStats::QueueSizeT(1) ,2,3); EXPECT_EQUAL(1u, stats.queueSize.max()); EXPECT_EQUAL(2u, stats.acceptedTasks); EXPECT_EQUAL(3u, stats.rejectedTasks); - stats += ThreadStackExecutor::Stats(ThreadExecutor::Stats::QueueSizeT(7),8,9); + stats += ExecutorStats(ExecutorStats::QueueSizeT(7),8,9); EXPECT_EQUAL(2u, stats.queueSize.count()); EXPECT_EQUAL(8u, stats.queueSize.total()); EXPECT_EQUAL(8u, stats.queueSize.max()); diff --git a/vespalib/src/vespa/vespalib/util/executor_stats.h b/vespalib/src/vespa/vespalib/util/executor_stats.h index 49d83c96714..f1f58685570 100644 --- a/vespalib/src/vespa/vespalib/util/executor_stats.h +++ b/vespalib/src/vespa/vespalib/util/executor_stats.h @@ -3,6 +3,7 @@ #pragma once #include <limits> +#include <cstdint> namespace vespalib { @@ -15,10 +16,10 @@ public: AggregatedAverage() : AggregatedAverage(0ul, T(0), std::numeric_limits<T>::max(), std::numeric_limits<T>::min()) { } explicit AggregatedAverage(T value) : AggregatedAverage(1, value, value, value) { } AggregatedAverage(size_t count_in, T total_in, T min_in, T max_in) - : _count(count_in), - _total(total_in), - _min(min_in), - _max(max_in) + : _count(count_in), + _total(total_in), + _min(min_in), + _max(max_in) { } AggregatedAverage & operator += (const AggregatedAverage & rhs) { add(rhs); diff --git a/vespalib/src/vespa/vespalib/util/threadexecutor.h b/vespalib/src/vespa/vespalib/util/threadexecutor.h index 9ab7aacfc4b..36c72fa4bb0 100644 --- a/vespalib/src/vespa/vespalib/util/threadexecutor.h +++ b/vespalib/src/vespa/vespalib/util/threadexecutor.h @@ -12,11 +12,6 @@ class ThreadExecutor : public Executor { public: /** - * Internal stats that we want to observe externally. Note that - * all stats are reset each time they are observed. - **/ - using Stats = ExecutorStats; - /** * Get number of threads in the executor pool. * @return number of threads in the pool */ @@ -26,7 +21,7 @@ public: * Observe and reset stats for this object. * @return stats **/ - virtual Stats getStats() = 0; + virtual ExecutorStats getStats() = 0; /** * Sets a new upper limit for accepted number of tasks. diff --git a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp index 32e47f366cc..f80a5b4ce32 100644 --- a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp +++ b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp @@ -203,12 +203,12 @@ ThreadStackExecutorBase::num_idle_workers() const return _workers.size(); } -ThreadStackExecutorBase::Stats +ExecutorStats ThreadStackExecutorBase::getStats() { std::unique_lock guard(_lock); - Stats stats = _stats; - _stats = Stats(); + ExecutorStats stats = _stats; + _stats = ExecutorStats(); _stats.queueSize.add(_taskCount); return stats; } diff --git a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.h b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.h index 59ed385b4f4..66a34bfde95 100644 --- a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.h +++ b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.h @@ -80,7 +80,7 @@ private: std::unique_ptr<FastOS_ThreadPool> _pool; mutable std::mutex _lock; std::condition_variable _cond; - Stats _stats; + ExecutorStats _stats; Gate _executorCompletion; ArrayQueue<TaggedTask> _tasks; ArrayQueue<Worker*> _workers; @@ -188,7 +188,7 @@ public: **/ size_t num_idle_workers() const; - Stats getStats() override; + ExecutorStats getStats() override; Task::UP execute(Task::UP task) override; |