Skip to content

Commit 947a480

Browse files
committed
Ensure that files are parsed/evaluated only once
When doing multithreaded evaluation, we want to ensure that any Nix file is parsed and evaluated only once. The easiest way to do this is to rely on thunks, since those ensure locking in the multithreaded evaluator. `fileEvalCache` is now a mapping from `SourcePath` to a `Value *`. The value is initially a thunk (pointing to a `ExprParseFile` helper object) that can be forced to parse and evaluate the file. So a subsequent thread requesting the same file will see a thunk that is possibly locked and wait for it. The parser cache is gone since it's no longer needed. However, there is a new `importResolutionCache` that maps `SourcePath`s to `SourcePath`s (e.g. `/foo` to `/foo/default.nix`). Previously we put multiple entries in `fileEvalCache`, which was ugly and could result in work duplication.
1 parent 1643e74 commit 947a480

File tree

2 files changed

+100
-62
lines changed

2 files changed

+100
-62
lines changed

src/libexpr/eval.cc

Lines changed: 91 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838

3939
#include <nlohmann/json.hpp>
4040
#include <boost/container/small_vector.hpp>
41+
#include <boost/unordered/concurrent_flat_map.hpp>
4142

4243
#include "nix/util/strings-inline.hh"
4344

@@ -192,6 +193,27 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env)
192193

193194
static constexpr size_t BASE_ENV_SIZE = 128;
194195

196+
struct EvalState::SrcToStore
197+
{
198+
boost::concurrent_flat_map<SourcePath, StorePath> inner;
199+
};
200+
201+
struct EvalState::ImportResolutionCache
202+
{
203+
boost::concurrent_flat_map<SourcePath, SourcePath> inner;
204+
};
205+
206+
struct EvalState::FileEvalCache
207+
{
208+
boost::concurrent_flat_map<
209+
SourcePath,
210+
Value *,
211+
std::hash<SourcePath>,
212+
std::equal_to<SourcePath>,
213+
traceable_allocator<std::pair<const SourcePath, Value *>>>
214+
inner;
215+
};
216+
195217
EvalState::EvalState(
196218
const LookupPath & lookupPathFromArguments,
197219
ref<Store> store,
@@ -265,6 +287,9 @@ EvalState::EvalState(
265287
, debugRepl(nullptr)
266288
, debugStop(false)
267289
, trylevel(0)
290+
, srcToStore(make_ref<SrcToStore>())
291+
, importResolutionCache(make_ref<ImportResolutionCache>())
292+
, fileEvalCache(make_ref<FileEvalCache>())
268293
, regexCache(makeRegexCache())
269294
#if NIX_USE_BOEHMGC
270295
, valueAllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
@@ -1036,61 +1061,84 @@ Value * ExprPath::maybeThunk(EvalState & state, Env & env)
10361061
return &v;
10371062
}
10381063

1039-
void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial)
1064+
/**
1065+
* A helper `Expr` class to lets us parse and evaluate Nix expressions
1066+
* from a thunk, ensuring that every file is parsed/evaluated only
1067+
* once (via the thunk stored in `EvalState::fileEvalCache`).
1068+
*/
1069+
struct ExprParseFile : Expr
10401070
{
1041-
FileEvalCache::iterator i;
1042-
if ((i = fileEvalCache.find(path)) != fileEvalCache.end()) {
1043-
v = i->second;
1044-
return;
1045-
}
1071+
SourcePath & path;
1072+
bool mustBeTrivial;
10461073

1047-
auto resolvedPath = resolveExprPath(path);
1048-
if ((i = fileEvalCache.find(resolvedPath)) != fileEvalCache.end()) {
1049-
v = i->second;
1050-
return;
1074+
ExprParseFile(SourcePath & path, bool mustBeTrivial)
1075+
: path(path)
1076+
, mustBeTrivial(mustBeTrivial)
1077+
{
10511078
}
10521079

1053-
printTalkative("evaluating file '%1%'", resolvedPath);
1054-
Expr * e = nullptr;
1080+
void eval(EvalState & state, Env & env, Value & v) override
1081+
{
1082+
printTalkative("evaluating file '%s'", path);
10551083

1056-
auto j = fileParseCache.find(resolvedPath);
1057-
if (j != fileParseCache.end())
1058-
e = j->second;
1084+
auto e = state.parseExprFromFile(path);
10591085

1060-
if (!e)
1061-
e = parseExprFromFile(resolvedPath);
1086+
try {
1087+
auto dts =
1088+
state.debugRepl
1089+
? makeDebugTraceStacker(
1090+
state, *e, state.baseEnv, e->getPos(), "while evaluating the file '%s':", path.to_string())
1091+
: nullptr;
1092+
1093+
// Enforce that 'flake.nix' is a direct attrset, not a
1094+
// computation.
1095+
if (mustBeTrivial && !(dynamic_cast<ExprAttrs *>(e)))
1096+
state.error<EvalError>("file '%s' must be an attribute set", path).debugThrow();
1097+
1098+
state.eval(e, v);
1099+
} catch (Error & e) {
1100+
state.addErrorTrace(e, "while evaluating the file '%s':", path.to_string());
1101+
throw;
1102+
}
1103+
}
1104+
};
10621105

1063-
fileParseCache.emplace(resolvedPath, e);
1106+
void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial)
1107+
{
1108+
auto resolvedPath = getConcurrent(importResolutionCache->inner, path);
10641109

1065-
try {
1066-
auto dts = debugRepl ? makeDebugTraceStacker(
1067-
*this,
1068-
*e,
1069-
this->baseEnv,
1070-
e->getPos(),
1071-
"while evaluating the file '%1%':",
1072-
resolvedPath.to_string())
1073-
: nullptr;
1074-
1075-
// Enforce that 'flake.nix' is a direct attrset, not a
1076-
// computation.
1077-
if (mustBeTrivial && !(dynamic_cast<ExprAttrs *>(e)))
1078-
error<EvalError>("file '%s' must be an attribute set", path).debugThrow();
1079-
eval(e, v);
1080-
} catch (Error & e) {
1081-
addErrorTrace(e, "while evaluating the file '%1%':", resolvedPath.to_string());
1082-
throw;
1110+
if (!resolvedPath) {
1111+
resolvedPath = resolveExprPath(path);
1112+
importResolutionCache->inner.emplace(path, *resolvedPath);
1113+
}
1114+
1115+
if (auto v2 = getConcurrent(fileEvalCache->inner, *resolvedPath)) {
1116+
forceValue(**v2, noPos);
1117+
v = **v2;
1118+
return;
10831119
}
10841120

1085-
fileEvalCache.emplace(resolvedPath, v);
1086-
if (path != resolvedPath)
1087-
fileEvalCache.emplace(path, v);
1121+
Value * vExpr;
1122+
ExprParseFile expr{*resolvedPath, mustBeTrivial};
1123+
1124+
fileEvalCache->inner.try_emplace_and_cvisit(
1125+
*resolvedPath,
1126+
nullptr,
1127+
[&](auto & i) {
1128+
vExpr = allocValue();
1129+
vExpr->mkThunk(&baseEnv, &expr);
1130+
i.second = vExpr;
1131+
},
1132+
[&](auto & i) { vExpr = i.second; });
1133+
1134+
forceValue(*vExpr, noPos);
1135+
1136+
v = *vExpr;
10881137
}
10891138

10901139
void EvalState::resetFileCache()
10911140
{
1092-
fileEvalCache.clear();
1093-
fileParseCache.clear();
1141+
fileEvalCache->inner.clear();
10941142
inputCache->clear();
10951143
}
10961144

@@ -2375,7 +2423,7 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat
23752423
if (nix::isDerivation(path.path.abs()))
23762424
error<EvalError>("file names are not allowed to end in '%1%'", drvExtension).debugThrow();
23772425

2378-
auto dstPathCached = get(*srcToStore.lock(), path);
2426+
auto dstPathCached = getConcurrent(srcToStore->inner, path);
23792427

23802428
auto dstPath = dstPathCached ? *dstPathCached : [&]() {
23812429
auto dstPath = fetchToStore(
@@ -2388,7 +2436,7 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat
23882436
nullptr,
23892437
repair);
23902438
allowPath(dstPath);
2391-
srcToStore.lock()->try_emplace(path, dstPath);
2439+
srcToStore->inner.try_emplace(path, dstPath);
23922440
printMsg(lvlChatty, "copied source '%1%' -> '%2%'", path, store->printStorePath(dstPath));
23932441
return dstPath;
23942442
}();

src/libexpr/include/nix/expr/eval.hh

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -438,31 +438,21 @@ private:
438438

439439
/* Cache for calls to addToStore(); maps source paths to the store
440440
paths. */
441-
Sync<std::unordered_map<SourcePath, StorePath>> srcToStore;
441+
struct SrcToStore;
442+
ref<SrcToStore> srcToStore;
442443

443444
/**
444-
* A cache from path names to parse trees.
445+
* A cache that maps paths to "resolved" paths for importing Nix
446+
* expressions, i.e. `/foo` to `/foo/default.nix`.
445447
*/
446-
typedef std::unordered_map<
447-
SourcePath,
448-
Expr *,
449-
std::hash<SourcePath>,
450-
std::equal_to<SourcePath>,
451-
traceable_allocator<std::pair<const SourcePath, Expr *>>>
452-
FileParseCache;
453-
FileParseCache fileParseCache;
448+
struct ImportResolutionCache;
449+
ref<ImportResolutionCache> importResolutionCache;
454450

455451
/**
456-
* A cache from path names to values.
452+
* A cache from resolved paths to values.
457453
*/
458-
typedef std::unordered_map<
459-
SourcePath,
460-
Value,
461-
std::hash<SourcePath>,
462-
std::equal_to<SourcePath>,
463-
traceable_allocator<std::pair<const SourcePath, Value>>>
464-
FileEvalCache;
465-
FileEvalCache fileEvalCache;
454+
struct FileEvalCache;
455+
ref<FileEvalCache> fileEvalCache;
466456

467457
/**
468458
* Associate source positions of certain AST nodes with their preceding doc comment, if they have one.

0 commit comments

Comments
 (0)