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.

src/libexpr/eval.cc

@@ -38,6 +38,7 @@
 
 #include <nlohmann/json.hpp>
 #include <boost/container/small_vector.hpp>
+#include <boost/unordered/concurrent_flat_map.hpp>
 
 #include "nix/util/strings-inline.hh"
 
@@ -192,6 +193,27 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env)
 
 static constexpr size_t BASE_ENV_SIZE = 128;
 
+struct EvalState::SrcToStore
+{
+    boost::concurrent_flat_map<SourcePath, StorePath> inner;
+};
+
+struct EvalState::ImportResolutionCache
+{
+    boost::concurrent_flat_map<SourcePath, SourcePath> inner;
+};
+
+struct EvalState::FileEvalCache
+{
+    boost::concurrent_flat_map<
+        SourcePath,
+        Value *,
+        std::hash<SourcePath>,
+        std::equal_to<SourcePath>,
+        traceable_allocator<std::pair<const SourcePath, Value *>>>
+        inner;
+};
+
 EvalState::EvalState(
     const LookupPath & lookupPathFromArguments,
     ref<Store> store,
@@ -264,6 +286,9 @@ EvalState::EvalState(
     , debugRepl(nullptr)
     , debugStop(false)
     , trylevel(0)
+    , srcToStore(make_ref<SrcToStore>())
+    , importResolutionCache(make_ref<ImportResolutionCache>())
+    , fileEvalCache(make_ref<FileEvalCache>())
     , regexCache(makeRegexCache())
 #if NIX_USE_BOEHMGC
     , valueAllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
@@ -1031,63 +1056,85 @@ Value * ExprPath::maybeThunk(EvalState & state, Env & env)
     return &v;
 }
 
-void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial)
+/**
+ * A helper `Expr` class to lets us parse and evaluate Nix expressions
+ * from a thunk, ensuring that every file is parsed/evaluated only
+ * once (via the thunk stored in `EvalState::fileEvalCache`).
+ */
+struct ExprParseFile : Expr
 {
-    FileEvalCache::iterator i;
+    SourcePath & path;
-    if ((i = fileEvalCache.find(path)) != fileEvalCache.end()) {
+    bool mustBeTrivial;
-        v = i->second;
+
-        return;
+    ExprParseFile(SourcePath & path, bool mustBeTrivial)
+        : path(path)
+        , mustBeTrivial(mustBeTrivial)
+    {
     }
 
-    auto resolvedPath = resolveExprPath(path);
+    void eval(EvalState & state, Env & env, Value & v) override
-    if ((i = fileEvalCache.find(resolvedPath)) != fileEvalCache.end()) {
+    {
-        v = i->second;
+        printTalkative("evaluating file '%s'", path);
-        return;
-    }
 
-    printTalkative("evaluating file '%1%'", resolvedPath);
+        auto e = state.parseExprFromFile(path);
-    Expr * e = nullptr;
-
-    auto j = fileParseCache.find(resolvedPath);
-    if (j != fileParseCache.end())
-        e = j->second;
-
-    if (!e)
-        e = parseExprFromFile(resolvedPath);
-
-    fileParseCache.emplace(resolvedPath, e);
 
         try {
-        auto dts = debugRepl ? makeDebugTraceStacker(
+            auto dts =
-                                   *this,
+                state.debugRepl
-                                   *e,
+                    ? makeDebugTraceStacker(
-                                   this->baseEnv,
+                          state, *e, state.baseEnv, e->getPos(), "while evaluating the file '%s':", path.to_string())
-                                   e->getPos(),
-                                   "while evaluating the file '%1%':",
-                                   resolvedPath.to_string())
                     : nullptr;
 
             // Enforce that 'flake.nix' is a direct attrset, not a
             // computation.
             if (mustBeTrivial && !(dynamic_cast<ExprAttrs *>(e)))
-            error<EvalError>("file '%s' must be an attribute set", path).debugThrow();
+                state.error<EvalError>("file '%s' must be an attribute set", path).debugThrow();
-        eval(e, v);
+
+            state.eval(e, v);
         } catch (Error & e) {
-        addErrorTrace(e, "while evaluating the file '%1%':", resolvedPath.to_string());
+            state.addErrorTrace(e, "while evaluating the file '%s':", path.to_string());
             throw;
         }
+    }
+};
 
-    fileEvalCache.emplace(resolvedPath, v);
+void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial)
-    if (path != resolvedPath)
+{
-        fileEvalCache.emplace(path, v);
+    auto resolvedPath = getConcurrent(importResolutionCache->inner, path);
+
+    if (!resolvedPath) {
+        resolvedPath = resolveExprPath(path);
+        importResolutionCache->inner.emplace(path, *resolvedPath);
+    }
+
+    if (auto v2 = getConcurrent(fileEvalCache->inner, *resolvedPath)) {
+        forceValue(**v2, noPos);
+        v = **v2;
+        return;
+    }
+
+    Value * vExpr;
+    ExprParseFile expr{*resolvedPath, mustBeTrivial};
+
+    fileEvalCache->inner.try_emplace_and_cvisit(
+        *resolvedPath,
+        nullptr,
+        [&](auto & i) {
+            vExpr = allocValue();
+            vExpr->mkThunk(&baseEnv, &expr);
+            i.second = vExpr;
+        },
+        [&](auto & i) { vExpr = i.second; });
+
+    forceValue(*vExpr, noPos);
+
+    v = *vExpr;
 }
 
 void EvalState::resetFileCache()
 {
-    fileEvalCache.clear();
+    fileEvalCache->inner.clear();
-    fileEvalCache.rehash(0);
+    fileEvalCache->inner.rehash(0);
-    fileParseCache.clear();
-    fileParseCache.rehash(0);
     inputCache->clear();
 }
 
@@ -2372,9 +2419,10 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat
     if (nix::isDerivation(path.path.abs()))
         error<EvalError>("file names are not allowed to end in '%1%'", drvExtension).debugThrow();
 
-    std::optional<StorePath> dstPath;
+    auto dstPathCached = getConcurrent(srcToStore->inner, path);
-    if (!srcToStore.cvisit(path, [&dstPath](const auto & kv) { dstPath.emplace(kv.second); })) {
+
-        dstPath.emplace(fetchToStore(
+    auto dstPath = dstPathCached ? *dstPathCached : [&]() {
+        auto dstPath = fetchToStore(
             fetchSettings,
             *store,
             path.resolveSymlinks(SymlinkResolution::Ancestors),
@@ -2382,14 +2430,15 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat
             path.baseName(),
             ContentAddressMethod::Raw::NixArchive,
             nullptr,
-            repair));
+            repair);
-        allowPath(*dstPath);
+        allowPath(dstPath);
-        srcToStore.try_emplace(path, *dstPath);
+        srcToStore->inner.try_emplace(path, dstPath);
-        printMsg(lvlChatty, "copied source '%1%' -> '%2%'", path, store->printStorePath(*dstPath));
+        printMsg(lvlChatty, "copied source '%1%' -> '%2%'", path, store->printStorePath(dstPath));
-    }
+        return dstPath;
+    }();
 
-    context.insert(NixStringContextElem::Opaque{.path = *dstPath});
+    context.insert(NixStringContextElem::Opaque{.path = dstPath});
-    return *dstPath;
+    return dstPath;
 }
 
 SourcePath EvalState::coerceToPath(const PosIdx pos, Value & v, NixStringContext & context, std::string_view errorCtx)

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

@@ -20,7 +20,6 @@
 // For `NIX_USE_BOEHMGC`, and if that's set, `GC_THREADS`
 #include "nix/expr/config.hh"
 
-#include <boost/unordered/concurrent_flat_map.hpp>
 #include <boost/unordered/unordered_flat_map.hpp>
 #include <map>
 #include <optional>
@@ -412,31 +411,21 @@ private:
 
     /* Cache for calls to addToStore(); maps source paths to the store
        paths. */
-    boost::concurrent_flat_map<SourcePath, StorePath, std::hash<SourcePath>> srcToStore;
+    struct SrcToStore;
+    ref<SrcToStore> srcToStore;
 
     /**
-     * A cache from path names to parse trees.
+     * A cache that maps paths to "resolved" paths for importing Nix
+     * expressions, i.e. `/foo` to `/foo/default.nix`.
      */
-    typedef boost::unordered_flat_map<
+    struct ImportResolutionCache;
-        SourcePath,
+    ref<ImportResolutionCache> importResolutionCache;
-        Expr *,
-        std::hash<SourcePath>,
-        std::equal_to<SourcePath>,
-        traceable_allocator<std::pair<const SourcePath, Expr *>>>
-        FileParseCache;
-    FileParseCache fileParseCache;
 
     /**
-     * A cache from path names to values.
+     * A cache from resolved paths to values.
      */
-    typedef boost::unordered_flat_map<
+    struct FileEvalCache;
-        SourcePath,
+    ref<FileEvalCache> fileEvalCache;
-        Value,
-        std::hash<SourcePath>,
-        std::equal_to<SourcePath>,
-        traceable_allocator<std::pair<const SourcePath, Value>>>
-        FileEvalCache;
-    FileEvalCache fileEvalCache;
 
     /**
      * Associate source positions of certain AST nodes with their preceding doc comment, if they have one.