properly set utilization

3 files changed, 32 insertions, 24 deletions

diff --git a/vespalib/src/tests/executor/threadstackexecutor_test.cpp b/vespalib/src/tests/executor/threadstackexecutor_test.cpp
index 00e23db244b..7fec2b76c06 100644
--- a/vespalib/src/tests/executor/threadstackexecutor_test.cpp
+++ b/vespalib/src/tests/executor/threadstackexecutor_test.cpp
@@ -4,7 +4,7 @@
 #include <vespa/vespalib/util/threadstackexecutor.h>
 #include <vespa/vespalib/util/backtrace.h>
 #include <vespa/vespalib/util/size_literals.h>
-#include <atomic>
+#include <thread>
 
 using namespace vespalib;
 
@@ -190,15 +190,15 @@ TEST_F("require that executor thread stack tag can be set", ThreadStackExecutor(
 
 TEST("require that stats can be accumulated") {
     EXPECT_TRUE(std::atomic<duration>::is_always_lock_free);
-    ExecutorStats stats(3, ExecutorStats::QueueSizeT(1) ,2,3,7, 0.6);
+    ExecutorStats stats(ExecutorStats::QueueSizeT(1) ,2,3,7);
+    stats.setUtil(3, 0.8);
     EXPECT_EQUAL(1u, stats.queueSize.max());
     EXPECT_EQUAL(2u, stats.acceptedTasks);
     EXPECT_EQUAL(3u, stats.rejectedTasks);
     EXPECT_EQUAL(7u, stats.wakeupCount);
-    EXPECT_EQUAL(3u, stats.threadCount);
-    EXPECT_EQUAL(0.6, stats.absUtil);
+    EXPECT_EQUAL(3u, stats.getThreadCount());
     EXPECT_EQUAL(0.2, stats.getUtil());
-    stats.aggregate(ExecutorStats(7, ExecutorStats::QueueSizeT(7),8,9,11, 1.9));
+    stats.aggregate(ExecutorStats(ExecutorStats::QueueSizeT(7),8,9,11).setUtil(7,0.5));
     EXPECT_EQUAL(2u, stats.queueSize.count());
     EXPECT_EQUAL(8u, stats.queueSize.total());
     EXPECT_EQUAL(8u, stats.queueSize.max());
@@ -206,18 +206,18 @@ TEST("require that stats can be accumulated") {
     EXPECT_EQUAL(8u, stats.queueSize.max());
     EXPECT_EQUAL(4.0, stats.queueSize.average());
 
-    EXPECT_EQUAL(10u, stats.threadCount);
+    EXPECT_EQUAL(10u, stats.getThreadCount());
     EXPECT_EQUAL(10u, stats.acceptedTasks);
     EXPECT_EQUAL(12u, stats.rejectedTasks);
     EXPECT_EQUAL(18u, stats.wakeupCount);
-    EXPECT_EQUAL(2.5, stats.absUtil);
-    EXPECT_EQUAL(0.25, stats.getUtil());
+    EXPECT_EQUAL(0.41, stats.getUtil());
 }
 
-TEST("Test that load is computed") {
+TEST("Test that utilization is computed") {
     ThreadStackExecutor executor(1, 128_Ki);
+    std::this_thread::sleep_for(1s);
     auto stats = executor.getStats();
-    EXPECT_EQUAL(0.0, stats.absUtil);
+    EXPECT_GREATER(0.01, stats.getUtil());
 }
 
 TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/vespalib/src/vespa/vespalib/util/executor_stats.h b/vespalib/src/vespa/vespalib/util/executor_stats.h
index 4b4354dc977..577ae933ec2 100644
--- a/vespalib/src/vespa/vespalib/util/executor_stats.h
+++ b/vespalib/src/vespa/vespalib/util/executor_stats.h
@@ -53,25 +53,28 @@ private:
  * Struct representing stats for an executor.
  * Note that aggregation requires sample interval to be the same(similar) for all samples.
  **/
-struct ExecutorStats {
+class ExecutorStats {
+private:
+    size_t   _threadCount;
+    double   _absUtil;
+public:
     using QueueSizeT = AggregatedAverage<size_t>;
-    uint32_t   threadCount;
     QueueSizeT queueSize;
     size_t     acceptedTasks;
     size_t     rejectedTasks;
     size_t     wakeupCount; // Number of times a worker was woken up,
-    double     absUtil;
-    ExecutorStats() : ExecutorStats(1, QueueSizeT(), 0, 0, 0, 1.0) {}
-    ExecutorStats(uint32_t executorCount_in, QueueSizeT queueSize_in, size_t accepted, size_t rejected, size_t wakeupCount_in, double absUtil_in)
-        : threadCount(executorCount_in),
+
+    ExecutorStats() : ExecutorStats(QueueSizeT(), 0, 0, 0) {}
+    ExecutorStats(QueueSizeT queueSize_in, size_t accepted, size_t rejected, size_t wakeupCount_in)
+        : _threadCount(1),
+          _absUtil(1.0),
           queueSize(queueSize_in),
           acceptedTasks(accepted),
           rejectedTasks(rejected),
-          wakeupCount(wakeupCount_in),
-          absUtil(absUtil_in)
+          wakeupCount(wakeupCount_in)
     {}
     void aggregate(const ExecutorStats & rhs) {
-        threadCount += rhs.threadCount;
+        _threadCount += rhs._threadCount;
         queueSize = QueueSizeT(queueSize.count() + rhs.queueSize.count(),
                                queueSize.total() + rhs.queueSize.total(),
                                queueSize.min() + rhs.queueSize.min(),
@@ -79,9 +82,15 @@ struct ExecutorStats {
         acceptedTasks += rhs.acceptedTasks;
         rejectedTasks += rhs.rejectedTasks;
         wakeupCount += rhs.wakeupCount;
-        absUtil += rhs.absUtil;
+        _absUtil += rhs._absUtil;
+    }
+    ExecutorStats & setUtil(uint32_t threadCount, double idle) {
+        _threadCount = threadCount;
+        _absUtil = (1.0 - idle) * threadCount;
+        return *this;
     }
-    double getUtil() const { return absUtil / threadCount; }
+    double getUtil() const { return _absUtil / _threadCount; }
+    size_t getThreadCount() const { return _threadCount; }
 };
 
 }
diff --git a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp
index 53777ee6e6b..9e6456d2f56 100644
--- a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp
+++ b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp
@@ -178,7 +178,6 @@ void
 ThreadStackExecutorBase::start(uint32_t threads)
 {
     assert(threads > 0);
-    _stats.threadCount = threads;
     _idleTracker.reset(steady_clock::now(), threads);
     for (uint32_t i = 0; i < threads; ++i) {
         FastOS_ThreadInterface *thread = _pool->NewThread(_thread_init.get());
@@ -232,12 +231,12 @@ ThreadStackExecutorBase::getStats()
 {
     std::unique_lock guard(_lock);
     ExecutorStats stats = _stats;
-    stats.threadCount = getNumThreads();
     steady_time now = steady_clock::now();
     for (size_t i(0); i < _workers.size(); i++) {
         _idleTracker.was_idle(_workers.access(i)->idleTracker.reset(now));
     }
-    stats.absUtil = _idleTracker.reset(now, 1);
+    size_t numThreads = getNumThreads();
+    stats.setUtil(numThreads, _idleTracker.reset(now, numThreads));
     _stats = ExecutorStats();
     _stats.queueSize.add(_taskCount);
     return stats;