From 66469fc281fc4abb3284574f77a8051fee8116b9 Mon Sep 17 00:00:00 2001
From: eldritch horrors <pennae@lix.systems>
Date: Fri, 2 Aug 2024 17:00:57 +0200
Subject: [PATCH] libstore: move Goal::waiteeDone into Worker::goalFinished

this begins a long and arduous journey to remove all result state from
Goal, to eventually drop the std::enable_shared_from_this base, and to
completely eliminate all unsynchronized modification of states of both
Goal and Worker. by the end of this we will hopefully be able to start
and reap multiple derivation builds in parallel, which should speed up
the process quite a bit (at least for short local builds, others might
not notice a large difference. the build hooks will remain a problem.)

Change-Id: I57dcd9b2cab4636ed4aa24cdec67124fef883345
---
 src/libstore/build/derivation-goal.cc |  4 +---
 src/libstore/build/derivation-goal.hh |  2 +-
 src/libstore/build/goal.cc            | 27 ---------------------------
 src/libstore/build/goal.hh            |  2 +-
 src/libstore/build/worker.cc          | 22 +++++++++++++++++++++-
 5 files changed, 24 insertions(+), 33 deletions(-)

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index ab7b2b88c..17a2b04f1 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -1541,10 +1541,8 @@ Goal::Finished DerivationGoal::done(
 }
 
 
-void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result)
+void DerivationGoal::waiteeDone(GoalPtr waitee)
 {
-    Goal::waiteeDone(waitee, result);
-
     if (!useDerivation) return;
 
     auto * dg = dynamic_cast<DerivationGoal *>(&*waitee);
diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh
index 268b717dd..c43e2aed5 100644
--- a/src/libstore/build/derivation-goal.hh
+++ b/src/libstore/build/derivation-goal.hh
@@ -332,7 +332,7 @@ struct DerivationGoal : public Goal
         SingleDrvOutputs builtOutputs = {},
         std::optional<Error> ex = {});
 
-    void waiteeDone(GoalPtr waitee, ExitCode result) override;
+    void waiteeDone(GoalPtr waitee) override;
 
     StorePathSet exportReferences(const StorePathSet & storePaths);
 
diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc
index 40a3bae8d..f26c2c671 100644
--- a/src/libstore/build/goal.cc
+++ b/src/libstore/build/goal.cc
@@ -18,33 +18,6 @@ void Goal::addWaitee(GoalPtr waitee)
 }
 
 
-void Goal::waiteeDone(GoalPtr waitee, ExitCode result)
-{
-    assert(waitees.count(waitee));
-    waitees.erase(waitee);
-
-    trace(fmt("waitee '%s' done; %d left", waitee->name, waitees.size()));
-
-    if (result == ecFailed || result == ecNoSubstituters || result == ecIncompleteClosure) ++nrFailed;
-
-    if (result == ecNoSubstituters) ++nrNoSubstituters;
-
-    if (result == ecIncompleteClosure) ++nrIncompleteClosure;
-
-    if (waitees.empty() || (result == ecFailed && !settings.keepGoing)) {
-
-        /* If we failed and keepGoing is not set, we remove all
-           remaining waitees. */
-        for (auto & goal : waitees) {
-            goal->waiters.extract(shared_from_this());
-        }
-        waitees.clear();
-
-        worker.wakeUp(shared_from_this());
-    }
-}
-
-
 void Goal::trace(std::string_view s)
 {
     debug("%1%: %2%", name, s);
diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh
index adb3ab94b..dd29b9fc4 100644
--- a/src/libstore/build/goal.hh
+++ b/src/libstore/build/goal.hh
@@ -135,7 +135,7 @@ public:
 
     void addWaitee(GoalPtr waitee);
 
-    virtual void waiteeDone(GoalPtr waitee, ExitCode result);
+    virtual void waiteeDone(GoalPtr waitee) { }
 
     virtual WorkResult handleChildOutput(int fd, std::string_view data)
     {
diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc
index f4c352b61..84727a377 100644
--- a/src/libstore/build/worker.cc
+++ b/src/libstore/build/worker.cc
@@ -154,7 +154,27 @@ void Worker::goalFinished(GoalPtr goal, Goal::Finished & f)
 
     for (auto & i : goal->waiters) {
         if (GoalPtr waiting = i.lock()) {
-            waiting->waiteeDone(goal, f.result);
+            assert(waiting->waitees.count(goal));
+            waiting->waitees.erase(goal);
+
+            waiting->trace(fmt("waitee '%s' done; %d left", goal->name, waiting->waitees.size()));
+
+            if (f.result != Goal::ecSuccess) ++waiting->nrFailed;
+            if (f.result == Goal::ecNoSubstituters) ++waiting->nrNoSubstituters;
+            if (f.result == Goal::ecIncompleteClosure) ++waiting->nrIncompleteClosure;
+
+            if (waiting->waitees.empty() || (f.result == Goal::ecFailed && !settings.keepGoing)) {
+                /* If we failed and keepGoing is not set, we remove all
+                   remaining waitees. */
+                for (auto & i : waiting->waitees) {
+                    i->waiters.extract(waiting);
+                }
+                waiting->waitees.clear();
+
+                wakeUp(waiting);
+            }
+
+            waiting->waiteeDone(goal);
         }
     }
     goal->waiters.clear();