From fc987b412385a3b10d58eabd516acc7ff3b27b11 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Fri, 2 Aug 2024 17:00:57 +0200 Subject: [PATCH] libstore: remove Goal::addWaitee Change-Id: I1b00d1a537d84790878cb0e81aaa1cbaa143d62d --- src/libstore/build/derivation-goal.cc | 34 ++++++++++--------- .../build/drv-output-substitution-goal.cc | 9 ++--- src/libstore/build/goal.cc | 7 ---- src/libstore/build/goal.hh | 14 +++++--- src/libstore/build/substitution-goal.cc | 7 ++-- src/libstore/build/worker.cc | 6 ++++ 6 files changed, 43 insertions(+), 34 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index b7df98472..ace00b1fb 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -174,10 +174,9 @@ Goal::WorkResult DerivationGoal::getDerivation() return loadDerivation(); } - addWaitee(worker.makePathSubstitutionGoal(drvPath)); state = &DerivationGoal::loadDerivation; - return StillAlive{}; + return WaitForGoals{{worker.makePathSubstitutionGoal(drvPath)}}; } @@ -268,11 +267,12 @@ Goal::WorkResult DerivationGoal::haveDerivation() /* We are first going to try to create the invalid output paths through substitutes. If that doesn't work, we'll build them. */ + WaitForGoals result; if (settings.useSubstitutes && parsedDrv->substitutesAllowed()) for (auto & [outputName, status] : initialOutputs) { if (!status.wanted) continue; if (!status.known) - addWaitee( + result.goals.insert( worker.makeDrvOutputSubstitutionGoal( DrvOutput{status.outputHash, outputName}, buildMode == bmRepair ? Repair : NoRepair @@ -280,18 +280,18 @@ Goal::WorkResult DerivationGoal::haveDerivation() ); else { auto * cap = getDerivationCA(*drv); - addWaitee(worker.makePathSubstitutionGoal( + result.goals.insert(worker.makePathSubstitutionGoal( status.known->path, buildMode == bmRepair ? Repair : NoRepair, cap ? std::optional { *cap } : std::nullopt)); } } - if (waitees.empty()) { /* to prevent hang (no wake-up event) */ + if (result.goals.empty()) { /* to prevent hang (no wake-up event) */ return outputsSubstitutionTried(); } else { state = &DerivationGoal::outputsSubstitutionTried; - return StillAlive{}; + return result; } } @@ -362,6 +362,8 @@ Goal::WorkResult DerivationGoal::outputsSubstitutionTried() produced using a substitute. So we have to build instead. */ Goal::WorkResult DerivationGoal::gaveUpOnSubstitution() { + WaitForGoals result; + /* At this point we are building all outputs, so if more are wanted there is no need to restart. */ needRestart = NeedRestartForMoreOutputs::BuildInProgressWillNotNeed; @@ -373,7 +375,7 @@ Goal::WorkResult DerivationGoal::gaveUpOnSubstitution() addWaiteeDerivedPath = [&](ref inputDrv, const DerivedPathMap::ChildNode & inputNode) { if (!inputNode.value.empty()) - addWaitee(worker.makeGoal( + result.goals.insert(worker.makeGoal( DerivedPath::Built { .drvPath = inputDrv, .outputs = inputNode.value, @@ -418,14 +420,14 @@ Goal::WorkResult DerivationGoal::gaveUpOnSubstitution() if (!settings.useSubstitutes) throw Error("dependency '%s' of '%s' does not exist, and substitution is disabled", worker.store.printStorePath(i), worker.store.printStorePath(drvPath)); - addWaitee(worker.makePathSubstitutionGoal(i)); + result.goals.insert(worker.makePathSubstitutionGoal(i)); } - if (waitees.empty()) {/* to prevent hang (no wake-up event) */ + if (result.goals.empty()) {/* to prevent hang (no wake-up event) */ return inputsRealised(); } else { state = &DerivationGoal::inputsRealised; - return StillAlive{}; + return result; } } @@ -466,6 +468,7 @@ Goal::WorkResult DerivationGoal::repairClosure() } /* Check each path (slow!). */ + WaitForGoals result; for (auto & i : outputClosure) { if (worker.pathContentsGood(i)) continue; printError( @@ -473,9 +476,9 @@ Goal::WorkResult DerivationGoal::repairClosure() worker.store.printStorePath(i), worker.store.printStorePath(drvPath)); auto drvPath2 = outputsToDrv.find(i); if (drvPath2 == outputsToDrv.end()) - addWaitee(worker.makePathSubstitutionGoal(i, Repair)); + result.goals.insert(worker.makePathSubstitutionGoal(i, Repair)); else - addWaitee(worker.makeGoal( + result.goals.insert(worker.makeGoal( DerivedPath::Built { .drvPath = makeConstantStorePathRef(drvPath2->second), .outputs = OutputsSpec::All { }, @@ -483,12 +486,12 @@ Goal::WorkResult DerivationGoal::repairClosure() bmRepair)); } - if (waitees.empty()) { + if (result.goals.empty()) { return done(BuildResult::AlreadyValid, assertPathValidity()); } state = &DerivationGoal::closureRepaired; - return StillAlive{}; + return result; } @@ -580,10 +583,9 @@ Goal::WorkResult DerivationGoal::inputsRealised() resolvedDrvGoal = worker.makeDerivationGoal( pathResolved, wantedOutputs, buildMode); - addWaitee(resolvedDrvGoal); state = &DerivationGoal::resolvedFinished; - return StillAlive{}; + return WaitForGoals{{resolvedDrvGoal}}; } std::function::ChildNode &)> accumInputPaths; diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 068f41a49..b41dae5d6 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -100,6 +100,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::realisationFetched() return tryNext(); } + WaitForGoals result; for (const auto & [depId, depPath] : outputInfo->dependentRealisations) { if (depId != id) { if (auto localOutputInfo = worker.store.queryRealisation(depId); @@ -115,17 +116,17 @@ Goal::WorkResult DrvOutputSubstitutionGoal::realisationFetched() ); return tryNext(); } - addWaitee(worker.makeDrvOutputSubstitutionGoal(depId)); + result.goals.insert(worker.makeDrvOutputSubstitutionGoal(depId)); } } - addWaitee(worker.makePathSubstitutionGoal(outputInfo->outPath)); + result.goals.insert(worker.makePathSubstitutionGoal(outputInfo->outPath)); - if (waitees.empty()) { + if (result.goals.empty()) { return outPathValid(); } else { state = &DrvOutputSubstitutionGoal::outPathValid; - return StillAlive{}; + return result; } } diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index f26c2c671..a4e66d989 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -11,13 +11,6 @@ bool CompareGoalPtrs::operator() (const GoalPtr & a, const GoalPtr & b) const { } -void Goal::addWaitee(GoalPtr waitee) -{ - waitees.insert(waitee); - waitee->waiters.insert(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 3d487f6da..83f52927e 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -109,13 +109,21 @@ public: struct [[nodiscard]] WaitForSlot {}; struct [[nodiscard]] WaitForAWhile {}; struct [[nodiscard]] ContinueImmediately {}; + struct [[nodiscard]] WaitForGoals { + Goals goals; + }; struct [[nodiscard]] Finished { ExitCode result; std::unique_ptr ex; }; - struct [[nodiscard]] WorkResult - : std::variant + struct [[nodiscard]] WorkResult : std::variant< + StillAlive, + WaitForSlot, + WaitForAWhile, + ContinueImmediately, + WaitForGoals, + Finished> { WorkResult() = delete; using variant::variant; @@ -137,8 +145,6 @@ public: virtual WorkResult work() = 0; - void addWaitee(GoalPtr waitee); - virtual void waiteeDone(GoalPtr waitee) { } virtual WorkResult handleChildOutput(int fd, std::string_view data) diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 77b003612..67a5f20bb 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -152,15 +152,16 @@ Goal::WorkResult PathSubstitutionGoal::tryNext() /* To maintain the closure invariant, we first have to realise the paths referenced by this one. */ + WaitForGoals result; for (auto & i : info->references) if (i != storePath) /* ignore self-references */ - addWaitee(worker.makePathSubstitutionGoal(i)); + result.goals.insert(worker.makePathSubstitutionGoal(i)); - if (waitees.empty()) {/* to prevent hang (no wake-up event) */ + if (result.goals.empty()) {/* to prevent hang (no wake-up event) */ return referencesValid(); } else { state = &PathSubstitutionGoal::referencesValid; - return StillAlive{}; + return result; } } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 32ac27483..712af6c7e 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -190,6 +190,12 @@ void Worker::handleWorkResult(GoalPtr goal, Goal::WorkResult how) [&](Goal::WaitForSlot) { waitForBuildSlot(goal); }, [&](Goal::WaitForAWhile) { waitForAWhile(goal); }, [&](Goal::ContinueImmediately) { wakeUp(goal); }, + [&](Goal::WaitForGoals & w) { + for (auto & dep : w.goals) { + goal->waitees.insert(dep); + dep->waiters.insert(goal); + } + }, [&](Goal::Finished & f) { goalFinished(goal, f); }, }, how