diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e34a03bd0b2..d37645ce6d6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -129,6 +129,7 @@ jobs: nix_config: - "lazy-trees = true" - "lazy-trees = false" + - "eval-cores = 24" glob: - "[0-d]*" - "[e-l]*" @@ -144,6 +145,7 @@ jobs: with: repository: DeterminateSystems/flake-regressions path: flake-regressions + ref: prefetch - name: Checkout flake-regressions-data uses: actions/checkout@v4 with: @@ -151,12 +153,17 @@ jobs: path: flake-regressions/tests - uses: DeterminateSystems/determinate-nix-action@main - uses: DeterminateSystems/flakehub-cache-action@main - - env: - PARALLEL: "-P 50%" + - name: Run flake regression tests + env: + PARALLEL: ${{ !contains(matrix.nix_config, 'eval-cores') && '-P 50%' || '-P 1' }} FLAKE_REGRESSION_GLOB: ${{ matrix.glob }} NIX_CONFIG: ${{ matrix.nix_config }} + PREFETCH: "1" + GC_INITIAL_HEAP_SIZE: "32G" run: | set -x + echo "PARALLEL: $PARALLEL" + echo "NIX_CONFIG: $NIX_CONFIG" if [ ! -z "${NSC_CACHE_PATH:-}" ]; then mkdir -p "${NSC_CACHE_PATH}/nix/xdg-cache" export XDG_CACHE_HOME="${NSC_CACHE_PATH}/nix/xdg-cache" @@ -164,6 +171,11 @@ jobs: nix build -L --out-link ./new-nix export PATH=$(pwd)/new-nix/bin:$PATH + nix config show lazy-trees + nix config show eval-cores + lscpu + nproc + if ! flake-regressions/eval-all.sh; then echo "Some failed, trying again" printf "\n\n\n\n\n\n\n\n" diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index fb90e2872e6..79913c17f65 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/src/libexpr-c/nix_api_value.cc @@ -175,6 +175,8 @@ ValueType nix_get_type(nix_c_context * context, const nix_value * value) switch (v.type()) { case nThunk: return NIX_TYPE_THUNK; + case nFailed: + return NIX_TYPE_FAILED; case nInt: return NIX_TYPE_INT; case nFloat: diff --git a/src/libexpr-c/nix_api_value.h b/src/libexpr-c/nix_api_value.h index 7cd6ad18087..263ca526e77 100644 --- a/src/libexpr-c/nix_api_value.h +++ b/src/libexpr-c/nix_api_value.h @@ -32,7 +32,8 @@ typedef enum { NIX_TYPE_ATTRS, NIX_TYPE_LIST, NIX_TYPE_FUNCTION, - NIX_TYPE_EXTERNAL + NIX_TYPE_EXTERNAL, + NIX_TYPE_FAILED, } ValueType; // forward declarations diff --git a/src/libexpr-tests/value/print.cc b/src/libexpr-tests/value/print.cc index 7647cd334d7..3b6889692b1 100644 --- a/src/libexpr-tests/value/print.cc +++ b/src/libexpr-tests/value/print.cc @@ -10,7 +10,7 @@ using namespace testing; struct ValuePrintingTests : LibExprTest { template - void test(Value v, std::string_view expected, A... args) + void test(Value & v, std::string_view expected, A... args) { std::stringstream out; v.print(state, out, args...); @@ -625,10 +625,11 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsElided) vThree.mkInt(3); builder.insert(state.symbols.create("three"), &vThree); - vAttrs.mkAttrs(builder.finish()); + Value vAttrs2; + vAttrs2.mkAttrs(builder.finish()); test( - vAttrs, + vAttrs2, "{ one = " ANSI_CYAN "1" ANSI_NORMAL "; " ANSI_FAINT "«2 attributes elided»" ANSI_NORMAL " }", PrintOptions{.ansiColors = true, .maxAttrs = 1}); } diff --git a/src/libexpr-tests/value/value.cc b/src/libexpr-tests/value/value.cc index 63501dd4995..c6349436fb7 100644 --- a/src/libexpr-tests/value/value.cc +++ b/src/libexpr-tests/value/value.cc @@ -11,7 +11,6 @@ TEST_F(ValueTest, unsetValue) { Value unsetValue; ASSERT_EQ(false, unsetValue.isValid()); - ASSERT_EQ(nThunk, unsetValue.type(true)); ASSERT_DEATH(unsetValue.type(), ""); } diff --git a/src/libexpr/eval-gc.cc b/src/libexpr/eval-gc.cc index 5a4ecf03575..d65ba8a58c8 100644 --- a/src/libexpr/eval-gc.cc +++ b/src/libexpr/eval-gc.cc @@ -35,6 +35,40 @@ static void * oomHandler(size_t requested) throw std::bad_alloc(); } +static size_t getFreeMem() +{ + /* On Linux, use the `MemAvailable` or `MemFree` fields from + /proc/cpuinfo. */ +# ifdef __linux__ + { + std::unordered_map fields; + for (auto & line : + tokenizeString>(readFile(std::filesystem::path("/proc/meminfo")), "\n")) { + auto colon = line.find(':'); + if (colon == line.npos) + continue; + fields.emplace(line.substr(0, colon), trim(line.substr(colon + 1))); + } + + auto i = fields.find("MemAvailable"); + if (i == fields.end()) + i = fields.find("MemFree"); + if (i != fields.end()) { + auto kb = tokenizeString>(i->second, " "); + if (kb.size() == 2 && kb[1] == "kB") + return string2Int(kb[0]).value_or(0) * 1024; + } + } +# endif + + /* On non-Linux systems, conservatively assume that 25% of memory is free. */ + long pageSize = sysconf(_SC_PAGESIZE); + long pages = sysconf(_SC_PHYS_PAGES); + if (pageSize != -1) + return (pageSize * pages) / 4; + return 0; +} + static inline void initGCReal() { /* Initialise the Boehm garbage collector. */ @@ -53,6 +87,8 @@ static inline void initGCReal() GC_INIT(); + GC_allow_register_threads(); + /* Register valid displacements in case we are using alignment niches for storing the type information. This way tagged pointers are considered to be valid, even when they are not aligned. */ @@ -62,8 +98,8 @@ static inline void initGCReal() GC_set_oom_fn(oomHandler); - /* Set the initial heap size to something fairly big (25% of - physical RAM, up to a maximum of 384 MiB) so that in most cases + /* Set the initial heap size to something fairly big (80% of + free RAM, up to a maximum of 8 GiB) so that in most cases we don't need to garbage collect at all. (Collection has a fairly significant overhead.) The heap size can be overridden through libgc's GC_INITIAL_HEAP_SIZE environment variable. We @@ -74,13 +110,10 @@ static inline void initGCReal() if (!getEnv("GC_INITIAL_HEAP_SIZE")) { size_t size = 32 * 1024 * 1024; # if HAVE_SYSCONF && defined(_SC_PAGESIZE) && defined(_SC_PHYS_PAGES) - size_t maxSize = 384 * 1024 * 1024; - long pageSize = sysconf(_SC_PAGESIZE); - long pages = sysconf(_SC_PHYS_PAGES); - if (pageSize != -1) - size = (pageSize * pages) / 4; // 25% of RAM - if (size > maxSize) - size = maxSize; + size_t maxSize = 8ULL * 1024 * 1024 * 1024; + auto free = getFreeMem(); + debug("free memory is %d bytes", free); + size = std::min((size_t) (free * 0.8), maxSize); # endif debug("setting initial heap size to %1% bytes", size); GC_expand_hp(size); diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 126f09e4cd3..0767dc353dc 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -22,6 +22,7 @@ #include "nix/fetchers/fetch-to-store.hh" #include "nix/fetchers/tarball.hh" #include "nix/fetchers/input-cache.hh" +#include "nix/expr/parallel-eval.hh" #include "parser-tab.hh" @@ -127,6 +128,8 @@ std::string_view showType(ValueType type, bool withArticle) return WA("a", "float"); case nThunk: return WA("a", "thunk"); + case nFailed: + return WA("a", "failure"); } unreachable(); } @@ -175,10 +178,10 @@ PosIdx Value::determinePos(const PosIdx pos) const bool Value::isTrivial() const { - return !isa() - && (!isa() - || (dynamic_cast(thunk().expr) && ((ExprAttrs *) thunk().expr)->dynamicAttrs.empty()) - || dynamic_cast(thunk().expr) || dynamic_cast(thunk().expr)); + return isFinished() + || (isa() + && ((dynamic_cast(thunk().expr) && ((ExprAttrs *) thunk().expr)->dynamicAttrs.empty()) + || dynamic_cast(thunk().expr) || dynamic_cast(thunk().expr))); } static Symbol getName(const AttrName & name, EvalState & state, Env & env) @@ -203,6 +206,7 @@ EvalState::EvalState( std::shared_ptr buildStore) : fetchSettings{fetchSettings} , settings{settings} + , executor{make_ref(settings)} , sWith(symbols.create("")) , sOutPath(symbols.create("outPath")) , sDrvPath(symbols.create("drvPath")) @@ -326,8 +330,6 @@ EvalState::EvalState( , trylevel(0) , regexCache(makeRegexCache()) #if NIX_USE_BOEHMGC - , valueAllocCache(std::allocate_shared(traceable_allocator(), nullptr)) - , env1AllocCache(std::allocate_shared(traceable_allocator(), nullptr)) , baseEnvP(std::allocate_shared(traceable_allocator(), &allocEnv(BASE_ENV_SIZE))) , baseEnv(**baseEnvP) #else @@ -484,7 +486,8 @@ void EvalState::checkURI(const std::string & uri) Value * EvalState::addConstant(const std::string & name, Value & v, Constant info) { Value * v2 = allocValue(); - *v2 = v; + // Do a raw copy since `operator =` barfs on thunks. + memcpy((char *) v2, (char *) &v, sizeof(Value)); addConstant(name, v2, info); return v2; } @@ -500,8 +503,10 @@ void EvalState::addConstant(const std::string & name, Value * v, Constant info) We might know the type of a thunk in advance, so be allowed to just write it down in that case. */ - if (auto gotType = v->type(true); gotType != nThunk) - assert(info.type == gotType); + if (v->isFinished()) { + if (auto gotType = v->type(); gotType != nThunk) + assert(info.type == gotType); + } /* Install value the base environment. */ staticBaseEnv->vars.emplace_back(symbols.create(name), baseEnvDispl); @@ -683,7 +688,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(); @@ -738,7 +743,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); @@ -957,7 +962,7 @@ Value * EvalState::getBool(bool b) return b ? &vTrue : &vFalse; } -unsigned long nrThunks = 0; +static std::atomic nrThunks = 0; static inline void mkThunk(Value & v, Env & env, Expr * expr) { @@ -1101,61 +1106,82 @@ 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; - if ((i = fileEvalCache.find(path)) != fileEvalCache.end()) { - v = i->second; - return; + SourcePath & path; + bool mustBeTrivial; + + ExprParseFile(SourcePath & path, bool mustBeTrivial) + : path(path) + , mustBeTrivial(mustBeTrivial) + { } - auto resolvedPath = resolveExprPath(path); - if ((i = fileEvalCache.find(resolvedPath)) != fileEvalCache.end()) { - v = i->second; - return; + 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; + } } +}; - printTalkative("evaluating file '%1%'", resolvedPath); - Expr * e = nullptr; +void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial) +{ + auto resolvedPath = getOptional(*importResolutionCache.readLock(), path); - auto j = fileParseCache.find(resolvedPath); - if (j != fileParseCache.end()) - e = j->second; + if (!resolvedPath) { + resolvedPath = resolveExprPath(path); + importResolutionCache.lock()->emplace(path, *resolvedPath); + } - if (!e) - e = parseExprFromFile(resolvedPath); + if (auto v2 = get(*fileEvalCache.readLock(), *resolvedPath)) { + forceValue(*const_cast(v2), noPos); + v = *v2; + return; + } - fileParseCache.emplace(resolvedPath, e); + Value * vExpr; + ExprParseFile expr{*resolvedPath, mustBeTrivial}; - 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; + { + auto cache(fileEvalCache.lock()); + auto [i, inserted] = cache->try_emplace(*resolvedPath); + if (inserted) + i->second.mkThunk(&baseEnv, &expr); + vExpr = &i->second; } - fileEvalCache.emplace(resolvedPath, v); - if (path != resolvedPath) - fileEvalCache.emplace(path, v); + forceValue(*vExpr, noPos); + + v = *vExpr; } void EvalState::resetFileCache() { - fileEvalCache.clear(); - fileParseCache.clear(); + fileEvalCache.lock()->clear(); inputCache->clear(); } @@ -1447,7 +1473,7 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) state.attrSelects[pos2]++; } - state.forceValue(*vAttrs, (pos2 ? pos2 : this->pos)); + state.forceValue(*vAttrs, pos2 ? pos2 : this->pos); } catch (Error & e) { if (pos2) { @@ -1506,6 +1532,8 @@ void ExprLambda::eval(EvalState & state, Env & env, Value & v) v.mkLambda(&env, this); } +thread_local size_t EvalState::callDepth = 0; + void EvalState::callFunction(Value & fun, std::span args, Value & vRes, const PosIdx pos) { auto _level = addCallDepth(pos); @@ -1521,15 +1549,16 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, forceValue(fun, pos); - Value vCur(fun); + Value vCur = fun; auto makeAppChain = [&]() { - vRes = vCur; for (auto arg : args) { auto fun2 = allocValue(); - *fun2 = vRes; - vRes.mkPrimOpApp(fun2, arg); + *fun2 = vCur; + vCur.reset(); + vCur.mkPrimOpApp(fun2, arg); } + vRes = vCur; }; const Attr * functor; @@ -1625,6 +1654,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, lambda.name ? concatStrings("'", symbols[lambda.name], "'") : "anonymous lambda") : nullptr; + vCur.reset(); lambda.body->eval(*this, env2, vCur); } catch (Error & e) { if (loggerSettings.showTrace.get()) { @@ -1659,7 +1689,9 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, primOpCalls[fn->name]++; try { - fn->fun(*this, vCur.determinePos(noPos), args.data(), vCur); + auto pos = vCur.determinePos(noPos); + vCur.reset(); + fn->fun(*this, pos, args.data(), vCur); } catch (Error & e) { if (fn->addTrace) addErrorTrace(e, pos, "while calling the '%1%' builtin", fn->name); @@ -1681,6 +1713,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, assert(primOp->isPrimOp()); auto arity = primOp->primOp()->arity; auto argsLeft = arity - argsDone; + assert(argsLeft); if (args.size() < argsLeft) { /* We still don't have enough arguments, so extend the tPrimOpApp chain. */ @@ -1709,7 +1742,9 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, // 2. Create a fake env (arg1, arg2, etc.) and a fake expr (arg1: arg2: etc: builtins.name arg1 arg2 // etc) // so the debugger allows to inspect the wrong parameters passed to the builtin. - fn->fun(*this, vCur.determinePos(noPos), vArgs, vCur); + auto pos = vCur.determinePos(noPos); + vCur.reset(); + fn->fun(*this, pos, vArgs, vCur); } catch (Error & e) { if (fn->addTrace) addErrorTrace(e, pos, "while calling the '%1%' builtin", fn->name); @@ -1726,6 +1761,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, heap-allocate a copy and use that instead. */ Value * args2[] = {allocValue(), args[0]}; *args2[0] = vCur; + vCur.reset(); try { callFunction(*functor->value, args2, vCur, functor->pos); } catch (Error & e) { @@ -1744,6 +1780,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, .debugThrow(); } + debug("DONE %x %x", &vRes, &vCur); vRes = vCur; } @@ -2112,16 +2149,6 @@ void ExprPos::eval(EvalState & state, Env & env, Value & v) state.mkPos(v, pos); } -void ExprBlackHole::eval(EvalState & state, [[maybe_unused]] Env & env, Value & v) -{ - throwInfiniteRecursionError(state, v); -} - -[[gnu::noinline]] [[noreturn]] void ExprBlackHole::throwInfiniteRecursionError(EvalState & state, Value & v) -{ - state.error("infinite recursion encountered").atPos(v.determinePos(noPos)).debugThrow(); -} - // always force this to be separate, otherwise forceValue may inline it and take // a massive perf hit [[gnu::noinline]] @@ -2154,6 +2181,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. + // FIXME: race, thunk might be updated by another thread auto dts = debugRepl && i.value->isThunk() ? makeDebugTraceStacker( *this, *i.value->thunk().expr, @@ -2777,8 +2805,11 @@ void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::st } return; - case nThunk: // Must not be left by forceValue - assert(false); + // Cannot be returned by forceValue(). + case nThunk: + case nFailed: + unreachable(); + default: // Note that we pass compiler flags that should make `default:` unreachable. // Also note that this probably ran after `eqValues`, which implements // the same logic more efficiently (without having to unwind stacks), @@ -2870,8 +2901,11 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v // !!! return v1.fpoint() == v2.fpoint(); - case nThunk: // Must not be left by forceValue - assert(false); + // Cannot be returned by forceValue(). + case nThunk: + case nFailed: + unreachable(); + default: // Note that we pass compiler flags that should make `default:` unreachable. error("eqValues: cannot compare %1% with %2%", showType(v1), showType(v2)) .withTrace(pos, errorCtx) @@ -2952,18 +2986,18 @@ void EvalState::printStatistics() #endif }; topObj["envs"] = { - {"number", nrEnvs}, - {"elements", nrValuesInEnvs}, + {"number", nrEnvs.load()}, + {"elements", nrValuesInEnvs.load()}, {"bytes", bEnvs}, }; topObj["nrExprs"] = Expr::nrExprs; topObj["list"] = { - {"elements", nrListElems}, + {"elements", nrListElems.load()}, {"bytes", bLists}, - {"concats", nrListConcats}, + {"concats", nrListConcats.load()}, }; topObj["values"] = { - {"number", nrValues}, + {"number", nrValues.load()}, {"bytes", bValues}, }; topObj["symbols"] = { @@ -2971,9 +3005,9 @@ void EvalState::printStatistics() {"bytes", symbols.totalSize()}, }; topObj["sets"] = { - {"number", nrAttrsets}, + {"number", nrAttrsets.load()}, {"bytes", bAttrsets}, - {"elements", nrAttrsInAttrsets}, + {"elements", nrAttrsInAttrsets.load()}, }; topObj["sizes"] = { {"Env", sizeof(Env)}, @@ -2981,13 +3015,18 @@ void EvalState::printStatistics() {"Bindings", sizeof(Bindings)}, {"Attr", sizeof(Attr)}, }; - topObj["nrOpUpdates"] = nrOpUpdates; - topObj["nrOpUpdateValuesCopied"] = nrOpUpdateValuesCopied; - topObj["nrThunks"] = nrThunks; - topObj["nrAvoided"] = nrAvoided; - topObj["nrLookups"] = nrLookups; - topObj["nrPrimOpCalls"] = nrPrimOpCalls; - topObj["nrFunctionCalls"] = nrFunctionCalls; + topObj["nrOpUpdates"] = nrOpUpdates.load(); + topObj["nrOpUpdateValuesCopied"] = nrOpUpdateValuesCopied.load(); + topObj["nrThunks"] = nrThunks.load(); + topObj["nrThunksAwaited"] = nrThunksAwaited.load(); + topObj["nrThunksAwaitedSlow"] = nrThunksAwaitedSlow.load(); + topObj["nrSpuriousWakeups"] = nrSpuriousWakeups.load(); + topObj["maxWaiting"] = maxWaiting.load(); + topObj["waitingTime"] = usWaiting / (double) 1000000; + topObj["nrAvoided"] = nrAvoided.load(); + topObj["nrLookups"] = nrLookups.load(); + topObj["nrPrimOpCalls"] = nrPrimOpCalls.load(); + topObj["nrFunctionCalls"] = nrFunctionCalls.load(); #if NIX_USE_BOEHMGC topObj["gc"] = { {"heapSize", heapSize}, @@ -3035,10 +3074,10 @@ void EvalState::printStatistics() } if (getEnv("NIX_SHOW_SYMBOLS").value_or("0") != "0") { + auto list = json::array(); + symbols.dump([&](std::string_view s) { list.emplace_back(std::string(s)); }); // XXX: overrides earlier assignment - topObj["symbols"] = json::array(); - auto & list = topObj["symbols"]; - symbols.dump([&](std::string_view s) { list.emplace_back(s); }); + topObj["symbols"] = std::move(list); } if (outPath == "-") { std::cerr << topObj.dump(2) << std::endl; @@ -3221,10 +3260,10 @@ Expr * EvalState::parse( char * text, size_t length, Pos::Origin origin, const SourcePath & basePath, std::shared_ptr & staticEnv) { DocCommentMap tmpDocComments; // Only used when not origin is not a SourcePath - DocCommentMap * docComments = &tmpDocComments; + auto * docComments = &tmpDocComments; if (auto sourcePath = std::get_if(&origin)) { - auto [it, _] = positionToDocComment.try_emplace(*sourcePath); + auto [it, _] = positionToDocComment.lock()->try_emplace(*sourcePath); docComments = &it->second; } @@ -3243,8 +3282,10 @@ DocComment EvalState::getDocCommentForPos(PosIdx pos) if (!path) return {}; - auto table = positionToDocComment.find(*path); - if (table == positionToDocComment.end()) + auto positionToDocComment_ = positionToDocComment.readLock(); + + auto table = positionToDocComment_->find(*path); + if (table == positionToDocComment_->end()) return {}; auto it = table->second.find(pos); diff --git a/src/libexpr/include/nix/expr/eval-inline.hh b/src/libexpr/include/nix/expr/eval-inline.hh index a1fd0ae4aa8..99b824743f0 100644 --- a/src/libexpr/include/nix/expr/eval-inline.hh +++ b/src/libexpr/include/nix/expr/eval-inline.hh @@ -36,6 +36,9 @@ Value * EvalState::allocValue() GC_malloc_many returns a linked list of objects of the given size, where the first word of each object is also the pointer to the next object in the list. This also means that we have to explicitly clear the first word of every object we take. */ + thread_local static std::shared_ptr valueAllocCache{ + std::allocate_shared(traceable_allocator(), nullptr)}; + if (!*valueAllocCache) { *valueAllocCache = GC_malloc_many(sizeof(Value)); if (!*valueAllocCache) @@ -66,6 +69,9 @@ Env & EvalState::allocEnv(size_t size) #if NIX_USE_BOEHMGC if (size == 1) { /* see allocValue for explanations. */ + thread_local static std::shared_ptr env1AllocCache{ + std::allocate_shared(traceable_allocator(), nullptr)}; + if (!*env1AllocCache) { *env1AllocCache = GC_malloc_many(sizeof(Env) + sizeof(Value *)); if (!*env1AllocCache) @@ -85,27 +91,56 @@ Env & EvalState::allocEnv(size_t size) return *env; } -[[gnu::always_inline]] -void EvalState::forceValue(Value & v, const PosIdx pos) +template +void ValueStorage>>::force( + EvalState & state, PosIdx pos) { - if (v.isThunk()) { - Env * env = v.thunk().env; - assert(env || v.isBlackhole()); - Expr * expr = v.thunk().expr; + // FIXME: check that the compiler won't reorder this below the + // load of p0. + auto p1_ = p1; + auto p0_ = p0.load(std::memory_order_acquire); + + auto pd = static_cast(p0_ & discriminatorMask); + + if (pd == pdThunk) { try { - v.mkBlackhole(); - // checkInterrupt(); - if (env) [[likely]] - expr->eval(*this, *env, v); - else - ExprBlackHole::throwInfiniteRecursionError(*this, v); + // Atomically set the thunk to "pending". + if (!p0.compare_exchange_strong(p0_, pdPending, std::memory_order_acquire, std::memory_order_acquire)) { + pd = static_cast(p0_ & discriminatorMask); + if (pd == pdPending || pd == pdAwaited) { + // The thunk is already "pending" or "awaited", so + // we need to wait for it. + p0_ = waitOnThunk(state, pd == pdAwaited); + goto done; + } + assert(pd != pdThunk); + // Another thread finished this thunk, no need to wait. + goto done; + } + + bool isApp = p1_ & discriminatorMask; + if (isApp) { + auto left = untagPointer(p0_); + auto right = untagPointer(p1_); + state.callFunction(*left, *right, (Value &) *this, pos); + } else { + auto env = untagPointer(p0_); + auto expr = untagPointer(p1_); + expr->eval(state, *env, (Value &) *this); + } } catch (...) { - v.mkThunk(env, expr); - tryFixupBlackHolePos(v, pos); + state.tryFixupBlackHolePos((Value &) *this, pos); + setStorage(new Value::Failed{.ex = std::current_exception()}); throw; } - } else if (v.isApp()) - callFunction(*v.app().left, *v.app().right, v, pos); + } + + else if (pd == pdPending || pd == pdAwaited) + p0_ = waitOnThunk(state, pd == pdAwaited); + +done: + if (InternalType(p0_ & 0xff) == tFailed) + std::rethrow_exception((std::bit_cast(p1))->ex); } [[gnu::always_inline]] diff --git a/src/libexpr/include/nix/expr/eval-settings.hh b/src/libexpr/include/nix/expr/eval-settings.hh index c14f263ecf4..88b797d8614 100644 --- a/src/libexpr/include/nix/expr/eval-settings.hh +++ b/src/libexpr/include/nix/expr/eval-settings.hh @@ -347,6 +347,14 @@ struct EvalSettings : Config This is not backward compatible with older versions of Nix. If disabled, lock file entries always contain a NAR hash. )"}; + + Setting evalCores{ + this, + 1, + "eval-cores", + R"( + The number of threads used to evaluate Nix expressions. + )"}; }; /** diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh index ac700e7485a..2dc37faa7f4 100644 --- a/src/libexpr/include/nix/expr/eval.hh +++ b/src/libexpr/include/nix/expr/eval.hh @@ -51,6 +51,7 @@ struct MountedSourceAccessor; namespace eval_cache { class EvalCache; } +struct Executor; /** * Increments a count on construction and decrements on destruction. @@ -220,6 +221,9 @@ class EvalState : public std::enable_shared_from_this public: const fetchers::Settings & fetchSettings; const EvalSettings & settings; + + ref executor; + SymbolTable symbols; PosTable positions; @@ -366,19 +370,13 @@ private: Sync> 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 std::unordered_map< - SourcePath, - Expr *, - std::hash, - std::equal_to, - traceable_allocator>> - FileParseCache; - FileParseCache fileParseCache; + SharedSync> importResolutionCache; /** - * A cache from path names to values. + * A cache from resolved paths to values. */ typedef std::unordered_map< SourcePath, @@ -387,13 +385,13 @@ private: std::equal_to, traceable_allocator>> FileEvalCache; - FileEvalCache fileEvalCache; + SharedSync fileEvalCache; /** * Associate source positions of certain AST nodes with their preceding doc comment, if they have one. * Grouped by file. */ - std::unordered_map positionToDocComment; + SharedSync> positionToDocComment; LookupPath lookupPath; @@ -404,18 +402,6 @@ private: */ std::shared_ptr regexCache; -#if NIX_USE_BOEHMGC - /** - * Allocation cache for GC'd Value objects. - */ - std::shared_ptr valueAllocCache; - - /** - * Allocation cache for size-1 Env objects. - */ - std::shared_ptr env1AllocCache; -#endif - public: EvalState( @@ -542,7 +528,10 @@ public: * application, call the function and overwrite `v` with the * result. Otherwise, this is a no-op. */ - inline void forceValue(Value & v, const PosIdx pos); + inline void forceValue(Value & v, const PosIdx pos) + { + v.force(*this, pos); + } void tryFixupBlackHolePos(Value & v, PosIdx pos); @@ -778,10 +767,11 @@ private: std::shared_ptr & staticEnv); /** - * Current Nix call stack depth, used with `max-call-depth` setting to throw stack overflow hopefully before we run - * out of system stack. + * Current Nix call stack depth, used with `max-call-depth` + * setting to throw stack overflow hopefully before we run out of + * system stack. */ - size_t callDepth = 0; + thread_local static size_t callDepth; public: @@ -953,20 +943,29 @@ private: */ std::string mkSingleDerivedPathStringRaw(const SingleDerivedPath & p); - unsigned long nrEnvs = 0; - unsigned long nrValuesInEnvs = 0; - unsigned long nrValues = 0; - unsigned long nrListElems = 0; - unsigned long nrLookups = 0; - unsigned long nrAttrsets = 0; - unsigned long nrAttrsInAttrsets = 0; - unsigned long nrAvoided = 0; - unsigned long nrOpUpdates = 0; - unsigned long nrOpUpdateValuesCopied = 0; - unsigned long nrListConcats = 0; - unsigned long nrPrimOpCalls = 0; - unsigned long nrFunctionCalls = 0; + std::atomic nrEnvs = 0; + std::atomic nrValuesInEnvs = 0; + std::atomic nrValues = 0; + std::atomic nrListElems = 0; + std::atomic nrLookups = 0; + std::atomic nrAttrsets = 0; + std::atomic nrAttrsInAttrsets = 0; + std::atomic nrAvoided = 0; + std::atomic nrOpUpdates = 0; + std::atomic nrOpUpdateValuesCopied = 0; + std::atomic nrListConcats = 0; + std::atomic nrPrimOpCalls = 0; + std::atomic nrFunctionCalls = 0; + +public: + std::atomic nrThunksAwaited{0}; + std::atomic nrThunksAwaitedSlow{0}; + std::atomic usWaiting{0}; + std::atomic currentlyWaiting{0}; + std::atomic maxWaiting{0}; + std::atomic nrSpuriousWakeups{0}; +private: bool countCalls; typedef std::map PrimOpCalls; diff --git a/src/libexpr/include/nix/expr/meson.build b/src/libexpr/include/nix/expr/meson.build index 333490ee499..f20afbb5910 100644 --- a/src/libexpr/include/nix/expr/meson.build +++ b/src/libexpr/include/nix/expr/meson.build @@ -23,6 +23,7 @@ headers = [config_pub_h] + files( 'get-drvs.hh', 'json-to-value.hh', 'nixexpr.hh', + 'parallel-eval.hh', 'parser-state.hh', 'primops.hh', 'print-ambiguous.hh', diff --git a/src/libexpr/include/nix/expr/nixexpr.hh b/src/libexpr/include/nix/expr/nixexpr.hh index 49bd7a3b659..7953643e157 100644 --- a/src/libexpr/include/nix/expr/nixexpr.hh +++ b/src/libexpr/include/nix/expr/nixexpr.hh @@ -632,20 +632,6 @@ struct ExprPos : Expr COMMON_METHODS }; -/* only used to mark thunks as black holes. */ -struct ExprBlackHole : Expr -{ - void show(const SymbolTable & symbols, std::ostream & str) const override {} - - void eval(EvalState & state, Env & env, Value & v) override; - - void bindVars(EvalState & es, const std::shared_ptr & env) override {} - - [[noreturn]] static void throwInfiniteRecursionError(EvalState & state, Value & v); -}; - -extern ExprBlackHole eBlackHole; - /* Static environments are used to map variable names onto (level, displacement) pairs used to obtain the value of the variable at runtime. */ diff --git a/src/libexpr/include/nix/expr/parallel-eval.hh b/src/libexpr/include/nix/expr/parallel-eval.hh new file mode 100644 index 00000000000..9356a925540 --- /dev/null +++ b/src/libexpr/include/nix/expr/parallel-eval.hh @@ -0,0 +1,77 @@ +#pragma once + +#include +#include +#include +#include + +#include "nix/util/sync.hh" +#include "nix/util/logging.hh" +#include "nix/util/environment-variables.hh" +#include "nix/util/util.hh" +#include "nix/util/signals.hh" + +#if NIX_USE_BOEHMGC +# include +#endif + +namespace nix { + +struct Executor +{ + using work_t = std::function; + + struct Item + { + std::promise promise; + work_t work; + }; + + struct State + { + std::multimap queue; + std::vector threads; + bool quit = false; + }; + + const bool enabled; + + Sync state_; + + std::condition_variable wakeup; + + Executor(const EvalSettings & evalSettings); + + ~Executor(); + + void worker(); + + std::vector> spawn(std::vector> && items); + + static thread_local bool amWorkerThread; +}; + +struct FutureVector +{ + Executor & executor; + + struct State + { + std::vector> futures; + }; + + Sync state_; + + // FIXME: add a destructor that cancels/waits for all futures. + + void spawn(std::vector> && work); + + void spawn(uint8_t prioPrefix, Executor::work_t && work) + { + spawn({{std::move(work), prioPrefix}}); + } + + void finishAll(); +}; + +} // namespace nix diff --git a/src/libexpr/include/nix/expr/symbol-table.hh b/src/libexpr/include/nix/expr/symbol-table.hh index 92f61d45ab5..c79d7533707 100644 --- a/src/libexpr/include/nix/expr/symbol-table.hh +++ b/src/libexpr/include/nix/expr/symbol-table.hh @@ -2,17 +2,13 @@ ///@file #include + #include "nix/expr/value.hh" -#include "nix/util/chunked-vector.hh" #include "nix/util/error.hh" +#include "nix/util/sync.hh" #include -#define USE_FLAT_SYMBOL_SET (BOOST_VERSION >= 108100) -#if USE_FLAT_SYMBOL_SET -# include -#else -# include -#endif +#include namespace nix { @@ -21,18 +17,24 @@ class SymbolValue : protected Value friend class SymbolStr; friend class SymbolTable; - uint32_t size_; - uint32_t idx; - - SymbolValue() = default; - -public: operator std::string_view() const noexcept { - return {c_str(), size_}; + // The actual string is stored directly after the value. + return (char *) (this + 1); } }; +struct ContiguousArena +{ + const char * data; + const size_t maxSize; + std::atomic size{0}; + + ContiguousArena(size_t maxSize); + + size_t allocate(size_t bytes); +}; + /** * Symbols have the property that they can be compared efficiently * (using an equality test), because the symbol table stores only one @@ -44,6 +46,7 @@ class Symbol friend class SymbolTable; private: + /// The offset of the symbol in `SymbolTable::arena`. uint32_t id; explicit Symbol(uint32_t id) noexcept @@ -85,25 +88,20 @@ class SymbolStr { friend class SymbolTable; - constexpr static size_t chunkSize{8192}; - using SymbolValueStore = ChunkedVector; - const SymbolValue * s; struct Key { using HashType = boost::hash; - SymbolValueStore & store; std::string_view s; std::size_t hash; - std::pmr::polymorphic_allocator & alloc; + ContiguousArena & arena; - Key(SymbolValueStore & store, std::string_view s, std::pmr::polymorphic_allocator & stringAlloc) - : store(store) - , s(s) + Key(std::string_view s, ContiguousArena & arena) + : s(s) , hash(HashType{}(s)) - , alloc(stringAlloc) + , arena(arena) { } }; @@ -114,26 +112,7 @@ public: { } - SymbolStr(const Key & key) - { - auto size = key.s.size(); - if (size >= std::numeric_limits::max()) { - throw Error("Size of symbol exceeds 4GiB and cannot be stored"); - } - // for multi-threaded implementations: lock store and allocator here - const auto & [v, idx] = key.store.add(SymbolValue{}); - if (size == 0) { - v.mkString("", nullptr); - } else { - auto s = key.alloc.allocate(size + 1); - memcpy(s, key.s.data(), size); - s[size] = '\0'; - v.mkString(s, nullptr); - } - v.size_ = size; - v.idx = idx; - this->s = &v; - } + SymbolStr(const Key & key); bool operator==(std::string_view s2) const noexcept { @@ -156,13 +135,13 @@ public: [[gnu::always_inline]] bool empty() const noexcept { - return s->size_ == 0; + return ((std::string_view) *s).empty(); } [[gnu::always_inline]] size_t size() const noexcept { - return s->size_; + return ((std::string_view) *s).size(); } [[gnu::always_inline]] @@ -171,11 +150,6 @@ public: return s; } - explicit operator Symbol() const noexcept - { - return Symbol{s->idx + 1}; - } - struct Hash { using is_transparent = void; @@ -226,76 +200,69 @@ private: * SymbolTable is an append only data structure. * During its lifetime the monotonic buffer holds all strings and nodes, if the symbol set is node based. */ - std::pmr::monotonic_buffer_resource buffer; - std::pmr::polymorphic_allocator stringAlloc{&buffer}; - SymbolStr::SymbolValueStore store{16}; + ContiguousArena arena; /** - * Transparent lookup of string view for a pointer to a ChunkedVector entry -> return offset into the store. - * ChunkedVector references are never invalidated. + * Transparent lookup of string view for a pointer to a + * SymbolValue in the arena. */ -#if USE_FLAT_SYMBOL_SET - boost::unordered_flat_set symbols{SymbolStr::chunkSize}; -#else - using SymbolValueAlloc = std::pmr::polymorphic_allocator; - boost::unordered_set symbols{ - SymbolStr::chunkSize, {&buffer}}; -#endif + boost::concurrent_flat_set symbols; public: + constexpr static size_t alignment = 8; + + SymbolTable() + : arena(1 << 30) + { + // Reserve symbol ID 0 and ensure alignment of the first allocation. + arena.allocate(alignment); + } + /** * Converts a string into a symbol. */ - Symbol create(std::string_view s) - { - // Most symbols are looked up more than once, so we trade off insertion performance - // for lookup performance. - // FIXME: make this thread-safe. - return [&](T && key) -> Symbol { - if constexpr (requires { symbols.insert(key); }) { - auto [it, _] = symbols.insert(key); - return Symbol(*it); - } else { - auto it = symbols.find(key); - if (it != symbols.end()) - return Symbol(*it); - - it = symbols.emplace(key).first; - return Symbol(*it); - } - }(SymbolStr::Key{store, s, stringAlloc}); - } + Symbol create(std::string_view s); std::vector resolve(const std::vector & symbols) const { std::vector result; result.reserve(symbols.size()); - for (auto sym : symbols) + for (auto & sym : symbols) result.push_back((*this)[sym]); return result; } SymbolStr operator[](Symbol s) const { - uint32_t idx = s.id - uint32_t(1); - if (idx >= store.size()) + if (s.id == 0 || s.id > arena.size) unreachable(); - return store[idx]; + return SymbolStr(*(SymbolValue *) (arena.data + s.id)); } - [[gnu::always_inline]] size_t size() const noexcept { - return store.size(); + return symbols.size(); } - size_t totalSize() const; + size_t totalSize() const + { + return arena.size; + } template void dump(T callback) const { - store.forEach(callback); +// FIXME +#if 0 + std::string_view left{arena.data, arena.size}; + while (!left.empty()) { + auto p = left.find((char) 0); + if (p == left.npos) break; + callback(left.substr(0, p)); + left = left.substr(p + 1); + } +#endif } }; @@ -309,5 +276,3 @@ struct std::hash return std::hash{}(s.id); } }; - -#undef USE_FLAT_SYMBOL_SET diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index a2833679bef..d12b84b44e3 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include #include #include #include @@ -19,6 +20,19 @@ namespace nix { struct Value; class BindingsBuilder; +static constexpr int discriminatorBits = 3; + +enum PrimaryDiscriminator : int { + pdSingleDWord = 0, + pdThunk = 1, + pdPending = 2, + pdAwaited = 3, + pdPairOfPointers = 4, + pdListN = 5, // FIXME: get rid of this by putting the size in the first word + pdString = 6, + pdPath = 7, // FIXME: get rid of this by ditching the `accessor` field +}; + /** * Internal type discriminator, which is more detailed than `ValueType`, as * it specifies the exact representation used (for types that have multiple @@ -29,27 +43,50 @@ class BindingsBuilder; * This also restricts the number of internal types represented with distinct memory layouts. */ typedef enum { - tUninitialized = 0, - /* layout: Single/zero field payload */ - tInt = 1, - tBool, - tNull, - tFloat, - tExternal, - tPrimOp, - tAttrs, - /* layout: Pair of pointers payload */ - tListSmall, - tPrimOpApp, - tApp, - tThunk, - tLambda, - /* layout: Single untaggable field */ - tListN, - tString, - tPath, + /* Values that have more type bits in the first word, and the + payload (a single word) in the second word. */ + tUninitialized = PrimaryDiscriminator::pdSingleDWord | (0 << discriminatorBits), + tInt = PrimaryDiscriminator::pdSingleDWord | (1 << discriminatorBits), + tFloat = PrimaryDiscriminator::pdSingleDWord | (2 << discriminatorBits), + tBool = PrimaryDiscriminator::pdSingleDWord | (3 << discriminatorBits), + tNull = PrimaryDiscriminator::pdSingleDWord | (4 << discriminatorBits), + tAttrs = PrimaryDiscriminator::pdSingleDWord | (5 << discriminatorBits), + tPrimOp = PrimaryDiscriminator::pdSingleDWord | (6 << discriminatorBits), + tFailed = PrimaryDiscriminator::pdSingleDWord | (7 << discriminatorBits), + tExternal = PrimaryDiscriminator::pdSingleDWord | (8 << discriminatorBits), + + /* Thunks. */ + tThunk = PrimaryDiscriminator::pdThunk | (0 << discriminatorBits), + tApp = PrimaryDiscriminator::pdThunk | (1 << discriminatorBits), + + tPending = PrimaryDiscriminator::pdPending, + tAwaited = PrimaryDiscriminator::pdAwaited, + + /* Values that consist of two pointers. The second word contains + more type bits in its alignment niche. */ + tListSmall = PrimaryDiscriminator::pdPairOfPointers | (0 << discriminatorBits), + tPrimOpApp = PrimaryDiscriminator::pdPairOfPointers | (1 << discriminatorBits), + tLambda = PrimaryDiscriminator::pdPairOfPointers | (2 << discriminatorBits), + + /* Special values. */ + tListN = PrimaryDiscriminator::pdListN, + tString = PrimaryDiscriminator::pdString, + tPath = PrimaryDiscriminator::pdPath, } 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 t) +{ + return t != tUninitialized && t != tThunk && t != tApp && t != tPending && t != tAwaited; +} + /** * This type abstracts over all actual value types in the language, * grouping together implementation details like tList*, different function @@ -57,6 +94,7 @@ typedef enum { */ typedef enum { nThunk, + nFailed, nInt, nFloat, nBool, @@ -73,7 +111,6 @@ class Bindings; struct Env; struct Expr; struct ExprLambda; -struct ExprBlackHole; struct PrimOp; class Symbol; class SymbolStr; @@ -265,6 +302,11 @@ struct ValueBase size_t size; Value * const * elems; }; + + struct Failed + { + std::exception_ptr ex; + }; }; template @@ -291,6 +333,7 @@ struct PayloadTypeToInternalType MACRO(PrimOp *, primOp, tPrimOp) \ MACRO(ValueBase::PrimOpApplicationThunk, primOpApp, tPrimOpApp) \ MACRO(ExternalValueBase *, external, tExternal) \ + MACRO(ValueBase::Failed *, failed, tFailed) \ MACRO(NixFloat, fpoint, tFloat) #define NIX_VALUE_PAYLOAD_TYPE(T, FIELD_NAME, DISCRIMINATOR) \ @@ -393,12 +436,44 @@ class ValueStorage::type; - using Payload = std::array; - Payload payload = {}; - static constexpr int discriminatorBits = 3; + /** + * For multithreaded evaluation, we have to make sure that thunks/apps + * (the only mutable types of values) are updated in a safe way. A + * value can have the following states (see `force()`): + * + * * "thunk"/"app". When forced, this value transitions to + * "pending". The current thread will evaluate the + * thunk/app. When done, it will override the value with the + * result. If the value is at that point in the "awaited" state, + * the thread will wake up any waiting threads. + * + * * "pending". This means it's currently being evaluated. If + * another thread forces this value, it transitions to "awaited" + * and the thread will wait for the value to be updated (see + * `waitOnThunk()`). + * + * * "awaited". Like pending, only it means that there already are + * one or more threads waiting for this thunk. + * + * To ensure race-free access, the non-atomic word `p1` must + * always be updated before `p0`. Writes to `p0` should use + * *release* semantics (so that `p1` and any referenced values become + * visible to threads that read `p0`), and reads from `p0` should + * use `*acquire* semantics. + * + * Note: at some point, we may want to switch to 128-bit atomics + * so that `p0` and `p1` can be updated together + * atomically. However, 128-bit atomics are a bit problematic at + * present on x86_64 (see + * e.g. https://ibraheem.ca/posts/128-bit-atomics/). + */ + std::atomic p0{0}; + PackedPointer p1{0}; + static constexpr PackedPointer discriminatorMask = (PackedPointer(1) << discriminatorBits) - 1; + // FIXME: move/update /** * The value is stored as a pair of 8-byte double words. All pointers are assumed * to be 8-byte aligned. This gives us at most 6 bits of discriminator bits @@ -428,15 +503,6 @@ class ValueStorage requires std::is_pointer_v @@ -447,7 +513,7 @@ class ValueStorage(payload[0] & discriminatorMask); + return static_cast(p0 & discriminatorMask); } static void assertAligned(PackedPointer val) noexcept @@ -455,13 +521,34 @@ class ValueStorage(p0_ & discriminatorMask); + if (pd == pdPending) + // Nothing to do; no thread is waiting on this thunk. + ; + else if (pd == pdAwaited) + // Slow path: wake up the threads that are waiting on this + // thunk. + notifyWaiters(); + else if (pd == pdThunk) { + printError("BAD FINISH %x", this); + unreachable(); + } + } + template void setSingleDWordPayload(PackedPointer untaggedVal) noexcept { - /* There's plenty of free upper bits in the first dword, which is - used only for the discriminator. */ - payload[0] = static_cast(pdSingleDWord) | (static_cast(type) << discriminatorBits); - payload[1] = untaggedVal; + /* There's plenty of free upper bits in the first byte, which + is used only for the discriminator. */ + finish(static_cast(type), untaggedVal); } template @@ -470,32 +557,42 @@ class ValueStorage= pdListN && discriminator <= pdPath); auto firstFieldPayload = std::bit_cast(firstPtrField); assertAligned(firstFieldPayload); - payload[0] = static_cast(discriminator) | firstFieldPayload; - payload[1] = std::bit_cast(untaggableField); + finish(static_cast(discriminator) | firstFieldPayload, std::bit_cast(untaggableField)); } template void setPairOfPointersPayload(T * firstPtrField, U * secondPtrField) noexcept { static_assert(type >= tListSmall && type <= tLambda); - { - auto firstFieldPayload = std::bit_cast(firstPtrField); - assertAligned(firstFieldPayload); - payload[0] = static_cast(pdPairOfPointers) | firstFieldPayload; - } - { - auto secondFieldPayload = std::bit_cast(secondPtrField); - assertAligned(secondFieldPayload); - payload[1] = (type - tListSmall) | secondFieldPayload; - } + auto firstFieldPayload = std::bit_cast(firstPtrField); + assertAligned(firstFieldPayload); + auto secondFieldPayload = std::bit_cast(secondPtrField); + assertAligned(secondFieldPayload); + finish( + static_cast(pdPairOfPointers) | firstFieldPayload, + ((type - tListSmall) >> discriminatorBits) | secondFieldPayload); + } + + template + void setThunkPayload(T * firstPtrField, U * secondPtrField) noexcept + { + static_assert(type >= tThunk && type <= tApp); + auto secondFieldPayload = std::bit_cast(secondPtrField); + assertAligned(secondFieldPayload); + p1 = ((type - tThunk) >> discriminatorBits) | secondFieldPayload; + auto firstFieldPayload = std::bit_cast(firstPtrField); + assertAligned(firstFieldPayload); + // Note: awaited values can never become a thunk, so no need + // to check for waiters. + p0.store(static_cast(pdThunk) | firstFieldPayload, std::memory_order_relaxed); } template requires std::is_pointer_v && std::is_pointer_v void getPairOfPointersPayload(T & firstPtrField, U & secondPtrField) const noexcept { - firstPtrField = untagPointer(payload[0]); - secondPtrField = untagPointer(payload[1]); + firstPtrField = untagPointer(p0); + secondPtrField = untagPointer(p1); } protected: @@ -503,42 +600,45 @@ protected: InternalType getInternalType() const noexcept { switch (auto pd = getPrimaryDiscriminator()) { - case pdUninitialized: - /* Discriminator value of zero is used to distinguish uninitialized values. */ - return tUninitialized; case pdSingleDWord: - /* Payloads that only use up a single double word store the InternalType - in the upper bits of the first double word. */ - return InternalType(payload[0] >> discriminatorBits); + /* Payloads that only use up a single double word store + the full InternalType in the first byte. */ + return InternalType(p0 & 0xff); + case pdThunk: + return static_cast(tThunk + ((p1 & discriminatorMask) << discriminatorBits)); + case pdPending: + return tPending; + case pdAwaited: + return tAwaited; + case pdPairOfPointers: + return static_cast(tListSmall + ((p1 & discriminatorMask) << discriminatorBits)); /* The order must match that of the enumerations defined in InternalType. */ case pdListN: case pdString: case pdPath: return static_cast(tListN + (pd - pdListN)); - case pdPairOfPointers: - return static_cast(tListSmall + (payload[1] & discriminatorMask)); [[unlikely]] default: unreachable(); } } -#define NIX_VALUE_STORAGE_DEF_PAIR_OF_PTRS(TYPE, MEMBER_A, MEMBER_B) \ - \ - void getStorage(TYPE & val) const noexcept \ - { \ - getPairOfPointersPayload(val MEMBER_A, val MEMBER_B); \ - } \ - \ - void setStorage(TYPE val) noexcept \ - { \ - setPairOfPointersPayload>(val MEMBER_A, val MEMBER_B); \ +#define NIX_VALUE_STORAGE_DEF_PAIR_OF_PTRS(TYPE, SET, MEMBER_A, MEMBER_B) \ + \ + void getStorage(TYPE & val) const noexcept \ + { \ + getPairOfPointersPayload(val MEMBER_A, val MEMBER_B); \ + } \ + \ + void setStorage(TYPE val) noexcept \ + { \ + SET>(val MEMBER_A, val MEMBER_B); \ } - NIX_VALUE_STORAGE_DEF_PAIR_OF_PTRS(SmallList, [0], [1]) - NIX_VALUE_STORAGE_DEF_PAIR_OF_PTRS(PrimOpApplicationThunk, .left, .right) - NIX_VALUE_STORAGE_DEF_PAIR_OF_PTRS(FunctionApplicationThunk, .left, .right) - NIX_VALUE_STORAGE_DEF_PAIR_OF_PTRS(ClosureThunk, .env, .expr) - NIX_VALUE_STORAGE_DEF_PAIR_OF_PTRS(Lambda, .env, .fun) + NIX_VALUE_STORAGE_DEF_PAIR_OF_PTRS(SmallList, setPairOfPointersPayload, [0], [1]) + NIX_VALUE_STORAGE_DEF_PAIR_OF_PTRS(PrimOpApplicationThunk, setPairOfPointersPayload, .left, .right) + NIX_VALUE_STORAGE_DEF_PAIR_OF_PTRS(Lambda, setPairOfPointersPayload, .env, .fun) + NIX_VALUE_STORAGE_DEF_PAIR_OF_PTRS(FunctionApplicationThunk, setThunkPayload, .left, .right) + NIX_VALUE_STORAGE_DEF_PAIR_OF_PTRS(ClosureThunk, setThunkPayload, .env, .expr) #undef NIX_VALUE_STORAGE_DEF_PAIR_OF_PTRS @@ -546,52 +646,57 @@ protected: { /* PackedPointerType -> int64_t here is well-formed, since the standard requires this conversion to follow 2's complement rules. This is just a no-op. */ - integer = NixInt(payload[1]); + integer = NixInt(p1); } void getStorage(bool & boolean) const noexcept { - boolean = payload[1]; + boolean = p1; } void getStorage(Null & null) const noexcept {} void getStorage(NixFloat & fpoint) const noexcept { - fpoint = std::bit_cast(payload[1]); + fpoint = std::bit_cast(p1); } void getStorage(ExternalValueBase *& external) const noexcept { - external = std::bit_cast(payload[1]); + external = std::bit_cast(p1); } void getStorage(PrimOp *& primOp) const noexcept { - primOp = std::bit_cast(payload[1]); + primOp = std::bit_cast(p1); } void getStorage(Bindings *& attrs) const noexcept { - attrs = std::bit_cast(payload[1]); + attrs = std::bit_cast(p1); } void getStorage(List & list) const noexcept { - list.elems = untagPointer(payload[0]); - list.size = payload[1]; + list.elems = untagPointer(p0); + list.size = p1; } void getStorage(StringWithContext & string) const noexcept { - string.context = untagPointer(payload[0]); - string.c_str = std::bit_cast(payload[1]); + string.context = untagPointer(p0); + string.c_str = std::bit_cast(p1); } void getStorage(Path & path) const noexcept { - path.accessor = untagPointer(payload[0]); - path.path = std::bit_cast(payload[1]); + path.accessor = untagPointer(p0); + path.path = std::bit_cast(p1); + } + + void getStorage(Failed *& failed) const noexcept + { + failed = std::bit_cast(p1); } void setStorage(NixInt integer) noexcept @@ -643,6 +748,65 @@ protected: { setUntaggablePayload(path.accessor, path.path); } + + void setStorage(Failed * failed) noexcept + { + setSingleDWordPayload(std::bit_cast(failed)); + } + + ValueStorage() {} + + ValueStorage(const ValueStorage & v) + { + *this = v; + } + + /** + * Copy a value. This is not allowed to be a thunk to avoid + * accidental work duplication. + */ + ValueStorage & operator=(const ValueStorage & v) + { + auto p1_ = v.p1; + auto p0_ = v.p0.load(std::memory_order_acquire); + auto pd = static_cast(p0_ & discriminatorMask); + if (pd == pdThunk || pd == pdPending || pd == pdAwaited) { + printError("UNFINISHED %x %08x %08x", this, p0_, p1_); + unreachable(); + } + finish(p0_, p1_); + return *this; + } + +public: + + inline void reset() + { + p1 = 0; + p0.store(0, std::memory_order_relaxed); + } + + /// Only used for testing. + inline void mkBlackhole() + { + p0.store(pdPending, std::memory_order_relaxed); + } + + void force(EvalState & state, PosIdx pos); + +private: + + /** + * Given a thunk that was observed to be in the pending or awaited + * state, wait for it to finish. Returns the first word of the + * value. + */ + PackedPointer waitOnThunk(EvalState & state, bool awaited); + + /** + * Wake up any threads that are waiting on this value. + */ + void notifyWaiters(); }; /** @@ -857,47 +1021,58 @@ public: void print(EvalState & state, std::ostream & str, PrintOptions options = PrintOptions{}); + // FIXME: optimize, only look at first word + inline bool isFinished() const + { + return nix::isFinished(getInternalType()); + } + // 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 - // type() == nThunk inline bool isThunk() const { return isa(); - }; + } inline bool isApp() const { return isa(); - }; + } - inline bool isBlackhole() const; + inline bool isBlackhole() const + { + auto t = getInternalType(); + return t == tPending || t == tAwaited; + } // type() == nFunction inline bool isLambda() const { return isa(); - }; + } inline bool isPrimOp() const { return isa(); - }; + } inline bool isPrimOpApp() const { return isa(); - }; + } + + inline bool isFailed() const + { + return isa(); + } /** * Returns the normal type of a Value. This only returns nThunk if * the Value hasn't been forceValue'd - * - * @param invalidIsThunk Instead of aborting an an invalid (probably - * 0, so uninitialized) internal type, return `nThunk`. */ - inline ValueType type(bool invalidIsThunk = false) const + inline ValueType type() const { switch (getInternalType()) { case tUninitialized: @@ -925,14 +1100,15 @@ public: return nExternal; case tFloat: return nFloat; + case tFailed: + return nFailed; case tThunk: case tApp: + case tPending: + case tAwaited: return nThunk; } - if (invalidIsThunk) - return nThunk; - else - unreachable(); + unreachable(); } /** @@ -1016,8 +1192,6 @@ public: setStorage(Lambda{.env = e, .fun = f}); } - inline void mkBlackhole(); - void mkPrimOp(PrimOp * p); inline void mkPrimOpApp(Value * l, Value * r) noexcept @@ -1040,6 +1214,11 @@ public: setStorage(n); } + inline void mkFailed() noexcept + { + setStorage(new Value::Failed{.ex = std::current_exception()}); + } + bool isList() const noexcept { return isa(); @@ -1059,8 +1238,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; @@ -1119,6 +1301,7 @@ public: return getStorage(); } + // FIXME: remove this since reading it is racy. ClosureThunk thunk() const noexcept { return getStorage(); @@ -1129,6 +1312,7 @@ public: return getStorage(); } + // FIXME: remove this since reading it is racy. FunctionApplicationThunk app() const noexcept { return getStorage(); @@ -1143,19 +1327,12 @@ public: { return getStorage().accessor; } -}; -extern ExprBlackHole eBlackHole; - -bool Value::isBlackhole() const -{ - return isThunk() && thunk().expr == (Expr *) &eBlackHole; -} - -void Value::mkBlackhole() -{ - mkThunk(nullptr, (Expr *) &eBlackHole); -} + Failed * failed() const noexcept + { + return getStorage(); + } +}; typedef std::vector> ValueVector; typedef std::unordered_map< diff --git a/src/libexpr/meson.build b/src/libexpr/meson.build index f5adafae031..9a4915e3523 100644 --- a/src/libexpr/meson.build +++ b/src/libexpr/meson.build @@ -149,11 +149,13 @@ sources = files( 'json-to-value.cc', 'lexer-helpers.cc', 'nixexpr.cc', + 'parallel-eval.cc', 'paths.cc', 'primops.cc', 'print-ambiguous.cc', 'print.cc', 'search-path.cc', + 'symbol-table.cc', 'value-to-json.cc', 'value-to-xml.cc', 'value/context.cc', diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index c0a25d1d4d6..d281b4883fe 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -13,8 +13,6 @@ namespace nix { unsigned long Expr::nrExprs = 0; -ExprBlackHole eBlackHole; - // FIXME: remove, because *symbols* are abstract and do not have a single // textual representation; see printIdentifier() std::ostream & operator<<(std::ostream & str, const SymbolStr & symbol) @@ -605,15 +603,6 @@ void ExprLambda::setDocComment(DocComment docComment) // belongs in the same conditional. body->setDocComment(docComment); } -}; - -/* Symbol table. */ - -size_t SymbolTable::totalSize() const -{ - size_t n = 0; - dump([&](SymbolStr s) { n += s.size(); }); - return n; } std::string DocComment::getInnerText(const PosTable & positions) const diff --git a/src/libexpr/parallel-eval.cc b/src/libexpr/parallel-eval.cc new file mode 100644 index 00000000000..eb2e5cb959f --- /dev/null +++ b/src/libexpr/parallel-eval.cc @@ -0,0 +1,224 @@ +#include "nix/expr/eval.hh" +#include "nix/expr/parallel-eval.hh" + +namespace nix { + +thread_local bool Executor::amWorkerThread{false}; + +Executor::Executor(const EvalSettings & evalSettings) + : enabled(evalSettings.evalCores > 1) +{ + debug("executor using %d threads", evalSettings.evalCores); + auto state(state_.lock()); + for (size_t n = 0; n < evalSettings.evalCores; ++n) + state->threads.push_back(std::thread([&]() { +#if NIX_USE_BOEHMGC + GC_stack_base sb; + GC_get_stack_base(&sb); + GC_register_my_thread(&sb); +#endif + worker(); +#if NIX_USE_BOEHMGC + GC_unregister_my_thread(); +#endif + })); +} + +Executor::~Executor() +{ + std::vector threads; + { + auto state(state_.lock()); + state->quit = true; + std::swap(threads, state->threads); + debug("executor shutting down with %d items left", state->queue.size()); + } + + wakeup.notify_all(); + + for (auto & thr : threads) + thr.join(); +} + +void Executor::worker() +{ + amWorkerThread = true; + + while (true) { + Item item; + + while (true) { + auto state(state_.lock()); + if (state->quit) + return; + if (!state->queue.empty()) { + item = std::move(state->queue.begin()->second); + state->queue.erase(state->queue.begin()); + break; + } + state.wait(wakeup); + } + + try { + item.work(); + item.promise.set_value(); + } catch (...) { + item.promise.set_exception(std::current_exception()); + } + } +} + +std::vector> Executor::spawn(std::vector> && items) +{ + if (items.empty()) + return {}; + + std::vector> futures; + + { + auto state(state_.lock()); + for (auto & item : items) { + std::promise promise; + futures.push_back(promise.get_future()); + thread_local std::random_device rd; + thread_local std::uniform_int_distribution dist(0, 1ULL << 48); + auto key = (uint64_t(item.second) << 48) | dist(rd); + state->queue.emplace(key, Item{.promise = std::move(promise), .work = std::move(item.first)}); + } + } + + wakeup.notify_all(); // FIXME + + return futures; +} + +void FutureVector::spawn(std::vector> && work) +{ + auto futures = executor.spawn(std::move(work)); + auto state(state_.lock()); + for (auto & future : futures) + state->futures.push_back(std::move(future)); +} + +void FutureVector::finishAll() +{ + while (true) { + std::vector> futures; + { + auto state(state_.lock()); + std::swap(futures, state->futures); + } + debug("got %d futures", futures.size()); + if (futures.empty()) + break; + std::exception_ptr ex; + for (auto & future : futures) + try { + future.get(); + } catch (...) { + if (ex) { + if (!getInterrupted()) + ignoreExceptionExceptInterrupt(); + } else + ex = std::current_exception(); + } + if (ex) + std::rethrow_exception(ex); + } +} + +struct WaiterDomain +{ + std::condition_variable cv; +}; + +static std::array, 128> waiterDomains; + +static Sync & getWaiterDomain(detail::ValueBase & v) +{ + auto domain = (((size_t) &v) >> 5) % waiterDomains.size(); + debug("HASH %x -> %d", &v, domain); + return waiterDomains[domain]; +} + +template<> +ValueStorage::PackedPointer ValueStorage::waitOnThunk(EvalState & state, bool awaited) +{ + state.nrThunksAwaited++; + + auto domain = getWaiterDomain(*this).lock(); + + if (awaited) { + /* Make sure that the value is still awaited, now that we're + holding the domain lock. */ + auto p0_ = p0.load(std::memory_order_acquire); + auto pd = static_cast(p0_ & discriminatorMask); + + /* If the value has been finalized in the meantime (i.e. is no + longer pending), we're done. */ + if (pd != pdAwaited) { + debug("VALUE DONE RIGHT AWAY 2 %x", this); + assert(pd != pdThunk && pd != pdPending); + return p0_; + } + } else { + /* Mark this value as being waited on. */ + PackedPointer p0_ = pdPending; + if (!p0.compare_exchange_strong(p0_, pdAwaited, std::memory_order_relaxed, std::memory_order_acquire)) { + /* If the value has been finalized in the meantime (i.e. is + no longer pending), we're done. */ + auto pd = static_cast(p0_ & discriminatorMask); + if (pd != pdAwaited) { + debug("VALUE DONE RIGHT AWAY %x", this); + assert(pd != pdThunk && pd != pdPending); + return p0_; + } + /* The value was already in the "waited on" state, so we're + not the only thread waiting on it. */ + debug("ALREADY AWAITED %x", this); + } else + debug("PENDING -> AWAITED %x", this); + } + + /* Wait for another thread to finish this value. */ + debug("AWAIT %x", this); + + if (state.settings.evalCores <= 1) + state.error("infinite recursion encountered") + .atPos(((Value &) *this).determinePos(noPos)) + .debugThrow(); + + state.nrThunksAwaitedSlow++; + state.currentlyWaiting++; + state.maxWaiting = std::max( + state.maxWaiting.load(std::memory_order_acquire), state.currentlyWaiting.load(std::memory_order_acquire)); + + auto now1 = std::chrono::steady_clock::now(); + + while (true) { + domain.wait(domain->cv); + debug("WAKEUP %x", this); + auto p0_ = p0.load(std::memory_order_acquire); + auto pd = static_cast(p0_ & discriminatorMask); + if (pd != pdAwaited) { + assert(pd != pdThunk && pd != pdPending); + auto now2 = std::chrono::steady_clock::now(); + state.usWaiting += std::chrono::duration_cast(now2 - now1).count(); + state.currentlyWaiting--; + return p0_; + } + state.nrSpuriousWakeups++; + } +} + +template<> +void ValueStorage::notifyWaiters() +{ + debug("NOTIFY %x", this); + + auto domain = getWaiterDomain(*this).lock(); + + domain->cv.notify_all(); +} + +} // namespace nix diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 3ce681e0093..1e0df590283 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -522,6 +522,7 @@ static void prim_typeOf(EvalState & state, const PosIdx pos, Value ** args, Valu t = "float"; break; case nThunk: + case nFailed: unreachable(); } v.mkString(t); @@ -3788,8 +3789,8 @@ static void anyOrAll(bool any, EvalState & state, const PosIdx pos, Value ** arg std::string_view errorCtx = any ? "while evaluating the return value of the function passed to builtins.any" : "while evaluating the return value of the function passed to builtins.all"; - Value vTmp; for (auto elem : args[1]->listView()) { + Value vTmp; state.callFunction(*args[0], *elem, vTmp, pos); bool res = state.forceBool(vTmp, pos, errorCtx); if (res == any) { @@ -5084,9 +5085,10 @@ void EvalState::createBaseEnv(const EvalSettings & evalSettings) )", }); - if (!settings.pureEval) { + if (!settings.pureEval) v.mkInt(time(0)); - } + else + v.mkNull(); addConstant( "__currentTime", v, @@ -5116,6 +5118,8 @@ void EvalState::createBaseEnv(const EvalSettings & evalSettings) if (!settings.pureEval) v.mkString(settings.getCurrentSystem()); + else + v.mkNull(); addConstant( "__currentSystem", v, diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index eedbd0e52bc..f263b2a0d73 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -129,7 +129,7 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value ** args std::optional inputAddressedMaybe; for (auto & attr : *args[0]->attrs()) { - const auto & attrName = state.symbols[attr.name]; + std::string_view attrName = state.symbols[attr.name]; auto attrHint = [&]() -> std::string { return fmt("while evaluating the attribute '%s' passed to builtins.fetchClosure", attrName); }; diff --git a/src/libexpr/print-ambiguous.cc b/src/libexpr/print-ambiguous.cc index b89746f093f..f80ef2b044b 100644 --- a/src/libexpr/print-ambiguous.cc +++ b/src/libexpr/print-ambiguous.cc @@ -78,6 +78,9 @@ void printAmbiguous(EvalState & state, Value & v, std::ostream & str, std::set"; diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index c9fdb4a093b..d7d360ad271 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -501,7 +501,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»"; @@ -512,6 +512,11 @@ class Printer } } + void printFailed(Value & v) + { + output << "«failed»"; + } + void printExternal(Value & v) { v.external()->print(output); @@ -587,6 +592,10 @@ class Printer printThunk(v); break; + case nFailed: + printFailed(v); + break; + case nExternal: printExternal(v); break; diff --git a/src/libexpr/symbol-table.cc b/src/libexpr/symbol-table.cc new file mode 100644 index 00000000000..90e7b746956 --- /dev/null +++ b/src/libexpr/symbol-table.cc @@ -0,0 +1,63 @@ +#include "nix/expr/symbol-table.hh" +#include "nix/util/logging.hh" + +#include + +namespace nix { + +#ifndef MAP_NORESERVE +# define MAP_NORESERVE 0 +#endif + +static void * allocateLazyMemory(size_t maxSize) +{ + auto p = mmap(nullptr, maxSize, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0); + if (p == MAP_FAILED) + throw SysError("allocating arena using mmap"); + return p; +} + +ContiguousArena::ContiguousArena(size_t maxSize) + : data((char *) allocateLazyMemory(maxSize)) + , maxSize(maxSize) +{ +} + +size_t ContiguousArena::allocate(size_t bytes) +{ + auto offset = size.fetch_add(bytes); + if (offset + bytes > maxSize) + throw Error("arena ran out of space"); + return offset; +} + +Symbol SymbolTable::create(std::string_view s) +{ + uint32_t idx; + + auto visit = [&](const SymbolStr & sym) { idx = ((const char *) sym.s) - arena.data; }; + + symbols.insert_and_visit(SymbolStr::Key{s, arena}, visit, visit); + + return Symbol(idx); +} + +SymbolStr::SymbolStr(const SymbolStr::Key & key) +{ + auto rawSize = sizeof(Value) + key.s.size() + 1; + auto size = ((rawSize + SymbolTable::alignment - 1) / SymbolTable::alignment) * SymbolTable::alignment; + + auto id = key.arena.allocate(size); + + auto v = (SymbolValue *) (const_cast(key.arena.data) + id); + auto s = (char *) (v + 1); + + memcpy(s, key.s.data(), key.s.size()); + s[key.s.size()] = 0; + + v->mkString(s, nullptr); + + this->s = v; +} + +} // namespace nix diff --git a/src/libexpr/value-to-json.cc b/src/libexpr/value-to-json.cc index 5154be020f5..25189200ef8 100644 --- a/src/libexpr/value-to-json.cc +++ b/src/libexpr/value-to-json.cc @@ -2,6 +2,7 @@ #include "nix/expr/eval-inline.hh" #include "nix/store/store-api.hh" #include "nix/util/signals.hh" +#include "nix/expr/parallel-eval.hh" #include #include @@ -13,94 +14,140 @@ using json = nlohmann::json; // TODO: rename. It doesn't print. json printValueAsJSON( - EvalState & state, bool strict, Value & v, const PosIdx pos, NixStringContext & context, bool copyToStore) + EvalState & state, bool strict, Value & v, const PosIdx pos, NixStringContext & context_, bool copyToStore) { - checkInterrupt(); + FutureVector futures(*state.executor); - if (strict) - state.forceValue(v, pos); + auto doParallel = state.executor->enabled && !Executor::amWorkerThread; - json out; + auto spawn = [&](auto work) { + if (doParallel) { + futures.spawn(0, [work{std::move(work)}]() { work(); }); + } else { + work(); + } + }; + + struct State + { + NixStringContext & context; + }; + + Sync state_{State{.context = context_}}; + + auto addContext = [&](const NixStringContext & context) { + auto state(state_.lock()); + for (auto & c : context) + state->context.insert(c); + }; + + std::function recurse; + + recurse = [&](json & res, Value & v, PosIdx pos) { + checkInterrupt(); + + if (strict) + state.forceValue(v, pos); - switch (v.type()) { + switch (v.type()) { - case nInt: - out = v.integer().value; - break; + case nInt: + res = v.integer().value; + break; - case nBool: - out = v.boolean(); - break; + case nBool: + res = v.boolean(); + break; - case nString: - copyContext(v, context); - out = v.c_str(); - break; + case nString: { + NixStringContext context; + copyContext(v, context); + addContext(context); + res = v.c_str(); + break; + } - case nPath: - if (copyToStore) - out = state.store->printStorePath(state.copyPathToStore(context, v.path(), v.determinePos(pos))); - else - out = v.path().path.abs(); - break; + case nPath: + if (copyToStore) { + NixStringContext context; + res = state.store->printStorePath(state.copyPathToStore(context, v.path(), v.determinePos(pos))); + addContext(context); + } else + res = v.path().path.abs(); + break; - case nNull: - // already initialized as null - break; + case nNull: + // already initialized as null + break; - case nAttrs: { - auto maybeString = state.tryAttrsToString(pos, v, context, false, false); - if (maybeString) { - out = *maybeString; + case nAttrs: { + NixStringContext context; + auto maybeString = state.tryAttrsToString(pos, v, context, false, false); + addContext(context); + if (maybeString) { + res = *maybeString; + break; + } + if (auto i = v.attrs()->get(state.sOutPath)) + return recurse(res, *i->value, i->pos); + else { + res = json::object(); + for (auto & a : v.attrs()->lexicographicOrder(state.symbols)) { + json & j = res.emplace(state.symbols[a->name], json()).first.value(); + spawn([&, strict, copyToStore, a]() { + try { + recurse(j, *a->value, a->pos); + } catch (Error & e) { + e.addTrace( + state.positions[a->pos], + HintFmt("while evaluating attribute '%1%'", state.symbols[a->name])); + throw; + } + }); + } + } break; } - if (auto i = v.attrs()->get(state.sOutPath)) - return printValueAsJSON(state, strict, *i->value, i->pos, context, copyToStore); - else { - out = json::object(); - for (auto & a : v.attrs()->lexicographicOrder(state.symbols)) { + + case nList: { + res = json::array(); + for (const auto & [i, elem] : enumerate(v.listView())) { try { - out.emplace( - state.symbols[a->name], - printValueAsJSON(state, strict, *a->value, a->pos, context, copyToStore)); + res.push_back(json()); + recurse(res.back(), *elem, pos); } catch (Error & e) { - e.addTrace( - state.positions[a->pos], HintFmt("while evaluating attribute '%1%'", state.symbols[a->name])); + e.addTrace(state.positions[pos], HintFmt("while evaluating list element at index %1%", i)); throw; } } + break; } - break; - } - case nList: { - out = json::array(); - int i = 0; - for (auto elem : v.listView()) { - try { - out.push_back(printValueAsJSON(state, strict, *elem, pos, context, copyToStore)); - } catch (Error & e) { - e.addTrace(state.positions[pos], HintFmt("while evaluating list element at index %1%", i)); - throw; - } - i++; + case nExternal: { + NixStringContext context; + res = v.external()->printValueAsJSON(state, strict, context, copyToStore); + addContext(context); + break; } - break; - } - case nExternal: - return v.external()->printValueAsJSON(state, strict, context, copyToStore); - break; + case nFloat: + res = v.fpoint(); + break; - case nFloat: - out = v.fpoint(); - break; + case nThunk: + case nFailed: + case nFunction: + state.error("cannot convert %1% to JSON", showType(v)).atPos(v.determinePos(pos)).debugThrow(); + } + }; - case nThunk: - case nFunction: - state.error("cannot convert %1% to JSON", showType(v)).atPos(v.determinePos(pos)).debugThrow(); - } - return out; + json res; + + recurse(res, v, pos); + + futures.finishAll(); + + return res; } void printValueAsJSON( diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index b3b986dae78..175186164ce 100644 --- a/src/libexpr/value-to-xml.cc +++ b/src/libexpr/value-to-xml.cc @@ -170,6 +170,11 @@ static void printValueAsXML( case nThunk: doc.writeEmptyElement("unevaluated"); + break; + + case nFailed: + doc.writeEmptyElement("failed"); + break; } } diff --git a/src/libfetchers/filtering-source-accessor.cc b/src/libfetchers/filtering-source-accessor.cc index e9b6c7d0ea4..daf4f97edc2 100644 --- a/src/libfetchers/filtering-source-accessor.cc +++ b/src/libfetchers/filtering-source-accessor.cc @@ -1,4 +1,5 @@ #include "nix/fetchers/filtering-source-accessor.hh" +#include "nix/util/sync.hh" namespace nix { @@ -74,8 +75,8 @@ void FilteringSourceAccessor::checkAccess(const CanonPath & path) struct AllowListSourceAccessorImpl : AllowListSourceAccessor { - std::set allowedPrefixes; - std::unordered_set allowedPaths; + SharedSync> allowedPrefixes; + SharedSync> allowedPaths; AllowListSourceAccessorImpl( ref next, @@ -90,12 +91,12 @@ struct AllowListSourceAccessorImpl : AllowListSourceAccessor bool isAllowed(const CanonPath & path) override { - return allowedPaths.contains(path) || path.isAllowed(allowedPrefixes); + return allowedPaths.readLock()->contains(path) || path.isAllowed(*allowedPrefixes.readLock()); } void allowPrefix(CanonPath prefix) override { - allowedPrefixes.insert(std::move(prefix)); + allowedPrefixes.lock()->insert(std::move(prefix)); } }; diff --git a/src/libflake/flake.cc b/src/libflake/flake.cc index fcd804adb4f..80d5c79fca3 100644 --- a/src/libflake/flake.cc +++ b/src/libflake/flake.cc @@ -28,7 +28,7 @@ namespace flake { static void forceTrivialValue(EvalState & state, Value & value, const PosIdx pos) { - if (value.isThunk() && value.isTrivial()) + if (!value.isFinished() && value.isTrivial()) state.forceValue(value, pos); } diff --git a/src/libstore/include/nix/store/remote-store.hh b/src/libstore/include/nix/store/remote-store.hh index 76591cf9390..0da9ec48b77 100644 --- a/src/libstore/include/nix/store/remote-store.hh +++ b/src/libstore/include/nix/store/remote-store.hh @@ -22,7 +22,7 @@ struct RemoteStoreConfig : virtual StoreConfig using StoreConfig::StoreConfig; const Setting maxConnections{ - this, 1, "max-connections", "Maximum number of concurrent connections to the Nix daemon."}; + this, 64, "max-connections", "Maximum number of concurrent connections to the Nix daemon."}; const Setting maxConnectionAge{ this, diff --git a/src/libutil/include/nix/util/pos-table.hh b/src/libutil/include/nix/util/pos-table.hh index d944b135317..4ef4b9af4cb 100644 --- a/src/libutil/include/nix/util/pos-table.hh +++ b/src/libutil/include/nix/util/pos-table.hh @@ -49,20 +49,29 @@ private: */ using LinesCache = LRUCache; - std::map origins; - mutable Sync linesCache; + // FIXME: this could be made lock-free (at least for access) if we + // have a data structure where pointers to existing positions are + // never invalidated. + struct State + { + std::map origins; + }; + + SharedSync state_; + const Origin * resolve(PosIdx p) const { if (p.id == 0) return nullptr; + auto state(state_.readLock()); const auto idx = p.id - 1; - /* we want the last key <= idx, so we'll take prev(first key > idx). - this is guaranteed to never rewind origin.begin because the first - key is always 0. */ - const auto pastOrigin = origins.upper_bound(idx); + /* We want the last key <= idx, so we'll take prev(first key > + idx). This is guaranteed to never rewind origin.begin + because the first key is always 0. */ + const auto pastOrigin = state->origins.upper_bound(idx); return &std::prev(pastOrigin)->second; } @@ -74,15 +83,16 @@ public: Origin addOrigin(Pos::Origin origin, size_t size) { + auto state(state_.lock()); uint32_t offset = 0; - if (auto it = origins.rbegin(); it != origins.rend()) + if (auto it = state->origins.rbegin(); it != state->origins.rend()) offset = it->first + it->second.size; // +1 because all PosIdx are offset by 1 to begin with, and // another +1 to ensure that all origins can point to EOF, eg // on (invalid) empty inputs. if (2 + offset + size < offset) return Origin{origin, offset, 0}; - return origins.emplace(offset, Origin{origin, offset, size}).first->second; + return state->origins.emplace(offset, Origin{origin, offset, size}).first->second; } PosIdx add(const Origin & origin, size_t offset) diff --git a/src/libutil/include/nix/util/util.hh b/src/libutil/include/nix/util/util.hh index 015086d39ea..bc0a6fe7c88 100644 --- a/src/libutil/include/nix/util/util.hh +++ b/src/libutil/include/nix/util/util.hh @@ -224,6 +224,15 @@ typename T::mapped_type * get(T & map, const typename T::key_type & key) return &i->second; } +template +std::optional getOptional(const T & map, const typename T::key_type & key) +{ + auto i = map.find(key); + if (i == map.end()) + return std::nullopt; + return {i->second}; +} + /** * 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 73a08116dd5..b932f6ab5e5 100644 --- a/src/libutil/posix-source-accessor.cc +++ b/src/libutil/posix-source-accessor.cc @@ -3,7 +3,7 @@ #include "nix/util/signals.hh" #include "nix/util/sync.hh" -#include +#include namespace nix { @@ -88,25 +88,23 @@ bool PosixSourceAccessor::pathExists(const CanonPath & path) std::optional PosixSourceAccessor::cachedLstat(const CanonPath & path) { - static SharedSync>> _cache; + using Cache = boost::concurrent_flat_map>; + static Cache cache; // Note: we convert std::filesystem::path to Path because the // former is not hashable on libc++. Path absPath = makeAbsPath(path).string(); - { - auto cache(_cache.readLock()); - auto i = cache->find(absPath); - if (i != cache->end()) - return i->second; - } + std::optional res; + cache.cvisit(absPath, [&](auto & x) { res.emplace(x.second); }); + if (res) + return *res; auto st = nix::maybeLstat(absPath.c_str()); - auto cache(_cache.lock()); - if (cache->size() >= 16384) - cache->clear(); - cache->emplace(absPath, st); + if (cache.size() >= 16384) + cache.clear(); + cache.emplace(absPath, st); return st; } diff --git a/src/nix/flake.cc b/src/nix/flake.cc index f0a8d3499cb..81edeb8ae84 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -17,6 +17,7 @@ #include "nix/util/users.hh" #include "nix/fetchers/fetch-to-store.hh" #include "nix/store/local-fs-store.hh" +#include "nix/expr/parallel-eval.hh" #include #include @@ -366,7 +367,7 @@ struct CmdFlakeCheck : FlakeCommand auto flake = lockFlake(); auto localSystem = std::string(settings.thisSystem.get()); - bool hasErrors = false; + std::atomic_bool hasErrors = false; auto reportError = [&](const Error & e) { try { throw e; @@ -381,7 +382,7 @@ struct CmdFlakeCheck : FlakeCommand } }; - StringSet omittedSystems; + Sync omittedSystems; // FIXME: rewrite to use EvalCache. @@ -400,7 +401,7 @@ struct CmdFlakeCheck : FlakeCommand auto checkSystemType = [&](std::string_view system, const PosIdx pos) { if (!checkAllSystems && system != localSystem) { - omittedSystems.insert(std::string(system)); + omittedSystems.lock()->insert(std::string(system)); return false; } else { return true; @@ -432,6 +433,8 @@ struct CmdFlakeCheck : FlakeCommand std::vector drvPaths; + FutureVector futures(*state->executor); + auto checkApp = [&](const std::string & attrPath, Value & v, const PosIdx pos) { try { Activity act(*logger, lvlInfo, actUnknown, fmt("checking app '%s'", attrPath)); @@ -500,9 +503,9 @@ struct CmdFlakeCheck : FlakeCommand } }; - std::function checkHydraJobs; + std::function checkHydraJobs; - checkHydraJobs = [&](std::string_view attrPath, Value & v, const PosIdx pos) { + checkHydraJobs = [&](const std::string & attrPath, Value & v, const PosIdx pos) { try { Activity act(*logger, lvlInfo, actUnknown, fmt("checking Hydra job '%s'", attrPath)); state->forceAttrs(v, pos, ""); @@ -510,15 +513,16 @@ struct CmdFlakeCheck : FlakeCommand if (state->isDerivation(v)) throw Error("jobset should not be a derivation at top-level"); - for (auto & attr : *v.attrs()) { - state->forceAttrs(*attr.value, attr.pos, ""); - auto attrPath2 = concatStrings(attrPath, ".", state->symbols[attr.name]); - if (state->isDerivation(*attr.value)) { - Activity act(*logger, lvlInfo, actUnknown, fmt("checking Hydra job '%s'", attrPath2)); - checkDerivation(attrPath2, *attr.value, attr.pos); - } else - checkHydraJobs(attrPath2, *attr.value, attr.pos); - } + for (auto & attr : *v.attrs()) + futures.spawn(1, [&, attrPath]() { + state->forceAttrs(*attr.value, attr.pos, ""); + auto attrPath2 = concatStrings(attrPath, ".", state->symbols[attr.name]); + if (state->isDerivation(*attr.value)) { + Activity act(*logger, lvlInfo, actUnknown, fmt("checking Hydra job '%s'", attrPath2)); + checkDerivation(attrPath2, *attr.value, attr.pos); + } else + checkHydraJobs(attrPath2, *attr.value, attr.pos); + }); } catch (Error & e) { e.addTrace(resolve(pos), HintFmt("while checking the Hydra jobset '%s'", attrPath)); @@ -586,213 +590,221 @@ struct CmdFlakeCheck : FlakeCommand } }; - { + auto checkFlake = [&]() { Activity act(*logger, lvlInfo, actUnknown, "evaluating flake"); auto vFlake = state->allocValue(); flake::callFlake(*state, flake, *vFlake); enumerateOutputs(*state, *vFlake, [&](std::string_view name, Value & vOutput, const PosIdx pos) { - Activity act(*logger, lvlInfo, actUnknown, fmt("checking flake output '%s'", name)); - - try { - evalSettings.enableImportFromDerivation.setDefault(name != "hydraJobs"); - - state->forceValue(vOutput, pos); - - std::string_view replacement = name == "defaultPackage" ? "packages..default" - : name == "defaultApp" ? "apps..default" - : name == "defaultTemplate" ? "templates.default" - : name == "defaultBundler" ? "bundlers..default" - : name == "overlay" ? "overlays.default" - : name == "devShell" ? "devShells..default" - : name == "nixosModule" ? "nixosModules.default" - : ""; - if (replacement != "") - warn("flake output attribute '%s' is deprecated; use '%s' instead", name, replacement); - - if (name == "checks") { - state->forceAttrs(vOutput, pos, ""); - for (auto & attr : *vOutput.attrs()) { - const auto & attr_name = state->symbols[attr.name]; - checkSystemName(attr_name, attr.pos); - if (checkSystemType(attr_name, attr.pos)) { - state->forceAttrs(*attr.value, attr.pos, ""); - for (auto & attr2 : *attr.value->attrs()) { - auto drvPath = checkDerivation( - fmt("%s.%s.%s", name, attr_name, state->symbols[attr2.name]), - *attr2.value, - attr2.pos); - if (drvPath && attr_name == settings.thisSystem.get()) { - auto path = DerivedPath::Built{ - .drvPath = makeConstantStorePathRef(*drvPath), - .outputs = OutputsSpec::All{}, - }; - drvPaths.push_back(std::move(path)); + futures.spawn(2, [&, name, pos]() { + Activity act(*logger, lvlInfo, actUnknown, fmt("checking flake output '%s'", name)); + + try { + evalSettings.enableImportFromDerivation.setDefault(name != "hydraJobs"); + + state->forceValue(vOutput, pos); + + std::string_view replacement = name == "defaultPackage" ? "packages..default" + : name == "defaultApp" ? "apps..default" + : name == "defaultTemplate" ? "templates.default" + : name == "defaultBundler" ? "bundlers..default" + : name == "overlay" ? "overlays.default" + : name == "devShell" ? "devShells..default" + : name == "nixosModule" ? "nixosModules.default" + : ""; + if (replacement != "") + warn("flake output attribute '%s' is deprecated; use '%s' instead", name, replacement); + + if (name == "checks") { + state->forceAttrs(vOutput, pos, ""); + for (auto & attr : *vOutput.attrs()) + futures.spawn(3, [&, name]() { + const auto & attr_name = state->symbols[attr.name]; + checkSystemName(attr_name, attr.pos); + if (checkSystemType(attr_name, attr.pos)) { + state->forceAttrs(*attr.value, attr.pos, ""); + for (auto & attr2 : *attr.value->attrs()) { + auto drvPath = checkDerivation( + fmt("%s.%s.%s", name, attr_name, state->symbols[attr2.name]), + *attr2.value, + attr2.pos); + if (drvPath && attr_name == settings.thisSystem.get()) { + auto path = DerivedPath::Built{ + .drvPath = makeConstantStorePathRef(*drvPath), + .outputs = OutputsSpec::All{}, + }; + drvPaths.push_back(std::move(path)); + } + } } - } - } + }); } - } - else if (name == "formatter") { - state->forceAttrs(vOutput, pos, ""); - for (auto & attr : *vOutput.attrs()) { - const auto & attr_name = state->symbols[attr.name]; - checkSystemName(attr_name, attr.pos); - if (checkSystemType(attr_name, attr.pos)) { - checkDerivation(fmt("%s.%s", name, attr_name), *attr.value, attr.pos); - }; + else if (name == "formatter") { + state->forceAttrs(vOutput, pos, ""); + for (auto & attr : *vOutput.attrs()) { + const auto & attr_name = state->symbols[attr.name]; + checkSystemName(attr_name, attr.pos); + if (checkSystemType(attr_name, attr.pos)) { + checkDerivation(fmt("%s.%s", name, attr_name), *attr.value, attr.pos); + }; + } } - } - else if (name == "packages" || name == "devShells") { - state->forceAttrs(vOutput, pos, ""); - for (auto & attr : *vOutput.attrs()) { - const auto & attr_name = state->symbols[attr.name]; - checkSystemName(attr_name, attr.pos); - if (checkSystemType(attr_name, attr.pos)) { - state->forceAttrs(*attr.value, attr.pos, ""); - for (auto & attr2 : *attr.value->attrs()) - checkDerivation( - fmt("%s.%s.%s", name, attr_name, state->symbols[attr2.name]), - *attr2.value, - attr2.pos); - }; + else if (name == "packages" || name == "devShells") { + state->forceAttrs(vOutput, pos, ""); + for (auto & attr : *vOutput.attrs()) + futures.spawn(3, [&, name]() { + const auto & attr_name = state->symbols[attr.name]; + checkSystemName(attr_name, attr.pos); + if (checkSystemType(attr_name, attr.pos)) { + state->forceAttrs(*attr.value, attr.pos, ""); + for (auto & attr2 : *attr.value->attrs()) + checkDerivation( + fmt("%s.%s.%s", name, attr_name, state->symbols[attr2.name]), + *attr2.value, + attr2.pos); + }; + }); } - } - else if (name == "apps") { - state->forceAttrs(vOutput, pos, ""); - for (auto & attr : *vOutput.attrs()) { - const auto & attr_name = state->symbols[attr.name]; - checkSystemName(attr_name, attr.pos); - if (checkSystemType(attr_name, attr.pos)) { - state->forceAttrs(*attr.value, attr.pos, ""); - for (auto & attr2 : *attr.value->attrs()) - checkApp( - fmt("%s.%s.%s", name, attr_name, state->symbols[attr2.name]), - *attr2.value, - attr2.pos); - }; + else if (name == "apps") { + state->forceAttrs(vOutput, pos, ""); + for (auto & attr : *vOutput.attrs()) { + const auto & attr_name = state->symbols[attr.name]; + checkSystemName(attr_name, attr.pos); + if (checkSystemType(attr_name, attr.pos)) { + state->forceAttrs(*attr.value, attr.pos, ""); + for (auto & attr2 : *attr.value->attrs()) + checkApp( + fmt("%s.%s.%s", name, attr_name, state->symbols[attr2.name]), + *attr2.value, + attr2.pos); + }; + } } - } - else if (name == "defaultPackage" || name == "devShell") { - state->forceAttrs(vOutput, pos, ""); - for (auto & attr : *vOutput.attrs()) { - const auto & attr_name = state->symbols[attr.name]; - checkSystemName(attr_name, attr.pos); - if (checkSystemType(attr_name, attr.pos)) { - checkDerivation(fmt("%s.%s", name, attr_name), *attr.value, attr.pos); - }; + else if (name == "defaultPackage" || name == "devShell") { + state->forceAttrs(vOutput, pos, ""); + for (auto & attr : *vOutput.attrs()) { + const auto & attr_name = state->symbols[attr.name]; + checkSystemName(attr_name, attr.pos); + if (checkSystemType(attr_name, attr.pos)) { + checkDerivation(fmt("%s.%s", name, attr_name), *attr.value, attr.pos); + }; + } } - } - else if (name == "defaultApp") { - state->forceAttrs(vOutput, pos, ""); - for (auto & attr : *vOutput.attrs()) { - const auto & attr_name = state->symbols[attr.name]; - checkSystemName(attr_name, attr.pos); - if (checkSystemType(attr_name, attr.pos)) { - checkApp(fmt("%s.%s", name, attr_name), *attr.value, attr.pos); - }; + else if (name == "defaultApp") { + state->forceAttrs(vOutput, pos, ""); + for (auto & attr : *vOutput.attrs()) { + const auto & attr_name = state->symbols[attr.name]; + checkSystemName(attr_name, attr.pos); + if (checkSystemType(attr_name, attr.pos)) { + checkApp(fmt("%s.%s", name, attr_name), *attr.value, attr.pos); + }; + } } - } - else if (name == "legacyPackages") { - state->forceAttrs(vOutput, pos, ""); - for (auto & attr : *vOutput.attrs()) { - checkSystemName(state->symbols[attr.name], attr.pos); - checkSystemType(state->symbols[attr.name], attr.pos); - // FIXME: do getDerivations? + else if (name == "legacyPackages") { + state->forceAttrs(vOutput, pos, ""); + for (auto & attr : *vOutput.attrs()) { + checkSystemName(state->symbols[attr.name], attr.pos); + checkSystemType(state->symbols[attr.name], attr.pos); + // FIXME: do getDerivations? + } } - } - else if (name == "overlay") - checkOverlay(name, vOutput, pos); + else if (name == "overlay") + checkOverlay(name, vOutput, pos); - else if (name == "overlays") { - state->forceAttrs(vOutput, pos, ""); - for (auto & attr : *vOutput.attrs()) - checkOverlay(fmt("%s.%s", name, state->symbols[attr.name]), *attr.value, attr.pos); - } + else if (name == "overlays") { + state->forceAttrs(vOutput, pos, ""); + for (auto & attr : *vOutput.attrs()) + checkOverlay(fmt("%s.%s", name, state->symbols[attr.name]), *attr.value, attr.pos); + } - else if (name == "nixosModule") - checkModule(name, vOutput, pos); + else if (name == "nixosModule") + checkModule(name, vOutput, pos); - else if (name == "nixosModules") { - state->forceAttrs(vOutput, pos, ""); - for (auto & attr : *vOutput.attrs()) - checkModule(fmt("%s.%s", name, state->symbols[attr.name]), *attr.value, attr.pos); - } + else if (name == "nixosModules") { + state->forceAttrs(vOutput, pos, ""); + for (auto & attr : *vOutput.attrs()) + checkModule(fmt("%s.%s", name, state->symbols[attr.name]), *attr.value, attr.pos); + } - else if (name == "nixosConfigurations") { - state->forceAttrs(vOutput, pos, ""); - for (auto & attr : *vOutput.attrs()) - checkNixOSConfiguration( - fmt("%s.%s", name, state->symbols[attr.name]), *attr.value, attr.pos); - } + else if (name == "nixosConfigurations") { + state->forceAttrs(vOutput, pos, ""); + for (auto & attr : *vOutput.attrs()) + checkNixOSConfiguration( + fmt("%s.%s", name, state->symbols[attr.name]), *attr.value, attr.pos); + } - else if (name == "hydraJobs") - checkHydraJobs(name, vOutput, pos); + else if (name == "hydraJobs") + checkHydraJobs(std::string(name), vOutput, pos); - else if (name == "defaultTemplate") - checkTemplate(name, vOutput, pos); + else if (name == "defaultTemplate") + checkTemplate(name, vOutput, pos); - else if (name == "templates") { - state->forceAttrs(vOutput, pos, ""); - for (auto & attr : *vOutput.attrs()) - checkTemplate(fmt("%s.%s", name, state->symbols[attr.name]), *attr.value, attr.pos); - } + else if (name == "templates") { + state->forceAttrs(vOutput, pos, ""); + for (auto & attr : *vOutput.attrs()) + checkTemplate(fmt("%s.%s", name, state->symbols[attr.name]), *attr.value, attr.pos); + } - else if (name == "defaultBundler") { - state->forceAttrs(vOutput, pos, ""); - for (auto & attr : *vOutput.attrs()) { - const auto & attr_name = state->symbols[attr.name]; - checkSystemName(attr_name, attr.pos); - if (checkSystemType(attr_name, attr.pos)) { - checkBundler(fmt("%s.%s", name, attr_name), *attr.value, attr.pos); - }; + else if (name == "defaultBundler") { + state->forceAttrs(vOutput, pos, ""); + for (auto & attr : *vOutput.attrs()) { + const auto & attr_name = state->symbols[attr.name]; + checkSystemName(attr_name, attr.pos); + if (checkSystemType(attr_name, attr.pos)) { + checkBundler(fmt("%s.%s", name, attr_name), *attr.value, attr.pos); + }; + } } - } - else if (name == "bundlers") { - state->forceAttrs(vOutput, pos, ""); - for (auto & attr : *vOutput.attrs()) { - const auto & attr_name = state->symbols[attr.name]; - checkSystemName(attr_name, attr.pos); - if (checkSystemType(attr_name, attr.pos)) { - state->forceAttrs(*attr.value, attr.pos, ""); - for (auto & attr2 : *attr.value->attrs()) { - checkBundler( - fmt("%s.%s.%s", name, attr_name, state->symbols[attr2.name]), - *attr2.value, - attr2.pos); - } - }; + else if (name == "bundlers") { + state->forceAttrs(vOutput, pos, ""); + for (auto & attr : *vOutput.attrs()) { + const auto & attr_name = state->symbols[attr.name]; + checkSystemName(attr_name, attr.pos); + if (checkSystemType(attr_name, attr.pos)) { + state->forceAttrs(*attr.value, attr.pos, ""); + for (auto & attr2 : *attr.value->attrs()) { + checkBundler( + fmt("%s.%s.%s", name, attr_name, state->symbols[attr2.name]), + *attr2.value, + attr2.pos); + } + }; + } } - } - else if ( - name == "lib" || name == "darwinConfigurations" || name == "darwinModules" - || name == "flakeModule" || name == "flakeModules" || name == "herculesCI" - || name == "homeConfigurations" || name == "homeModule" || name == "homeModules" - || name == "nixopsConfigurations") - // Known but unchecked community attribute - ; + else if ( + name == "lib" || name == "darwinConfigurations" || name == "darwinModules" + || name == "flakeModule" || name == "flakeModules" || name == "herculesCI" + || name == "homeConfigurations" || name == "homeModule" || name == "homeModules" + || name == "nixopsConfigurations") + // Known but unchecked community attribute + ; - else - warn("unknown flake output '%s'", name); + else + warn("unknown flake output '%s'", name); - } catch (Error & e) { - e.addTrace(resolve(pos), HintFmt("while checking flake output '%s'", name)); - reportError(e); - } + } catch (Error & e) { + e.addTrace(resolve(pos), HintFmt("while checking flake output '%s'", name)); + reportError(e); + } + }); }); - } + }; + + futures.spawn(1, checkFlake); + futures.finishAll(); if (build && !drvPaths.empty()) { + // FIXME: should start building while evaluating. Activity act(*logger, lvlInfo, actUnknown, fmt("running %d flake checks", drvPaths.size())); auto missing = store->queryMissing(drvPaths); @@ -824,12 +836,12 @@ struct CmdFlakeCheck : FlakeCommand if (hasErrors) throw Error("some errors were encountered during the evaluation"); - if (!omittedSystems.empty()) { + if (!omittedSystems.lock()->empty()) { // TODO: empty system is not visible; render all as nix strings? warn( "The check omitted these incompatible systems: %s\n" "Use '--all-systems' to check all.", - concatStringsSep(", ", omittedSystems)); + concatStringsSep(", ", *omittedSystems.lock())); }; }; }; @@ -1167,125 +1179,57 @@ struct CmdFlakeShow : FlakeCommand, MixJSON auto flake = std::make_shared(lockFlake()); auto localSystem = std::string(settings.thisSystem.get()); - std::function & attrPath, const Symbol & attr)> - hasContent; - - // For frameworks it's important that structures are as lazy as possible - // to prevent infinite recursions, performance issues and errors that - // aren't related to the thing to evaluate. As a consequence, they have - // to emit more attributes than strictly (sic) necessary. - // However, these attributes with empty values are not useful to the user - // so we omit them. - hasContent = - [&](eval_cache::AttrCursor & visitor, const std::vector & attrPath, const Symbol & attr) -> bool { - auto attrPath2(attrPath); - attrPath2.push_back(attr); - auto attrPathS = state->symbols.resolve(attrPath2); - const auto & attrName = state->symbols[attr]; - - auto visitor2 = visitor.getAttr(attrName); - - try { - if ((attrPathS[0] == "apps" || attrPathS[0] == "checks" || attrPathS[0] == "devShells" - || attrPathS[0] == "legacyPackages" || attrPathS[0] == "packages") - && (attrPathS.size() == 1 || attrPathS.size() == 2)) { - for (const auto & subAttr : visitor2->getAttrs()) { - if (hasContent(*visitor2, attrPath2, subAttr)) { - return true; - } - } - return false; - } - - if ((attrPathS.size() == 1) - && (attrPathS[0] == "formatter" || attrPathS[0] == "nixosConfigurations" - || attrPathS[0] == "nixosModules" || attrPathS[0] == "overlays")) { - for (const auto & subAttr : visitor2->getAttrs()) { - if (hasContent(*visitor2, attrPath2, subAttr)) { - return true; - } - } - return false; - } + auto cache = openEvalCache(*state, flake); - // If we don't recognize it, it's probably content - return true; - } catch (EvalError & e) { - // Some attrs may contain errors, e.g. legacyPackages of - // nixpkgs. We still want to recurse into it, instead of - // skipping it at all. - return true; - } - }; + auto j = nlohmann::json::object(); - std::function & attrPath, - const std::string & headerPrefix, - const std::string & nextPrefix)> - visit; + std::function visit; - visit = [&](eval_cache::AttrCursor & visitor, - const std::vector & attrPath, - const std::string & headerPrefix, - const std::string & nextPrefix) -> nlohmann::json { - auto j = nlohmann::json::object(); + FutureVector futures(*state->executor); + visit = [&](eval_cache::AttrCursor & visitor, nlohmann::json & j) { + auto attrPath = visitor.getAttrPath(); auto attrPathS = state->symbols.resolve(attrPath); Activity act(*logger, lvlInfo, actUnknown, fmt("evaluating '%s'", concatStringsSep(".", attrPathS))); try { auto recurse = [&]() { - if (!json) - logger->cout("%s", headerPrefix); - std::vector attrs; for (const auto & attr : visitor.getAttrs()) { - if (hasContent(visitor, attrPath, attr)) - attrs.push_back(attr); - } - - for (const auto & [i, attr] : enumerate(attrs)) { const auto & attrName = state->symbols[attr]; - bool last = i + 1 == attrs.size(); auto visitor2 = visitor.getAttr(attrName); - auto attrPath2(attrPath); - attrPath2.push_back(attr); - auto j2 = visit( - *visitor2, - attrPath2, - fmt(ANSI_GREEN "%s%s" ANSI_NORMAL ANSI_BOLD "%s" ANSI_NORMAL, - nextPrefix, - last ? treeLast : treeConn, - attrName), - nextPrefix + (last ? treeNull : treeLine)); - if (json) - j.emplace(attrName, std::move(j2)); + auto & j2 = *j.emplace(attrName, nlohmann::json::object()).first; + futures.spawn(1, [&, visitor2]() { visit(*visitor2, j2); }); } }; auto showDerivation = [&]() { auto name = visitor.getAttr(state->sName)->getString(); - - if (json) { - std::optional description; - if (auto aMeta = visitor.maybeGetAttr(state->sMeta)) { - if (auto aDescription = aMeta->maybeGetAttr(state->sDescription)) - description = aDescription->getString(); - } - j.emplace("type", "derivation"); - j.emplace("name", name); - j.emplace("description", description ? *description : ""); - } else { - logger->cout( - "%s: %s '%s'", - headerPrefix, + std::optional description; + if (auto aMeta = visitor.maybeGetAttr(state->sMeta)) { + if (auto aDescription = aMeta->maybeGetAttr(state->sDescription)) + description = aDescription->getString(); + } + j.emplace("type", "derivation"); + if (!json) + j.emplace( + "subtype", attrPath.size() == 2 && attrPathS[0] == "devShell" ? "development environment" : attrPath.size() >= 2 && attrPathS[0] == "devShells" ? "development environment" : attrPath.size() == 3 && attrPathS[0] == "checks" ? "derivation" : attrPath.size() >= 1 && attrPathS[0] == "hydraJobs" ? "derivation" - : "package", - name); + : "package"); + j.emplace("name", name); + if (description) + j.emplace("description", *description); + }; + + auto omit = [&](std::string_view flag) { + if (json) + logger->warn(fmt("%s omitted (use '%s' to show)", concatStringsSep(".", attrPathS), flag)); + else { + j.emplace("type", "omitted"); + j.emplace("message", fmt(ANSI_WARNING "omitted" ANSI_NORMAL " (use '%s' to show)", flag)); } }; @@ -1307,14 +1251,7 @@ struct CmdFlakeShow : FlakeCommand, MixJSON || (attrPath.size() == 3 && (attrPathS[0] == "checks" || attrPathS[0] == "packages" || attrPathS[0] == "devShells"))) { if (!showAllSystems && std::string(attrPathS[1]) != localSystem) { - if (!json) - logger->cout( - fmt("%s " ANSI_WARNING "omitted" ANSI_NORMAL " (use '--all-systems' to show)", - headerPrefix)); - else { - logger->warn( - fmt("%s omitted (use '--all-systems' to show)", concatStringsSep(".", attrPathS))); - } + omit("--all-systems"); } else { try { if (visitor.isDerivation()) @@ -1322,15 +1259,8 @@ struct CmdFlakeShow : FlakeCommand, MixJSON else throw Error("expected a derivation"); } catch (IFDError & e) { - if (!json) { - logger->cout( - fmt("%s " ANSI_WARNING "omitted due to use of import from derivation" ANSI_NORMAL, - headerPrefix)); - } else { - logger->warn( - fmt("%s omitted due to use of import from derivation", - concatStringsSep(".", attrPathS))); - } + logger->warn(fmt( + "%s omitted due to use of import from derivation", concatStringsSep(".", attrPathS))); } } } @@ -1342,14 +1272,8 @@ struct CmdFlakeShow : FlakeCommand, MixJSON else recurse(); } catch (IFDError & e) { - if (!json) { - logger->cout( - fmt("%s " ANSI_WARNING "omitted due to use of import from derivation" ANSI_NORMAL, - headerPrefix)); - } else { - logger->warn(fmt( - "%s omitted due to use of import from derivation", concatStringsSep(".", attrPathS))); - } + logger->warn( + fmt("%s omitted due to use of import from derivation", concatStringsSep(".", attrPathS))); } } @@ -1357,21 +1281,9 @@ struct CmdFlakeShow : FlakeCommand, MixJSON if (attrPath.size() == 1) recurse(); else if (!showLegacy) { - if (!json) - logger->cout(fmt( - "%s " ANSI_WARNING "omitted" ANSI_NORMAL " (use '--legacy' to show)", headerPrefix)); - else { - logger->warn(fmt("%s omitted (use '--legacy' to show)", concatStringsSep(".", attrPathS))); - } + omit("--legacy"); } else if (!showAllSystems && std::string(attrPathS[1]) != localSystem) { - if (!json) - logger->cout( - fmt("%s " ANSI_WARNING "omitted" ANSI_NORMAL " (use '--all-systems' to show)", - headerPrefix)); - else { - logger->warn( - fmt("%s omitted (use '--all-systems' to show)", concatStringsSep(".", attrPathS))); - } + omit("--all-systems"); } else { try { if (visitor.isDerivation()) @@ -1380,15 +1292,8 @@ struct CmdFlakeShow : FlakeCommand, MixJSON // FIXME: handle recurseIntoAttrs recurse(); } catch (IFDError & e) { - if (!json) { - logger->cout( - fmt("%s " ANSI_WARNING "omitted due to use of import from derivation" ANSI_NORMAL, - headerPrefix)); - } else { - logger->warn( - fmt("%s omitted due to use of import from derivation", - concatStringsSep(".", attrPathS))); - } + logger->warn(fmt( + "%s omitted due to use of import from derivation", concatStringsSep(".", attrPathS))); } } } @@ -1404,28 +1309,17 @@ struct CmdFlakeShow : FlakeCommand, MixJSON } if (!aType || aType->getString() != "app") state->error("not an app definition").debugThrow(); - if (json) { - j.emplace("type", "app"); - if (description) - j.emplace("description", *description); - } else { - logger->cout( - "%s: app: " ANSI_BOLD "%s" ANSI_NORMAL, - headerPrefix, - description ? *description : "no description"); - } + j.emplace("type", "app"); + if (description) + j.emplace("description", *description); } else if ( (attrPath.size() == 1 && attrPathS[0] == "defaultTemplate") || (attrPath.size() == 2 && attrPathS[0] == "templates")) { auto description = visitor.getAttr("description")->getString(); - if (json) { - j.emplace("type", "template"); - j.emplace("description", description); - } else { - logger->cout("%s: template: " ANSI_BOLD "%s" ANSI_NORMAL, headerPrefix, description); - } + j.emplace("type", "template"); + j.emplace("description", description); } else { @@ -1438,25 +1332,85 @@ struct CmdFlakeShow : FlakeCommand, MixJSON || (attrPath.size() == 2 && attrPathS[0] == "nixosModules") ? std::make_pair("nixos-module", "NixOS module") : std::make_pair("unknown", "unknown"); - if (json) { - j.emplace("type", type); - } else { - logger->cout("%s: " ANSI_WARNING "%s" ANSI_NORMAL, headerPrefix, description); - } + j.emplace("type", type); + j.emplace("description", description); } } catch (EvalError & e) { if (!(attrPath.size() > 0 && attrPathS[0] == "legacyPackages")) throw; } - - return j; }; - auto cache = openEvalCache(*state, flake); + futures.spawn(1, [&]() { visit(*cache->getRoot(), j); }); + futures.finishAll(); - auto j = visit(*cache->getRoot(), {}, fmt(ANSI_BOLD "%s" ANSI_NORMAL, flake->flake.lockedRef), ""); if (json) printJSON(j); + else { + + // For frameworks it's important that structures are as + // lazy as possible to prevent infinite recursions, + // performance issues and errors that aren't related to + // the thing to evaluate. As a consequence, they have to + // emit more attributes than strictly (sic) necessary. + // However, these attributes with empty values are not + // useful to the user so we omit them. + std::function hasContent; + + hasContent = [&](const nlohmann::json & j) -> bool { + if (j.find("type") != j.end()) + return true; + else { + for (auto & j2 : j) + if (hasContent(j2)) + return true; + return false; + } + }; + + // Render the JSON into a tree representation. + std::function + render; + + render = [&](nlohmann::json j, const std::string & headerPrefix, const std::string & nextPrefix) { + if (j.find("type") != j.end()) { + std::string s; + + std::string type = j["type"]; + if (type == "omitted") { + s = j["message"]; + } else if (type == "derivation") { + s = (std::string) j["subtype"] + " '" + (std::string) j["name"] + "'"; + } else { + s = type; + } + + logger->cout("%s: %s '%s'", headerPrefix, type, s); + return; + } + + logger->cout("%s", headerPrefix); + + auto nonEmpty = nlohmann::json::object(); + for (const auto & j2 : j.items()) { + if (hasContent(j2.value())) + nonEmpty[j2.key()] = j2.value(); + } + + for (const auto & [i, j2] : enumerate(nonEmpty.items())) { + bool last = i + 1 == nonEmpty.size(); + render( + j2.value(), + fmt(ANSI_GREEN "%s%s" ANSI_NORMAL ANSI_BOLD "%s" ANSI_NORMAL, + nextPrefix, + last ? treeLast : treeConn, + j2.key()), + nextPrefix + (last ? treeNull : treeLine)); + } + }; + + render(j, fmt(ANSI_BOLD "%s" ANSI_NORMAL, flake->flake.lockedRef), ""); + } } }; diff --git a/src/nix/main.cc b/src/nix/main.cc index 256263ad65f..fff44ed1c09 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -245,8 +245,8 @@ static void showHelp(std::vector subcommand, NixArgs & toplevel) vDump->mkString(toplevel.dumpCli()); auto vRes = state.allocValue(); - state.callFunction(*vGenerateManpage, state.getBuiltin("false"), *vRes, noPos); - state.callFunction(*vRes, *vDump, *vRes, noPos); + Value * args[]{&state.getBuiltin("false"), vDump}; + state.callFunction(*vGenerateManpage, args, *vRes, noPos); auto attr = vRes->attrs()->get(state.symbols.create(mdName + ".md")); if (!attr) diff --git a/src/nix/search.cc b/src/nix/search.cc index 562af31518e..692d2052d59 100644 --- a/src/nix/search.cc +++ b/src/nix/search.cc @@ -11,6 +11,7 @@ #include "nix/expr/attr-path.hh" #include "nix/util/hilite.hh" #include "nix/util/strings-inline.hh" +#include "nix/expr/parallel-eval.hh" #include #include @@ -84,11 +85,13 @@ struct CmdSearch : InstallableValueCommand, MixJSON auto state = getEvalState(); - std::optional jsonOut; + std::optional> jsonOut; if (json) - jsonOut = json::object(); + jsonOut.emplace(json::object()); - uint64_t results = 0; + std::atomic results = 0; + + FutureVector futures(*state->executor); std::function & attrPath, bool initialRecurse)> visit; @@ -96,15 +99,22 @@ struct CmdSearch : InstallableValueCommand, MixJSON visit = [&](eval_cache::AttrCursor & cursor, const std::vector & attrPath, bool initialRecurse) { auto attrPathS = state->symbols.resolve(attrPath); - Activity act(*logger, lvlInfo, actUnknown, fmt("evaluating '%s'", concatStringsSep(".", attrPathS))); + /* + Activity act(*logger, lvlInfo, actUnknown, + fmt("evaluating '%s'", concatStringsSep(".", attrPathS))); + */ try { auto recurse = [&]() { + std::vector> work; for (const auto & attr : cursor.getAttrs()) { auto cursor2 = cursor.getAttr(state->symbols[attr]); auto attrPath2(attrPath); attrPath2.push_back(attr); - visit(*cursor2, attrPath2, false); + work.emplace_back( + [cursor2, attrPath2, visit]() { visit(*cursor2, attrPath2, false); }, + std::string_view(state->symbols[attr]).find("Packages") != std::string_view::npos ? 0 : 2); } + futures.spawn(std::move(work)); }; if (cursor.isDerivation()) { @@ -148,21 +158,21 @@ struct CmdSearch : InstallableValueCommand, MixJSON if (found) { results++; if (json) { - (*jsonOut)[attrPath2] = { + (*jsonOut->lock())[attrPath2] = { {"pname", name.name}, {"version", name.version}, {"description", description}, }; } else { - if (results > 1) - logger->cout(""); - logger->cout( - "* %s%s", - wrap("\e[0;1m", hiliteMatches(attrPath2, attrPathMatches, ANSI_GREEN, "\e[0;1m")), - name.version != "" ? " (" + name.version + ")" : ""); + auto out = + fmt("%s* %s%s", + results > 1 ? "\n" : "", + wrap("\e[0;1m", hiliteMatches(attrPath2, attrPathMatches, ANSI_GREEN, "\e[0;1m")), + name.version != "" ? " (" + name.version + ")" : ""); if (description != "") - logger->cout( - " %s", hiliteMatches(description, descriptionMatches, ANSI_GREEN, ANSI_NORMAL)); + out += fmt( + "\n %s", hiliteMatches(description, descriptionMatches, ANSI_GREEN, ANSI_NORMAL)); + logger->cout(out); } } } @@ -187,14 +197,21 @@ struct CmdSearch : InstallableValueCommand, MixJSON } }; - for (auto & cursor : installable->getCursors(*state)) - visit(*cursor, cursor->getAttrPath(), true); + std::vector> work; + for (auto & cursor : installable->getCursors(*state)) { + work.emplace_back([cursor, visit]() { visit(*cursor, cursor->getAttrPath(), true); }, 1); + } + + futures.spawn(std::move(work)); + futures.finishAll(); if (json) - printJSON(*jsonOut); + printJSON(*(jsonOut->lock())); if (!json && !results) throw Error("no results for the given search term(s)!"); + + printError("Found %d matching packages.", results); } }; diff --git a/tests/functional/flakes/show.sh b/tests/functional/flakes/show.sh index 7fcc6aca9b4..98fdbf78861 100755 --- a/tests/functional/flakes/show.sh +++ b/tests/functional/flakes/show.sh @@ -59,13 +59,7 @@ cat >flake.nix < show-output.json -nix eval --impure --expr ' -let show_output = builtins.fromJSON (builtins.readFile ./show-output.json); -in -assert show_output == { }; -true -' +[[ $(nix flake show --all-systems --legacy | wc -l) = 1 ]] # Test that attributes with errors are handled correctly. # nixpkgs.legacyPackages is a particularly prominent instance of this. diff --git a/tests/functional/lang/eval-fail-blackhole.err.exp b/tests/functional/lang/eval-fail-blackhole.err.exp index 95e33a5fe45..d11eb338f9a 100644 --- a/tests/functional/lang/eval-fail-blackhole.err.exp +++ b/tests/functional/lang/eval-fail-blackhole.err.exp @@ -7,8 +7,8 @@ error: 3| x = y; error: infinite recursion encountered - at /pwd/lang/eval-fail-blackhole.nix:3:7: + at /pwd/lang/eval-fail-blackhole.nix:2:3: + 1| let { 2| body = x; + | ^ 3| x = y; - | ^ - 4| y = x; diff --git a/tests/functional/lang/eval-fail-recursion.err.exp b/tests/functional/lang/eval-fail-recursion.err.exp index 8bfb4e12e47..21bf7a695bd 100644 --- a/tests/functional/lang/eval-fail-recursion.err.exp +++ b/tests/functional/lang/eval-fail-recursion.err.exp @@ -7,8 +7,8 @@ error: 3| in error: infinite recursion encountered - at /pwd/lang/eval-fail-recursion.nix:2:14: - 1| let - 2| a = { } // a; - | ^ + at /pwd/lang/eval-fail-recursion.nix:4:1: 3| in + 4| a.foo + | ^ + 5| diff --git a/tests/functional/lang/eval-fail-scope-5.err.exp b/tests/functional/lang/eval-fail-scope-5.err.exp index 6edc85f4f16..557054b5354 100644 --- a/tests/functional/lang/eval-fail-scope-5.err.exp +++ b/tests/functional/lang/eval-fail-scope-5.err.exp @@ -21,8 +21,8 @@ error: 8| x ? y, error: infinite recursion encountered - at /pwd/lang/eval-fail-scope-5.nix:8:11: - 7| { - 8| x ? y, - | ^ - 9| y ? x, + at /pwd/lang/eval-fail-scope-5.nix:13:3: + 12| + 13| body = f { }; + | ^ + 14| diff --git a/tests/functional/misc.sh b/tests/functional/misc.sh index cb4d4139f4c..55d30fb8e3a 100755 --- a/tests/functional/misc.sh +++ b/tests/functional/misc.sh @@ -22,11 +22,11 @@ expect 1 nix-env -q --foo 2>&1 | grep "unknown flag" # Eval Errors. eval_arg_res=$(nix-instantiate --eval -E 'let a = {} // a; in a.foo' 2>&1 || true) -echo $eval_arg_res | grep "at «string»:1:15:" +echo $eval_arg_res | grep "at «string»:1:12:" echo $eval_arg_res | grep "infinite recursion encountered" eval_stdin_res=$(echo 'let a = {} // a; in a.foo' | nix-instantiate --eval -E - 2>&1 || true) -echo $eval_stdin_res | grep "at «stdin»:1:15:" +echo $eval_stdin_res | grep "at «stdin»:1:12:" echo $eval_stdin_res | grep "infinite recursion encountered" # Attribute path errors