From 9889c79fe355ce9d1b6ee7814cdeec0fbf70823a Mon Sep 17 00:00:00 2001
From: eldritch horrors <pennae@lix.systems>
Date: Mon, 30 Sep 2024 01:31:30 +0200
Subject: [PATCH] libstore: turn Worker::updateStatistics into a promise

we'll now loop to update displayed statistics, and use this loop to
limit the update rate to 50 times per second. we could have updated
much more frequently before this (once per iteration of `runImpl`),
much faster than would ever be useful in practice. aggressive stats
updates can even impede progress due to terminal or network delays.

Change-Id: Ifba755a2569f73c919b1fbb06a142c0951395d6d
---
 src/libstore/build/worker.cc | 21 ++++++++++++---------
 src/libstore/build/worker.hh |  7 ++++---
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc
index 9317a5ac2..5be706e42 100644
--- a/src/libstore/build/worker.cc
+++ b/src/libstore/build/worker.cc
@@ -258,11 +258,13 @@ void Worker::childTerminated(GoalPtr goal)
 }
 
 
-void Worker::updateStatistics()
-{
-    // only update progress info while running. this notably excludes updating
-    // progress info while destroying, which causes the progress bar to assert
-    if (running && statisticsOutdated) {
+kj::Promise<Result<void>> Worker::updateStatistics()
+try {
+    while (true) {
+        statisticsUpdateInhibitor = co_await statisticsUpdateSignal.acquire();
+
+        // only update progress info while running. this notably excludes updating
+        // progress info while destroying, which causes the progress bar to assert
         actDerivations.progress(
             doneBuilds, expectedBuilds + doneBuilds, runningBuilds, failedBuilds
         );
@@ -275,8 +277,11 @@ void Worker::updateStatistics()
         act.setExpected(actFileTransfer, expectedDownloadSize + doneDownloadSize);
         act.setExpected(actCopyPath, expectedNarSize + doneNarSize);
 
-        statisticsOutdated = false;
+        // limit to 50fps. that should be more than good enough for anything we do
+        co_await aio.provider->getTimer().afterDelay(20 * kj::MILLISECONDS);
     }
+} catch (...) {
+    co_return result::failure(std::current_exception());
 }
 
 std::vector<GoalPtr> Worker::run(std::function<Targets (GoalFactory &)> req)
@@ -287,14 +292,12 @@ std::vector<GoalPtr> Worker::run(std::function<Targets (GoalFactory &)> req)
     running = true;
     Finally const _stop([&] { running = false; });
 
-    updateStatistics();
-
     topGoals.clear();
     for (auto & [goal, _promise] : _topGoals) {
         topGoals.insert(goal);
     }
 
-    auto promise = runImpl();
+    auto promise = runImpl().exclusiveJoin(updateStatistics());
 
     // TODO GC interface?
     if (auto localStore = dynamic_cast<LocalStore *>(&store); localStore && settings.minFree != 0) {
diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh
index bb51a2114..d6cde8384 100644
--- a/src/libstore/build/worker.hh
+++ b/src/libstore/build/worker.hh
@@ -167,16 +167,17 @@ private:
     /**
       * Pass current stats counters to the logger for progress bar updates.
       */
-    void updateStatistics();
+    kj::Promise<Result<void>> updateStatistics();
 
-    bool statisticsOutdated = true;
+    AsyncSemaphore statisticsUpdateSignal{1};
+    std::optional<AsyncSemaphore::Token> statisticsUpdateInhibitor;
 
     /**
       * Mark statistics as outdated, such that `updateStatistics` will be called.
       */
     void updateStatisticsLater()
     {
-        statisticsOutdated = true;
+        statisticsUpdateInhibitor = {};
     }
 
     kj::Promise<Result<void>> runImpl();