From 732de75f67f030886fd6bb421d49481caa3aa8cf Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 30 Sep 2024 01:31:30 +0200 Subject: 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(-) (limited to 'src/libstore') 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, kj::Promise> 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> 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 struct CachedGoal { @@ -149,11 +144,6 @@ private: kj::Own> childFinished; - /** - * Wake up a goal (i.e., there is something for it to do). - */ - void wakeUp(GoalPtr goal); - /** * Wait for input to become available. */ -- cgit v1.2.3