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

Properly type the derivation and substitution goal maps

Browse source 
As a bonus, Worker::removeGoal is less inefficient.
This commit is contained in:
John Ericson 2020-10-11 17:07:14 +00:00
parent 8cc510fb79
commit 5c74a6147b
2 changed files with 22 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
src/libstore/build.hh
View file

@ -28,7 +28,6 @@ struct HookInstance;
/* A pointer to a goal. */ /* A pointer to a goal. */
struct Goal; struct Goal;
class DerivationGoal;
typedef std::shared_ptr<Goal> GoalPtr; typedef std::shared_ptr<Goal> GoalPtr;
typedef std::weak_ptr<Goal> WeakGoalPtr; typedef std::weak_ptr<Goal> WeakGoalPtr;
@ -140,6 +139,8 @@ struct Child
steady_time_point timeStarted; steady_time_point timeStarted;
}; };
class DerivationGoal;
class SubstitutionGoal;
/* The worker class. */ /* The worker class. */
class Worker class Worker
@ -167,8 +168,8 @@ private:
/* Maps used to prevent multiple instantiations of a goal for the /* Maps used to prevent multiple instantiations of a goal for the
same derivation / path. */ same derivation / path. */
WeakGoalMap derivationGoals; std::map<StorePath, std::weak_ptr<DerivationGoal>> derivationGoals;
WeakGoalMap substitutionGoals; std::map<StorePath, std::weak_ptr<SubstitutionGoal>> substitutionGoals;
/* Goals waiting for busy paths to be unlocked. */ /* Goals waiting for busy paths to be unlocked. */
WeakGoals waitingForAnyGoal; WeakGoals waitingForAnyGoal;
@ -242,7 +243,7 @@ public:
const StringSet & wantedOutputs, BuildMode buildMode = bmNormal); const StringSet & wantedOutputs, BuildMode buildMode = bmNormal);
/* substitution goal */ /* substitution goal */
GoalPtr makeSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt); std::shared_ptr<SubstitutionGoal> makeSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt);
/* Remove a dead goal. */ /* Remove a dead goal. */
void removeGoal(GoalPtr goal); void removeGoal(GoalPtr goal);
@ -305,8 +306,6 @@ public:
typedef enum {rpAccept, rpDecline, rpPostpone} HookReply; typedef enum {rpAccept, rpDecline, rpPostpone} HookReply;
class SubstitutionGoal;
/* Unless we are repairing, we don't both to test validity and just assume it, /* Unless we are repairing, we don't both to test validity and just assume it,
so the choices are `Absent` or `Valid`. */ so the choices are `Absent` or `Valid`. */
enum struct PathStatus { enum struct PathStatus {

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

@ -39,16 +39,13 @@ std::shared_ptr<DerivationGoal> Worker::makeDerivationGoalCommon(
const StringSet & wantedOutputs, const StringSet & wantedOutputs,
std::function<std::shared_ptr<DerivationGoal>()> mkDrvGoal) std::function<std::shared_ptr<DerivationGoal>()> mkDrvGoal)
{ {
WeakGoalPtr & abstract_goal_weak = derivationGoals[drvPath]; std::weak_ptr<DerivationGoal> & goal_weak = derivationGoals[drvPath];
GoalPtr abstract_goal = abstract_goal_weak.lock(); // FIXME std::shared_ptr<DerivationGoal> goal = goal_weak.lock();
std::shared_ptr<DerivationGoal> goal; if (!goal) {
if (!abstract_goal) {
goal = mkDrvGoal(); goal = mkDrvGoal();
abstract_goal_weak = goal; goal_weak = goal;
wakeUp(goal); wakeUp(goal);
} else { } else {
goal = std::dynamic_pointer_cast<DerivationGoal>(abstract_goal);
assert(goal);
goal->addWantedOutputs(wantedOutputs); goal->addWantedOutputs(wantedOutputs);
} }
return goal; return goal;
@ -73,10 +70,10 @@ std::shared_ptr<DerivationGoal> Worker::makeBasicDerivationGoal(const StorePath
} }
GoalPtr Worker::makeSubstitutionGoal(const StorePath & path, RepairFlag repair, std::optional<ContentAddress> ca) std::shared_ptr<SubstitutionGoal> Worker::makeSubstitutionGoal(const StorePath & path, RepairFlag repair, std::optional<ContentAddress> ca)
{ {
WeakGoalPtr & goal_weak = substitutionGoals[path]; std::weak_ptr<SubstitutionGoal> & goal_weak = substitutionGoals[path];
GoalPtr goal = goal_weak.lock(); // FIXME auto goal = goal_weak.lock(); // FIXME
if (!goal) { if (!goal) {
goal = std::make_shared<SubstitutionGoal>(path, *this, repair, ca); goal = std::make_shared<SubstitutionGoal>(path, *this, repair, ca);
goal_weak = goal; goal_weak = goal;
@ -85,14 +82,14 @@ GoalPtr Worker::makeSubstitutionGoal(const StorePath & path, RepairFlag repair,
return goal; return goal;
} }
template<typename G>
static void removeGoal(GoalPtr goal, WeakGoalMap & goalMap) static void removeGoal(std::shared_ptr<G> goal, std::map<StorePath, std::weak_ptr<G>> & goalMap)
{ {
/* !!! inefficient */ /* !!! inefficient */
for (WeakGoalMap::iterator i = goalMap.begin(); for (typename std::map<StorePath, std::weak_ptr<G>>::iterator i = goalMap.begin();
i != goalMap.end(); ) i != goalMap.end(); )
if (i->second.lock() == goal) { if (i->second.lock() == goal) {
WeakGoalMap::iterator j = i; ++j; typename std::map<StorePath, std::weak_ptr<G>>::iterator j = i; ++j;
goalMap.erase(i); goalMap.erase(i);
i = j; i = j;
} }
@ -102,8 +99,12 @@ static void removeGoal(GoalPtr goal, WeakGoalMap & goalMap)
void Worker::removeGoal(GoalPtr goal) void Worker::removeGoal(GoalPtr goal)
{ {
nix::removeGoal(goal, derivationGoals); if (auto drvGoal = std::dynamic_pointer_cast<DerivationGoal>(goal))
nix::removeGoal(goal, substitutionGoals); nix::removeGoal(drvGoal, derivationGoals);
else if (auto subGoal = std::dynamic_pointer_cast<SubstitutionGoal>(goal))
nix::removeGoal(subGoal, substitutionGoals);
else
assert(false);
if (topGoals.find(goal) != topGoals.end()) { if (topGoals.find(goal) != topGoals.end()) {
topGoals.erase(goal); topGoals.erase(goal);
/* If a top-level goal failed, then kill all other goals /* If a top-level goal failed, then kill all other goals