lix-project/lix
20
57
Fork 31
Code Issues 347 Code Review (Gerrit) Projects 3 Releases Wiki Activity

libstore: remove worker removeGoal

Browse source 
this was immensely inefficient on large caches, as can exist when many
derivations are buildable simultaneously. since we have smart pointers
to goals we can do cache maintenance in goal deleters instead, and use
the exact iterators instead of doing a linear search. this *does* rely
on goals being deleted to remove them from the cache, which isn't true
for toplevel goals. those would have previously been removed when done
in all cases, removing the cache entry when keep-going is set. this is
arguably incorrect since it might result in those goals being retried,
although that could only happen with dynamic derivations or the likes.
(luckily dynamic derivations not complete enough to allow this at all)

Change-Id: I8e750b868393588c33e4829333d370f2c509ce99
This commit is contained in:
eldritch horrors 2024-09-25 23:57:46 +02:00
parent 47ddd11933
commit ccd2862666
2 changed files with 28 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

66
src/libstore/build/worker.cc
View file

@ -65,7 +65,26 @@ std::pair<std::shared_ptr<G>, kj::Promise<void>> Worker::makeGoalCommon(
auto & goal_weak = it->second;
auto goal = goal_weak.goal.lock();
if (!goal) {
goal = create();
struct Deleter
{
std::map<ID, CachedGoal<G>> * from;
std::map<ID, CachedGoal<G>>::const_iterator iter;
void operator()(G * g)
{
from->erase(iter);
delete g;
}
};
// this dance is necessary to be memory-safe in exceptional cases.
// if *anything* here throws we must still delete the goal object.
goal = [&] {
std::unique_ptr<G, Deleter> tmp(nullptr, Deleter{&map, it});
tmp.reset(create().release());
return tmp;
}();
goal->notify = std::move(goal_weak.fulfiller);
goal_weak.goal = goal;
wakeUp(goal);
@ -165,21 +184,6 @@ std::pair<GoalPtr, kj::Promise<void>> Worker::makeGoal(const DerivedPath & req,
}
template<typename G>
static void removeGoal(std::shared_ptr<G> goal, auto & goalMap)
{
/* !!! inefficient */
for (auto i = goalMap.begin();
i != goalMap.end(); )
if (i->second.goal.lock() == goal) {
auto j = i; ++j;
goalMap.erase(i);
i = j;
}
else ++i;
}
void Worker::goalFinished(GoalPtr goal, Goal::Finished & f)
{
goal->trace("done");
@ -192,9 +196,16 @@ void Worker::goalFinished(GoalPtr goal, Goal::Finished & f)
hashMismatch |= f.hashMismatch;
checkMismatch |= f.checkMismatch;
removeGoal(goal);
goal->notify->fulfill();
goal->cleanup();
if (topGoals.find(goal) != topGoals.end()) {
topGoals.erase(goal);
/* If a top-level goal failed, then kill all other goals
(unless keepGoing was set). */
if (goal->exitCode == Goal::ecFailed && !settings.keepGoing)
topGoals.clear();
}
}
void Worker::handleWorkResult(GoalPtr goal, Goal::WorkResult how)
@ -210,27 +221,6 @@ void Worker::handleWorkResult(GoalPtr goal, Goal::WorkResult how)
updateStatistics();
}
void Worker::removeGoal(GoalPtr goal)
{
if (auto drvGoal = std::dynamic_pointer_cast<DerivationGoal>(goal))
nix::removeGoal(drvGoal, derivationGoals);
else if (auto subGoal = std::dynamic_pointer_cast<PathSubstitutionGoal>(goal))
nix::removeGoal(subGoal, substitutionGoals);
else if (auto subGoal = std::dynamic_pointer_cast<DrvOutputSubstitutionGoal>(goal))
nix::removeGoal(subGoal, drvOutputSubstitutionGoals);
else
assert(false);
if (topGoals.find(goal) != topGoals.end()) {
topGoals.erase(goal);
/* If a top-level goal failed, then kill all other goals
(unless keepGoing was set). */
if (goal->exitCode == Goal::ecFailed && !settings.keepGoing)
topGoals.clear();
}
}
void Worker::wakeUp(GoalPtr goal)
{
goal->trace("woken up");

5
src/libstore/build/worker.hh
View file

@ -158,11 +158,6 @@ private:
*/
void waitForInput();
/**
* Remove a dead goal.
*/
void removeGoal(GoalPtr goal);
/**
* Registers a running child process.
*/