diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 43e4c3643ab..bf55a9c9cca 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -38,7 +38,6 @@ #include #include -#include #include "nix/util/strings-inline.hh" @@ -265,9 +264,6 @@ EvalState::EvalState( , debugRepl(nullptr) , debugStop(false) , trylevel(0) - , srcToStore(make_ref()) - , importResolutionCache(make_ref()) - , fileEvalCache(make_ref()) , regexCache(makeRegexCache()) #if NIX_USE_BOEHMGC , valueAllocCache(std::allocate_shared(traceable_allocator(), nullptr)) @@ -1030,85 +1026,63 @@ Value * ExprPath::maybeThunk(EvalState & state, Env & env) return &v; } -/** - * 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 -{ - SourcePath & path; - bool mustBeTrivial; - - ExprParseFile(SourcePath & path, bool mustBeTrivial) - : path(path) - , mustBeTrivial(mustBeTrivial) - { - } - - void eval(EvalState & state, Env & env, Value & v) override - { - printTalkative("evaluating file '%s'", path); - - auto e = state.parseExprFromFile(path); - - try { - auto dts = - state.debugRepl - ? makeDebugTraceStacker( - state, *e, state.baseEnv, e->getPos(), "while evaluating the file '%s':", path.to_string()) - : nullptr; - - // Enforce that 'flake.nix' is a direct attrset, not a - // computation. - if (mustBeTrivial && !(dynamic_cast(e))) - state.error("file '%s' must be an attribute set", path).debugThrow(); - - state.eval(e, v); - } catch (Error & e) { - state.addErrorTrace(e, "while evaluating the file '%s':", path.to_string()); - throw; - } - } -}; - void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial) { - auto resolvedPath = getConcurrent(*importResolutionCache, path); - - if (!resolvedPath) { - resolvedPath = resolveExprPath(path); - importResolutionCache->emplace(path, *resolvedPath); + FileEvalCache::iterator i; + if ((i = fileEvalCache.find(path)) != fileEvalCache.end()) { + v = i->second; + return; } - if (auto v2 = getConcurrent(*fileEvalCache, *resolvedPath)) { - forceValue(**v2, noPos); - v = **v2; + auto resolvedPath = resolveExprPath(path); + if ((i = fileEvalCache.find(resolvedPath)) != fileEvalCache.end()) { + v = i->second; return; } - Value * vExpr; - ExprParseFile expr{*resolvedPath, mustBeTrivial}; + printTalkative("evaluating file '%1%'", resolvedPath); + Expr * e = nullptr; - fileEvalCache->try_emplace_and_cvisit( - *resolvedPath, - nullptr, - [&](auto & i) { - vExpr = allocValue(); - vExpr->mkThunk(&baseEnv, &expr); - i.second = vExpr; - }, - [&](auto & i) { vExpr = i.second; }); + auto j = fileParseCache.find(resolvedPath); + if (j != fileParseCache.end()) + e = j->second; + + if (!e) + e = parseExprFromFile(resolvedPath); + + fileParseCache.emplace(resolvedPath, e); - forceValue(*vExpr, noPos); + try { + auto dts = debugRepl ? makeDebugTraceStacker( + *this, + *e, + this->baseEnv, + 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(e))) + error("file '%s' must be an attribute set", path).debugThrow(); + eval(e, v); + } catch (Error & e) { + addErrorTrace(e, "while evaluating the file '%1%':", resolvedPath.to_string()); + throw; + } - v = *vExpr; + fileEvalCache.emplace(resolvedPath, v); + if (path != resolvedPath) + fileEvalCache.emplace(path, v); } void EvalState::resetFileCache() { - importResolutionCache->clear(); - fileEvalCache->clear(); + fileEvalCache.clear(); + fileEvalCache.rehash(0); + fileParseCache.clear(); + fileParseCache.rehash(0); inputCache->clear(); } @@ -2427,10 +2401,9 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat if (nix::isDerivation(path.path.abs())) error("file names are not allowed to end in '%1%'", drvExtension).debugThrow(); - auto dstPathCached = getConcurrent(*srcToStore, path); - - auto dstPath = dstPathCached ? *dstPathCached : [&]() { - auto dstPath = fetchToStore( + std::optional dstPath; + if (!srcToStore.cvisit(path, [&dstPath](const auto & kv) { dstPath.emplace(kv.second); })) { + dstPath.emplace(fetchToStore( fetchSettings, *store, path.resolveSymlinks(SymlinkResolution::Ancestors), @@ -2438,15 +2411,14 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat path.baseName(), ContentAddressMethod::Raw::NixArchive, nullptr, - repair); - allowPath(dstPath); - srcToStore->try_emplace(path, dstPath); - printMsg(lvlChatty, "copied source '%1%' -> '%2%'", path, store->printStorePath(dstPath)); - return dstPath; - }(); + repair)); + allowPath(*dstPath); + srcToStore.try_emplace(path, *dstPath); + printMsg(lvlChatty, "copied source '%1%' -> '%2%'", path, store->printStorePath(*dstPath)); + } - context.insert(NixStringContextElem::Opaque{.path = dstPath}); - return dstPath; + context.insert(NixStringContextElem::Opaque{.path = *dstPath}); + return *dstPath; } SourcePath EvalState::coerceToPath(const PosIdx pos, Value & v, NixStringContext & context, std::string_view errorCtx) diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh index 8f7a0ec327b..64f52858106 100644 --- a/src/libexpr/include/nix/expr/eval.hh +++ b/src/libexpr/include/nix/expr/eval.hh @@ -20,9 +20,8 @@ // For `NIX_USE_BOEHMGC`, and if that's set, `GC_THREADS` #include "nix/expr/config.hh" +#include #include -#include - #include #include #include @@ -404,30 +403,37 @@ private: /* Cache for calls to addToStore(); maps source paths to the store paths. */ - ref> srcToStore; + boost::concurrent_flat_map> srcToStore; /** - * A cache that maps paths to "resolved" paths for importing Nix - * expressions, i.e. `/foo` to `/foo/default.nix`. + * A cache from path names to parse trees. */ - ref> importResolutionCache; + typedef boost::unordered_flat_map< + SourcePath, + Expr *, + std::hash, + std::equal_to, + traceable_allocator>> + FileParseCache; + FileParseCache fileParseCache; /** - * A cache from resolved paths to values. + * A cache from path names to values. */ - ref, std::equal_to, - traceable_allocator>>> - fileEvalCache; + traceable_allocator>> + FileEvalCache; + FileEvalCache fileEvalCache; /** * Associate source positions of certain AST nodes with their preceding doc comment, if they have one. * Grouped by file. */ - boost::unordered_flat_map positionToDocComment; + boost::unordered_flat_map> positionToDocComment; LookupPath lookupPath; diff --git a/src/libfetchers/filtering-source-accessor.cc b/src/libfetchers/filtering-source-accessor.cc index a99ecacef0b..d0991ae23db 100644 --- a/src/libfetchers/filtering-source-accessor.cc +++ b/src/libfetchers/filtering-source-accessor.cc @@ -59,12 +59,12 @@ void FilteringSourceAccessor::checkAccess(const CanonPath & path) struct AllowListSourceAccessorImpl : AllowListSourceAccessor { std::set allowedPrefixes; - boost::unordered_flat_set allowedPaths; + boost::unordered_flat_set> allowedPaths; AllowListSourceAccessorImpl( ref next, std::set && allowedPrefixes, - boost::unordered_flat_set && allowedPaths, + boost::unordered_flat_set> && allowedPaths, MakeNotAllowedError && makeNotAllowedError) : AllowListSourceAccessor(SourcePath(next), std::move(makeNotAllowedError)) , allowedPrefixes(std::move(allowedPrefixes)) @@ -86,7 +86,7 @@ struct AllowListSourceAccessorImpl : AllowListSourceAccessor ref AllowListSourceAccessor::create( ref next, std::set && allowedPrefixes, - boost::unordered_flat_set && allowedPaths, + boost::unordered_flat_set> && allowedPaths, MakeNotAllowedError && makeNotAllowedError) { return make_ref( diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index a3652e5222e..4ed94a4ed62 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -817,7 +817,7 @@ struct GitSourceAccessor : SourceAccessor return toHash(*git_tree_entry_id(entry)); } - boost::unordered_flat_map lookupCache; + boost::unordered_flat_map> lookupCache; /* Recursively look up 'path' relative to the root. */ git_tree_entry * lookup(State & state, const CanonPath & path) @@ -1254,7 +1254,7 @@ GitRepoImpl::getAccessor(const WorkdirInfo & wd, bool exportIgnore, MakeNotAllow makeFSSourceAccessor(path), std::set{wd.files}, // Always allow access to the root, but not its children. - boost::unordered_flat_set{CanonPath::root}, + boost::unordered_flat_set>{CanonPath::root}, std::move(makeNotAllowedError)) .cast(); if (exportIgnore) diff --git a/src/libfetchers/include/nix/fetchers/filtering-source-accessor.hh b/src/libfetchers/include/nix/fetchers/filtering-source-accessor.hh index f8a57bfb366..1d4028be580 100644 --- a/src/libfetchers/include/nix/fetchers/filtering-source-accessor.hh +++ b/src/libfetchers/include/nix/fetchers/filtering-source-accessor.hh @@ -72,7 +72,7 @@ struct AllowListSourceAccessor : public FilteringSourceAccessor static ref create( ref next, std::set && allowedPrefixes, - boost::unordered_flat_set && allowedPaths, + boost::unordered_flat_set> && allowedPaths, MakeNotAllowedError && makeNotAllowedError); using FilteringSourceAccessor::FilteringSourceAccessor; diff --git a/src/libutil/include/nix/util/canon-path.hh b/src/libutil/include/nix/util/canon-path.hh index dd07929b4f4..334c9e33246 100644 --- a/src/libutil/include/nix/util/canon-path.hh +++ b/src/libutil/include/nix/util/canon-path.hh @@ -8,8 +8,6 @@ #include #include -#include - namespace nix { /** @@ -260,17 +258,11 @@ public: */ std::string makeRelative(const CanonPath & path) const; - friend std::size_t hash_value(const CanonPath &); + friend struct std::hash; }; std::ostream & operator<<(std::ostream & stream, const CanonPath & path); -inline std::size_t hash_value(const CanonPath & path) -{ - boost::hash hasher; - return hasher(path.path); -} - } // namespace nix template<> @@ -278,8 +270,8 @@ struct std::hash { using is_avalanching = std::true_type; - std::size_t operator()(const nix::CanonPath & path) const noexcept + std::size_t operator()(const nix::CanonPath & s) const noexcept { - return nix::hash_value(path); + return std::hash{}(s.path); } }; diff --git a/src/libutil/include/nix/util/source-path.hh b/src/libutil/include/nix/util/source-path.hh index 08f9fe580b0..f7cfc8ef72b 100644 --- a/src/libutil/include/nix/util/source-path.hh +++ b/src/libutil/include/nix/util/source-path.hh @@ -119,23 +119,15 @@ struct SourcePath std::ostream & operator<<(std::ostream & str, const SourcePath & path); -inline std::size_t hash_value(const SourcePath & path) -{ - std::size_t hash = 0; - boost::hash_combine(hash, path.accessor->number); - boost::hash_combine(hash, path.path); - return hash; -} - } // namespace nix template<> struct std::hash { - using is_avalanching = std::true_type; - std::size_t operator()(const nix::SourcePath & s) const noexcept { - return nix::hash_value(s); + std::size_t hash = 0; + hash_combine(hash, s.accessor->number, s.path); + return hash; } }; diff --git a/src/libutil/include/nix/util/util.hh b/src/libutil/include/nix/util/util.hh index 26f03938aa5..2e78120fc92 100644 --- a/src/libutil/include/nix/util/util.hh +++ b/src/libutil/include/nix/util/util.hh @@ -220,17 +220,6 @@ typename T::mapped_type * get(T & map, const K & key) template typename T::mapped_type * get(T && map, const K & key) = delete; -/** - * Look up a value in a `boost::concurrent_flat_map`. - */ -template -std::optional getConcurrent(const T & map, const typename T::key_type & key) -{ - std::optional res; - map.cvisit(key, [&](auto & x) { res = x.second; }); - return res; -} - /** * Get a value for the specified key from an associate container, or a default value if the key isn't present. */ diff --git a/src/libutil/posix-source-accessor.cc b/src/libutil/posix-source-accessor.cc index c524f3e4f9a..877c63331a5 100644 --- a/src/libutil/posix-source-accessor.cc +++ b/src/libutil/posix-source-accessor.cc @@ -95,7 +95,9 @@ std::optional PosixSourceAccessor::cachedLstat(const CanonPath & pa // former is not hashable on libc++. Path absPath = makeAbsPath(path).string(); - if (auto res = getConcurrent(cache, absPath)) + std::optional res; + cache.cvisit(absPath, [&](auto & x) { res.emplace(x.second); }); + if (res) return *res; auto st = nix::maybeLstat(absPath.c_str());