From 732de75f67f030886fd6bb421d49481caa3aa8cf 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: remove Worker::wakeUp()

Worker::run() is now entirely based on the kj event loop and promises,
so we need not handle awakeness of goals manually any more. every goal
can instead, once it has finished a partial work call, defer itself to
being called again in the next iteration of the loop. same end effect.

Change-Id: I320eee2fa60bcebaabd74d1323fa96d1402c1d15
---
 src/libstore/build/worker.cc | 33 +++++++--------------------------
 src/libstore/build/worker.hh | 10 ----------
 2 files changed, 7 insertions(+), 36 deletions(-)

diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc
index e9904a1f5..9317a5ac2 100644
--- a/src/libstore/build/worker.cc
+++ b/src/libstore/build/worker.cc
@@ -44,7 +44,6 @@ Worker::~Worker()
        are in trouble, since goals may call childTerminated() etc. in
        their destructors). */
     topGoals.clear();
-    awake.clear();
     children.clear();
 
     assert(expectedSubstitutions == 0);
@@ -68,7 +67,10 @@ std::pair<std::shared_ptr<G>, kj::Promise<void>> Worker::makeGoalCommon(
         goal = create();
         goal->notify = std::move(goal_weak.fulfiller);
         goal_weak.goal = goal;
-        wakeUp(goal);
+        // do not start working immediately, this round of the event loop
+        // may have more calls to this function lined up that'll also run
+        // modify(). starting early can then cause the goals to misbehave
+        childStarted(goal, kj::evalLater([goal] { return goal->work(); }));
     } else {
         modify(*goal);
     }
@@ -201,7 +203,9 @@ void Worker::handleWorkResult(GoalPtr goal, Goal::WorkResult how)
 {
     std::visit(
         overloaded{
-            [&](Goal::StillAlive) { wakeUp(goal); },
+            [&](Goal::StillAlive) {
+                childStarted(goal, kj::evalLater([goal] { return goal->work(); }));
+            },
             [&](Goal::Finished & f) { goalFinished(goal, f); },
         },
         how
@@ -230,13 +234,6 @@ void Worker::removeGoal(GoalPtr goal)
 }
 
 
-void Worker::wakeUp(GoalPtr goal)
-{
-    goal->trace("woken up");
-    awake.insert(goal);
-}
-
-
 void Worker::childStarted(GoalPtr goal, kj::Promise<Result<Goal::WorkResult>> promise)
 {
     children.add(promise
@@ -322,26 +319,11 @@ try {
 
         checkInterrupt();
 
-        /* Call every wake goal (in the ordering established by
-           CompareGoalPtrs). */
-        while (!awake.empty() && !topGoals.empty()) {
-            Goals awake2 = std::move(awake);
-            for (auto & goal : awake2) {
-                checkInterrupt();
-                childStarted(goal, goal->work());
-
-                if (topGoals.empty()) break; // stuff may have been cancelled
-            }
-        }
-
         if (topGoals.empty()) break;
 
         /* Wait for input. */
         if (!children.isEmpty())
             (co_await waitForInput()).value();
-        else {
-            assert(!awake.empty());
-        }
 
         if (childException) {
             std::rethrow_exception(childException);
@@ -351,7 +333,6 @@ try {
     /* If --keep-going is not set, it's possible that the main goal
        exited while some of its subgoals were still active.  But if
        --keep-going *is* set, then they must all be finished now. */
-    assert(!settings.keepGoing || awake.empty());
     assert(!settings.keepGoing || children.isEmpty());
 
     co_return result::success();
diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh
index dc85c43e3..bb51a2114 100644
--- a/src/libstore/build/worker.hh
+++ b/src/libstore/build/worker.hh
@@ -91,11 +91,6 @@ private:
      */
     Goals topGoals;
 
-    /**
-     * Goals that are ready to do some work.
-     */
-    Goals awake;
-
     template<typename G>
     struct CachedGoal
     {
@@ -149,11 +144,6 @@ private:
 
     kj::Own<kj::PromiseFulfiller<void>> childFinished;
 
-    /**
-     * Wake up a goal (i.e., there is something for it to do).
-     */
-    void wakeUp(GoalPtr goal);
-
     /**
      * Wait for input to become available.
      */