Skip to content

Commit

Permalink
Cleanups
Browse files Browse the repository at this point in the history
  • Loading branch information
@edolstra
edolstra committed Jun 19, 2024
1 parent f6cbd6a commit 6103246
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 83 deletions.
25 changes: 10 additions & 15 deletions src/libexpr/eval-inline.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,18 @@ void EvalState::forceValue(Value & v, const PosIdx pos)
{
auto type = v.internalType.load(std::memory_order_acquire);

if (isFinished(type))
goto done;

if (type == tThunk) {
try {
if (!v.internalType.compare_exchange_strong(type, tPending, std::memory_order_acquire, std::memory_order_acquire)) {
if (type == tPending || type == tAwaited) {
waitOnThunk(v, type == tAwaited);
goto done;
}
if (type != tThunk && type != tPending && type != tAwaited)
// FIXME: tFailed
return;
if (isFinished(type))
goto done;
printError("NO LONGER THUNK %x %d", this, type);
abort();
}
Expand Down Expand Up @@ -131,9 +133,8 @@ void EvalState::forceValue(Value & v, const PosIdx pos)
waitOnThunk(v, type == tAwaited);
goto done;
}
if (type != tThunk && type != tPending && type != tAwaited)
// FIXME: tFailed
return;
if (isFinished(type))
goto done;
printError("NO LONGER APP %x %d", this, type);
abort();
}
Expand All @@ -144,17 +145,11 @@ void EvalState::forceValue(Value & v, const PosIdx pos)
}
}
else if (type == tPending || type == tAwaited)
waitOnThunk(v, type == tAwaited);
else if (type == tFailed)
std::rethrow_exception(v.payload.failed->ex);
type = waitOnThunk(v, type == tAwaited);

// FIXME: remove
done:
auto type2 = v.internalType.load(std::memory_order_acquire);
if (!(type2 != tThunk && type2 != tApp && type2 != tPending && type2 != tAwaited)) {
printError("THUNK NOT FORCED %x %s %d", this, showType(v), type);
abort();
}
if (type == tFailed)
std::rethrow_exception(v.payload.failed->ex);
}


Expand Down
21 changes: 10 additions & 11 deletions src/libexpr/eval.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,12 @@ PosIdx Value::determinePos(const PosIdx pos) const
bool Value::isTrivial() const
{
return
internalType != tApp
&& internalType != tPrimOpApp
&& (internalType != tThunk
|| (dynamic_cast<ExprAttrs *>(payload.thunk.expr)
&& ((ExprAttrs *) payload.thunk.expr)->dynamicAttrs.empty())
|| dynamic_cast<ExprLambda *>(payload.thunk.expr)
|| dynamic_cast<ExprList *>(payload.thunk.expr));
isFinished()
|| (internalType == tThunk
&& ((dynamic_cast<ExprAttrs *>(payload.thunk.expr)
&& ((ExprAttrs *) payload.thunk.expr)->dynamicAttrs.empty())
|| dynamic_cast<ExprLambda *>(payload.thunk.expr)
|| dynamic_cast<ExprList *>(payload.thunk.expr)));
}


Expand Down Expand Up @@ -568,7 +567,7 @@ void printStaticEnvBindings(const SymbolTable & st, const StaticEnv & se)
// just for the current level of Env, not the whole chain.
void printWithBindings(const SymbolTable & st, const Env & env)
{
if (!env.values[0]->isThunk()) {
if (env.values[0]->isFinished()) {
std::cout << "with: ";
std::cout << ANSI_MAGENTA;
auto j = env.values[0]->attrs()->begin();
Expand Down Expand Up @@ -624,7 +623,7 @@ void mapStaticEnvBindings(const SymbolTable & st, const StaticEnv & se, const En
if (env.up && se.up) {
mapStaticEnvBindings(st, *se.up, *env.up, vm);

if (se.isWith && !env.values[0]->isThunk()) {
if (se.isWith && env.values[0]->isFinished()) {
// add 'with' bindings.
for (auto & j : *env.values[0]->attrs())
vm.insert_or_assign(std::string(st[j.name]), j.value);
Expand Down Expand Up @@ -1047,7 +1046,7 @@ void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial)

{
auto cache(fileEvalCache.lock());
auto [i, inserted] = cache->emplace(*resolvedPath, Value());
auto [i, inserted] = cache->try_emplace(*resolvedPath);
if (inserted)
i->second.mkThunk(nullptr, &expr);
vExpr = &i->second;
Expand Down Expand Up @@ -2051,7 +2050,7 @@ void EvalState::forceValueDeep(Value & v)
for (auto & i : *v.attrs())
try {
// If the value is a thunk, we're evaling. Otherwise no trace necessary.
auto dts = debugRepl && i.value->isThunk()
auto dts = debugRepl && i.value->internalType == tThunk
? makeDebugTraceStacker(*this, *i.value->payload.thunk.expr, *i.value->payload.thunk.env, positions[i.pos],
"while evaluating the attribute '%1%'", symbols[i.name])
: nullptr;
Expand Down
5 changes: 3 additions & 2 deletions src/libexpr/eval.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,9 +443,10 @@ public:

/**
* Given a thunk that was observed to be in the pending or awaited
* state, wait for it to finish.
* state, wait for it to finish. Returns the new type of the
* value.
*/
void waitOnThunk(Value & v, bool awaited);
InternalType waitOnThunk(Value & v, bool awaited);

void tryFixupBlackHolePos(Value & v, PosIdx pos);

Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/flake/flake.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ static std::tuple<StorePath, FlakeRef, FlakeRef> fetchOrSubstituteTree(

static void forceTrivialValue(EvalState & state, Value & value, const PosIdx pos)
{
if (value.isThunk() && value.isTrivial())
if (!value.isFinished() && value.isTrivial())
state.forceValue(value, pos);
}

Expand Down
23 changes: 10 additions & 13 deletions src/libexpr/parallel-eval.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ static Sync<WaiterDomain> & getWaiterDomain(Value & v)

std::atomic<uint64_t> nrThunksAwaited, nrThunksAwaitedSlow, usWaiting, currentlyWaiting, maxWaiting;

void EvalState::waitOnThunk(Value & v, bool awaited)
InternalType EvalState::waitOnThunk(Value & v, bool awaited)
{
nrThunksAwaited++;

Expand All @@ -30,23 +30,23 @@ void EvalState::waitOnThunk(Value & v, bool awaited)
holding the domain lock. */
auto type = v.internalType.load(std::memory_order_acquire);

/* If the value has been finalized in the meantime (i.e is no
/* If the value has been finalized in the meantime (i.e. is no
longer pending), we're done. */
if (type != tAwaited) {
debug("VALUE DONE RIGHT AWAY 2 %x", &v);
assert(type != tThunk && type != tApp && type != tPending && type != tAwaited);
return;
assert(isFinished(type));
return type;
}
} else {
/* Mark this value as being waited on. */
auto type = tPending;
if (!v.internalType.compare_exchange_strong(type, tAwaited, std::memory_order_relaxed, std::memory_order_acquire)) {
/* If the value has been finalized in the meantime (i.e is
/* If the value has been finalized in the meantime (i.e. is
no longer pending), we're done. */
if (type != tAwaited) {
debug("VALUE DONE RIGHT AWAY %x", &v);
assert(type != tThunk && type != tApp && type != tPending && type != tAwaited);
return;
assert(isFinished(type));
return type;
}
/* The value was already in the "waited on" state, so we're
not the only thread waiting on it. */
Expand All @@ -55,6 +55,7 @@ void EvalState::waitOnThunk(Value & v, bool awaited)
debug("PENDING -> AWAITED %x", &v);
}

/* Wait for another thread to finish this value. */
debug("AWAIT %x", &v);

nrThunksAwaitedSlow++;
Expand All @@ -68,15 +69,11 @@ void EvalState::waitOnThunk(Value & v, bool awaited)
debug("WAKEUP %x", &v);
auto type = v.internalType.load(std::memory_order_acquire);
if (type != tAwaited) {
if (type == tFailed) {
currentlyWaiting--;
std::rethrow_exception(v.payload.failed->ex);
}
assert(type != tThunk && type != tApp && type != tPending && type != tAwaited);
assert(isFinished(type));
auto now2 = std::chrono::steady_clock::now();
usWaiting += std::chrono::duration_cast<std::chrono::microseconds>(now2 - now1).count();
currentlyWaiting--;
return;
return type;
}
printError("SPURIOUS %s", &v);
}
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/print.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ class Printer
output << "«potential infinite recursion»";
if (options.ansiColors)
output << ANSI_NORMAL;
} else if (v.isThunk() || v.isApp()) {
} else if (!v.isFinished()) {
if (options.ansiColors)
output << ANSI_MAGENTA;
output << "«thunk»";
Expand Down
92 changes: 52 additions & 40 deletions src/libexpr/value.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,45 @@ namespace nix {
struct Value;
class BindingsBuilder;


typedef enum {
/* Unfinished values. */
tUninitialized = 0,
tInt = 1,
tBool = 2,
tString = 3,
tPath = 4,
tNull = 5,
tAttrs = 6,
tList1 = 7,
tList2 = 8,
tListN = 9,
tThunk = 10,
tApp = 11,
tLambda = 12,
tPrimOp = 13,
tPrimOpApp = 14,
tExternal = 15,
tFloat = 16,
tPending = 17,
tAwaited = 18,
tFailed = 19,
tThunk,
tApp,
tPending,
tAwaited,

/* Finished values. */
tInt = 32, // Do not move tInt (see isFinished()).
tBool,
tString,
tPath,
tNull,
tAttrs,
tList1,
tList2,
tListN,
tLambda,
tPrimOp,
tPrimOpApp,
tExternal,
tFloat,
tFailed,
} InternalType;

/**
* Return true if `type` denotes a "finished" value, i.e. a weak-head
* normal form.
*
* Note that tPrimOpApp is considered "finished" because it represents
* a primop call with an incomplete number of arguments, and therefore
* cannot be evaluated further.
*/
inline bool isFinished(InternalType type)
{
return type >= tInt;
}

/**
* This type abstracts over all actual value types in the language,
* grouping together implementation details like tList*, different function
Expand Down Expand Up @@ -172,7 +187,6 @@ private:
std::atomic<InternalType> internalType{tUninitialized};

friend std::string showType(const Value & v);

friend class EvalState;

public:
Expand All @@ -185,14 +199,14 @@ public:
{ *this = v; }

/**
* Copy a value. This is not allowed to be a thunk.
* Copy a value. This is not allowed to be a thunk to avoid
* accidental work duplication.
*/
Value & operator =(const Value & v)
{
auto type = v.internalType.load(std::memory_order_acquire);
debug("ASSIGN %x %d %d", this, internalType, type);
//assert(type != tThunk && type != tApp && type != tPending && type != tAwaited);
if (!(type != tThunk && type != tApp && type != tPending && type != tAwaited)) {
//debug("ASSIGN %x %d %d", this, internalType, type);
if (!nix::isFinished(type)) {
printError("UNEXPECTED TYPE %x %s", this, showType(v));
abort();
}
Expand All @@ -202,13 +216,11 @@ public:

void print(EvalState &state, std::ostream &str, PrintOptions options = PrintOptions {});

// Functions needed to distinguish the type
// These should be removed eventually, by putting the functionality that's
// needed by callers into methods of this type
inline bool isFinished() const
{
return nix::isFinished(internalType.load(std::memory_order_acquire));
}

// type() == nThunk
inline bool isThunk() const { return internalType == tThunk; };
inline bool isApp() const { return internalType == tApp; };
inline bool isBlackhole() const;

// type() == nFunction
Expand Down Expand Up @@ -327,17 +339,14 @@ public:
debug("FINISH %x %d %d", this, internalType, newType);
payload = newPayload;

// TODO: need a barrier here to ensure the payload of the
// value is updated before the type field.

auto oldType = internalType.exchange(newType, std::memory_order_release);

if (oldType == tPending)
// Nothing to do; no thread is waiting on this thunk.
;
else if (oldType == tUninitialized)
if (oldType == tUninitialized)
// Uninitialized value; nothing to do.
;
else if (oldType == tPending)
// Nothing to do; no thread is waiting on this thunk.
;
else if (oldType == tAwaited)
// Slow path: wake up the threads that are waiting on this
// thunk.
Expand Down Expand Up @@ -516,8 +525,11 @@ public:

/**
* Check whether forcing this value requires a trivial amount of
* computation. In particular, function applications are
* non-trivial.
* computation. A value is trivial if it's finished or if it's a
* thunk whose expression is an attrset with no dynamic
* attributes, a lambda or a list. Note that it's up to the caller
* to check whether the members of those attrsets or lists must be
* trivial.
*/
bool isTrivial() const;

Expand Down

0 comments on commit 6103246

Please sign in to comment.