From 548c973e8282bbec5b14f3860218b23564dc0381 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Thu, 25 Jul 2024 18:05:42 +0200 Subject: [PATCH] libstore: remove Worker::updateProgress just update progress every time a goal has returned from work(). there seem to be no performance penalties, and the code is much simpler now. Change-Id: I288ee568b764ee61f40a498d986afda49987cb50 --- src/libstore/build/derivation-goal.cc | 5 ----- src/libstore/build/drv-output-substitution-goal.cc | 2 -- src/libstore/build/substitution-goal.cc | 6 ------ src/libstore/build/worker.cc | 13 +++++++++++++ src/libstore/build/worker.hh | 8 -------- 5 files changed, 13 insertions(+), 21 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index d0152355f..8899cebfb 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -79,7 +79,6 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, trace("created"); mcExpectedBuilds = std::make_unique>(worker.expectedBuilds); - worker.updateProgress(); } @@ -100,7 +99,6 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation trace("created"); mcExpectedBuilds = std::make_unique>(worker.expectedBuilds); - worker.updateProgress(); /* Prevent the .chroot directory from being garbage-collected. (See isActiveTempFile() in gc.cc.) */ @@ -670,7 +668,6 @@ void DerivationGoal::started() act = std::make_unique(*logger, lvlInfo, actBuild, msg, Logger::Fields{worker.store.printStorePath(drvPath), hook ? machineName : "", 1, 1}); mcRunningBuilds = std::make_unique>(worker.runningBuilds); - worker.updateProgress(); } void DerivationGoal::tryToBuild() @@ -1537,8 +1534,6 @@ void DerivationGoal::done( worker.failedBuilds++; } - worker.updateProgress(); - auto traceBuiltOutputsFile = getEnv("_NIX_TRACE_BUILT_OUTPUTS").value_or(""); if (traceBuiltOutputsFile != "") { std::fstream fs; diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 735a07f96..4be48c41d 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -48,7 +48,6 @@ void DrvOutputSubstitutionGoal::tryNext() maintainRunningSubstitutions = std::make_unique>(worker.runningCASubstitutions); - worker.updateProgress(); if (subs.size() == 0) { /* None left. Terminate this goal and let someone else deal @@ -62,7 +61,6 @@ void DrvOutputSubstitutionGoal::tryNext() if (substituterFailed) { worker.failedSubstitutions++; - worker.updateProgress(); } return; diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 1330f061c..a88bdea26 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -86,7 +86,6 @@ void PathSubstitutionGoal::tryNext() if (substituterFailed) { worker.failedSubstitutions++; - worker.updateProgress(); } return; @@ -150,8 +149,6 @@ void PathSubstitutionGoal::tryNext() ? std::make_unique>(worker.expectedDownloadSize, narInfo->fileSize) : nullptr; - worker.updateProgress(); - /* Bail out early if this substituter lacks a valid signature. LocalStore::addToStore() also checks for this, but only after we've downloaded the path. */ @@ -210,7 +207,6 @@ void PathSubstitutionGoal::tryToRun() } maintainRunningSubstitutions = std::make_unique>(worker.runningSubstitutions); - worker.updateProgress(); outPipe.create(); @@ -289,8 +285,6 @@ void PathSubstitutionGoal::finished() worker.doneNarSize += maintainExpectedNar->delta; maintainExpectedNar.reset(); - worker.updateProgress(); - done(ecSuccess, BuildResult::Substituted); } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 402cdc5b4..25b20511b 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -300,6 +300,19 @@ void Worker::run(const Goals & _topGoals) for (auto & goal : awake2) { checkInterrupt(); goal->work(); + + actDerivations.progress( + doneBuilds, expectedBuilds + doneBuilds, runningBuilds, failedBuilds + ); + actSubstitutions.progress( + doneSubstitutions, + expectedSubstitutions + doneSubstitutions, + runningSubstitutions, + failedSubstitutions + ); + act.setExpected(actFileTransfer, expectedDownloadSize + doneDownloadSize); + act.setExpected(actCopyPath, expectedNarSize + doneNarSize); + if (topGoals.empty()) break; // stuff may have been cancelled } } diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index bbebeeb9c..eee47e23d 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -281,14 +281,6 @@ public: bool pathContentsGood(const StorePath & path); void markContentsGood(const StorePath & path); - - void updateProgress() - { - actDerivations.progress(doneBuilds, expectedBuilds + doneBuilds, runningBuilds, failedBuilds); - actSubstitutions.progress(doneSubstitutions, expectedSubstitutions + doneSubstitutions, runningSubstitutions, failedSubstitutions); - act.setExpected(actFileTransfer, expectedDownloadSize + doneDownloadSize); - act.setExpected(actCopyPath, expectedNarSize + doneNarSize); - } }; }