diff --git a/src/libexpr-tests/tecnix-dependency-tracking.cc b/src/libexpr-tests/tecnix-dependency-tracking.cc index 44f89c569..a9a77b77d 100644 --- a/src/libexpr-tests/tecnix-dependency-tracking.cc +++ b/src/libexpr-tests/tecnix-dependency-tracking.cc @@ -46,9 +46,10 @@ static bool containsPath(const std::vector & paths, std::string_vie static std::vector flattenFrame(const ref & graph, const TrackedSourceDepsFrame & frame) { - std::vector direct( - frame.directSourceAccessSetAccesses.begin(), frame.directSourceAccessSetAccesses.end()); - std::vector children(frame.childSourceAccessSets.begin(), frame.childSourceAccessSets.end()); + auto frameAccesses = frame.directSourceAccessSetAccesses(); + auto frameChildren = frame.childSourceAccessSets(); + std::vector direct(frameAccesses.begin(), frameAccesses.end()); + std::vector children(frameChildren.begin(), frameChildren.end()); return graph->flatten(direct, children); } diff --git a/src/libexpr/include/nix/expr/tecnix/access-set-graph.hh b/src/libexpr/include/nix/expr/tecnix/access-set-graph.hh index 54eef0115..140b09013 100644 --- a/src/libexpr/include/nix/expr/tecnix/access-set-graph.hh +++ b/src/libexpr/include/nix/expr/tecnix/access-set-graph.hh @@ -33,6 +33,22 @@ struct EvalSourceAccessSetNode uint64_t hash = 0; }; +/** + * One memoised `internAccessSet` *input*: the canonical (sorted, deduplicated) + * direct access ids and child set ids a frame published with, and the set id + * that input resolved to. Keyed on the input rather than on the flattened + * union, so a repeat publish never has to materialise the union at all. + */ +struct EvalSourceAccessSetInputNode +{ + uint32_t first = 0; + uint32_t directCount = 0; + uint32_t childCount = 0; + uint32_t nextWithSameHash = 0; + uint64_t hash = 0; + EvalSourceAccessSetId accessSet = emptyEvalSourceAccessSetId; +}; + class EvalSourceAccessSetGraph { std::atomic enabled{false}; @@ -43,11 +59,27 @@ class EvalSourceAccessSetGraph std::vector accessSetItems; std::vector singletonAccessSets; boost::unordered_flat_map accessSetIdsByHash; - boost::unordered_flat_map pairUnionAccessSets; + std::vector inputKeys; + std::vector inputKeyItems; + boost::unordered_flat_map inputKeyIdsByHash; mutable std::vector seenAccessGenerations; mutable uint32_t nextFlattenGeneration = 1; bool accessSetEquals(EvalSourceAccessSetId id, const std::vector & items) const; + bool inputKeyEquals( + uint32_t id, + std::span directAccesses, + std::span children) const; + EvalSourceAccessSetId lookupInputKey( + uint64_t hash, + std::span directAccesses, + std::span children, + bool & found) const; + void rememberInputKey( + uint64_t hash, + std::span directAccesses, + std::span children, + EvalSourceAccessSetId accessSet); public: EvalSourceAccessSetGraph(); diff --git a/src/libexpr/include/nix/expr/tecnix/source-deps.hh b/src/libexpr/include/nix/expr/tecnix/source-deps.hh index 16de66962..4d9da7c3c 100644 --- a/src/libexpr/include/nix/expr/tecnix/source-deps.hh +++ b/src/libexpr/include/nix/expr/tecnix/source-deps.hh @@ -5,8 +5,6 @@ #include "nix/util/pos-idx.hh" #include "nix/util/ref.hh" -#include - #include #include #include @@ -50,21 +48,35 @@ forceValueTracked(EvalState & state, Value & v, PosIdx pos, TrackingContext & tr std::vector parseGitPorcelainZDirtyPaths(std::string_view output); -using EvalSourceAccessIdFrameVector = boost::container::small_vector; -using EvalSourceAccessSetIdFrameVector = boost::container::small_vector; - /** * A stack-resident accumulator for one bracketed region of evaluation: the * force of one value (`value` set) or a source-deps scope / target root * (`value` null). Collects direct path accesses and inherited child labels; * interned into one set id when the region publishes. + * + * Frames are strictly LIFO within a tracking context, so their entries do not + * need per-frame containers: they live in two stacks owned by the context and + * a frame stores only the stack sizes at entry (its "watermarks"). A frame's + * entries are whatever sits above its marks. + * + * This makes the two operations that dominate tracked evaluation nearly free. + * Entering and leaving a region that records nothing -- the majority of forces + * -- costs two loads and two compares rather than constructing and destroying + * two containers. Merging an unpublished frame into its parent costs nothing + * at all, because the entries are already contiguous inside the parent's own + * region; leaving them in place *is* the merge. Publishing truncates back to + * the marks and pushes the single interned id in their place. + * + * The frame is trivially destructible, so it emits no destructor. */ struct TrackedSourceDepsFrame { TrackingContext & trackingCtx; Value * value = nullptr; - EvalSourceAccessIdFrameVector directSourceAccessSetAccesses; - EvalSourceAccessSetIdFrameVector childSourceAccessSets; + /** Size of the context's access stack when this frame was entered. */ + uint32_t accessBase = 0; + /** Size of the context's child stack when this frame was entered. */ + uint32_t childBase = 0; EvalSourceAccessSetId accessSet = emptyEvalSourceAccessSetId; TrackedSourceDepsFrame * previous = nullptr; TrackedSourceDepsFrame * nearestValueForceFrame = nullptr; @@ -72,6 +84,11 @@ struct TrackedSourceDepsFrame TrackedSourceDepsFrame( TrackingContext & trackingCtx, Value * value = nullptr, TrackedSourceDepsFrame * previous = nullptr); + + /** The direct accesses recorded into this frame, as a view into the context stack. */ + std::span directSourceAccessSetAccesses() const; + /** The child labels recorded into this frame, as a view into the context stack. */ + std::span childSourceAccessSets() const; }; /** @@ -95,6 +112,12 @@ struct TrackedSourceDepsFrame struct TrackingContext { ref sourceAccessSetGraph; + /** + * Backing storage for every frame in this context. Declared before + * `rootFrame` so they are constructed before it reads their sizes. + */ + std::vector frameAccessStack; + std::vector frameChildStack; TrackedSourceDepsFrame rootFrame; // Always captures the EvalState-owned source-access graph; no foreign graph constructor exists. diff --git a/src/libexpr/primops/tecnix.cc b/src/libexpr/primops/tecnix.cc index 40e8f866b..b42085977 100644 --- a/src/libexpr/primops/tecnix.cc +++ b/src/libexpr/primops/tecnix.cc @@ -166,6 +166,9 @@ struct TecnixArgs Value * resolverArgs = nullptr; std::string argsKey; std::vector targets; + /** The caller asked for the tracked source closure (`includeDependencies`), + so it must be computed even when the eval cache would not need it. */ + bool requireDependencies = false; }; /** The persistent-cache row family these arguments address. */ @@ -342,6 +345,26 @@ static void configureTecnixRepoContext(EvalState & state, const TecnixArgs & arg configureTectonixContext(state, args.gitDir, args.rev, args.checkoutPath); } +/** + * Whether source-access tracking should run at all. + * + * Tracking exists to compute the dependency closure. That closure has exactly + * two consumers: the eval cache, which keys rows on it, and an explicit + * `includeDependencies` request, which returns it. With neither, tracking + * every force, interning the sets and fingerprinting the paths is pure + * overhead. Gating here also makes "not tracking" a whole-evaluation property, + * which is what lets the resolver module be shared in that mode. + */ +static bool tecnixSourceTrackingEnabled(const EvalState & state, const TecnixArgs & tArgs) +{ + return tArgs.requireDependencies || (state.settings.pureEval && state.settings.tecnixEvalCache); +} + +/** Keyspace separator for modules built without tracking; see + getTecnixModuleValue. The embedded NUL keeps it disjoint from any resolver + path, which cannot contain one. */ +static constexpr std::string_view untrackedTecnixModuleKeyPrefix{"untracked\0", 10}; + /** * Import the explicit resolver file from the git repo and return a value from * the attrset produced by calling it with `args` (e.g. `resolve` or @@ -352,20 +375,92 @@ static void configureTecnixRepoContext(EvalState & state, const TecnixArgs & arg static Value & getTecnixModuleValue(EvalState & state, const PosIdx pos, const TecnixArgs & tArgs, std::string_view attrName) { - // Get resolver file path from the lazily-mounted Tecnix repo accessor. - auto resolverPath = getTecnixRepoPath(state, tArgs.resolver); - auto modulePath = SourcePath(state.rootFS, CanonPath(resolverPath)); - - // Import the resolver file (a function taking the opaque `args` value) and call it. - auto * moduleFn = state.allocValue(); - state.evalFile(modulePath, *moduleFn); - if (!tArgs.resolverArgs) state.error("missing Tecnix resolver args").atPos(pos).debugThrow(); - auto * moduleVal = state.allocValue(); - state.callFunction(*moduleFn, *tArgs.resolverArgs, *moduleVal, pos); - state.forceAttrs(*moduleVal, pos, "while evaluating tecnix module"); + auto buildModule = [&]() -> Value * { + // Get resolver file path from the lazily-mounted Tecnix repo accessor. + auto resolverPath = getTecnixRepoPath(state, tArgs.resolver); + auto modulePath = SourcePath(state.rootFS, CanonPath(resolverPath)); + + // Import the resolver file (a function taking the opaque `args` value) and call it. + auto * moduleFn = state.allocValue(); + state.evalFile(modulePath, *moduleFn); + + auto * moduleVal = state.allocValue(); + state.callFunction(*moduleFn, *tArgs.resolverArgs, *moduleVal, pos); + state.forceAttrs(*moduleVal, pos, "while evaluating tecnix module"); + return moduleVal; + }; + + auto * trackingCtx = currentTecnixThreadState.trackingContext; + + Value * moduleVal = nullptr; + + if (!trackingCtx && tecnixSourceTrackingEnabled(state, tArgs)) { + /* Tracking is enabled for this evaluation but this particular build is + untracked, so it has no label to replay. Caching it would let a later + tracked caller reuse a module whose accesses were never recorded. */ + moduleVal = buildModule(); + } else if (!trackingCtx) { + /* This call does not track, so the module it builds carries no label. + Share it only with other untracked calls: tracking is decided per + call (`includeDependencies` turns it on with the cache off), so a + tracked call later in the same evaluation must not inherit a module + whose accesses were never recorded -- it would silently drop the + resolver's own files from every target's closure. Hence the separate + keyspace rather than a shared entry. */ + auto & moduleCache = *state.tecnixEvalData().tecnixModuleCache; + auto cacheKey = std::string(untrackedTecnixModuleKeyPrefix) + tArgs.resolver + '\0' + tArgs.argsKey; + moduleCache.try_emplace_and_cvisit( + cacheKey, + EvalTecnixModuleCacheEntry{}, + [&](auto & i) { + moduleVal = buildModule(); + i.second.value = RootValue(moduleVal); + i.second.sourceDeps = emptyEvalSourceAccessSetId; + }, + [&](auto & i) { moduleVal = *i.second.value; }); + } else { + /* Reuse the applied module for this (resolver, args) pair. Without this + every builtin re-applies the resolver and gets a fresh, wholly + unevaluated attrset, so discovering target names and then resolving + them walks the zone graph twice over. */ + auto & moduleCache = *state.tecnixEvalData().tecnixModuleCache; + auto cacheKey = tArgs.resolver + '\0' + tArgs.argsKey; + + auto sourceDeps = emptyEvalSourceAccessSetId; + bool hit = false; + moduleCache.cvisit(cacheKey, [&](auto & i) { + moduleVal = *i.second.value; + sourceDeps = i.second.sourceDeps; + hit = true; + }); + + if (!hit) { + /* Scope the build so the accesses it makes — importing the resolver + above all — are interned into one set that later contexts can + replay, instead of only landing in whichever frame happened to be + current the first time. */ + TrackedSourceDepsScope moduleScope(*trackingCtx); + moduleVal = buildModule(); + sourceDeps = moduleScope.finish(moduleVal); + + moduleCache.try_emplace_and_cvisit( + cacheKey, + EvalTecnixModuleCacheEntry{}, + [&](auto & i) { + i.second.value = RootValue(moduleVal); + i.second.sourceDeps = sourceDeps; + }, + [&](auto & i) { + moduleVal = *i.second.value; + sourceDeps = i.second.sourceDeps; + }); + } else { + recordTrackedSourceAccessSetDependency(*trackingCtx, sourceDeps); + } + } auto attr = moduleVal->attrs()->get(state.symbols.create(attrName)); if (!attr) @@ -392,10 +487,10 @@ static SourceAccessSetSnapshot snapshotSourceAccessSetTracking(const TrackingCon // Tracking contexts are thread-confined: the snapshot runs on the thread // that owns the context, after its evaluation has completed. SourceAccessSetSnapshot snapshot; - snapshot.directAccesses.assign( - ctx.rootFrame.directSourceAccessSetAccesses.begin(), ctx.rootFrame.directSourceAccessSetAccesses.end()); - snapshot.accessSetEdges.assign( - ctx.rootFrame.childSourceAccessSets.begin(), ctx.rootFrame.childSourceAccessSets.end()); + auto rootAccesses = ctx.rootFrame.directSourceAccessSetAccesses(); + auto rootChildren = ctx.rootFrame.childSourceAccessSets(); + snapshot.directAccesses.assign(rootAccesses.begin(), rootAccesses.end()); + snapshot.accessSetEdges.assign(rootChildren.begin(), rootChildren.end()); return snapshot; } @@ -461,6 +556,7 @@ static TecnixDiscoveryResult discoverTecnixTargetNames( EvalState & state, const PosIdx pos, const TecnixArgs & tArgs, DependencyFingerprintCache & fingerprintCache) { bool useCache = state.settings.pureEval && state.settings.tecnixEvalCache; + bool track = tecnixSourceTrackingEnabled(state, tArgs); std::string cacheKey{tecnixTargetNamesCacheKey}; if (useCache) { @@ -480,14 +576,20 @@ static TecnixDiscoveryResult discoverTecnixTargetNames( } printTalkative("tecnixTargetNames: discovery cache miss, evaluating"); - TrackingContext trackingCtx(state); + std::optional trackingCtx; std::vector targetNames; - { - ActiveTrackingContext activeTrackingCtx(trackingCtx); + if (track) { + trackingCtx.emplace(state); + ActiveTrackingContext activeTrackingCtx(*trackingCtx); + targetNames = evalTargetNamesOnly(state, pos, tArgs); + } else { targetNames = evalTargetNamesOnly(state, pos, tArgs); } - auto trackedPaths = collectSourceAccessSetTrackedPaths(trackingCtx); - auto dependencies = dependencyFingerprints(getTecnixRepoAccessor(state), trackedPaths, fingerprintCache); + DependencyClosure dependencies; + if (trackingCtx) { + auto trackedPaths = collectSourceAccessSetTrackedPaths(*trackingCtx); + dependencies = dependencyFingerprints(getTecnixRepoAccessor(state), trackedPaths, fingerprintCache); + } if (useCache && !dependencies.empty()) { std::vector upserts; @@ -601,6 +703,7 @@ static void prim_tecnixTargets(EvalState & state, const PosIdx pos, Value ** arg args, state.symbols.create("includeDependencies"), "while evaluating the 'includeDependencies' argument to builtins.tecnixTargets"); + tArgs.requireDependencies = includeDependencies; if (includeDependencies) { auto includeTargets = getTecnixBoolAttr( @@ -722,6 +825,12 @@ struct PreparedTrackedResolveFunction static PreparedTrackedResolveFunction prepareTrackedResolveFunction(EvalState & state, const PosIdx pos, const TecnixArgs & tArgs) { + if (!tecnixSourceTrackingEnabled(state, tArgs)) + return { + .resolveFn = &getResolveFunction(state, pos, tArgs), + .sourceDeps = emptyEvalSourceAccessSetId, + }; + TrackingContext trackingCtx(state); ActiveTrackingContext activeTrackingCtx(trackingCtx); @@ -741,7 +850,8 @@ static TargetDependencyResult evalTargetDependencies( Value & resolveFn, EvalSourceAccessSetId resolveSourceDeps, const std::string & target, - bool keepTargetValue) + bool keepTargetValue, + bool track) { auto started = std::chrono::steady_clock::now(); printTalkative( @@ -749,13 +859,18 @@ static TargetDependencyResult evalTargetDependencies( target, Executor::amWorkerThread ? "worker" : "main"); - TrackingContext trackingCtx(state); - if (resolveSourceDeps != emptyEvalSourceAccessSetId) - recordTrackedSourceAccessSetDependency(trackingCtx, resolveSourceDeps); + std::optional trackingCtx; + if (track) { + trackingCtx.emplace(state); + if (resolveSourceDeps != emptyEvalSourceAccessSetId) + recordTrackedSourceAccessSetDependency(*trackingCtx, resolveSourceDeps); + } Value * targetValue = nullptr; std::string drvPath; { - ActiveTrackingContext activeTrackingCtx(trackingCtx); + std::optional activeTrackingCtx; + if (trackingCtx) + activeTrackingCtx.emplace(*trackingCtx); auto * targetArg = state.allocValue(); targetArg->mkString(target, state.mem); @@ -766,7 +881,9 @@ static TargetDependencyResult evalTargetDependencies( targetValue = resolveResult; } - auto snapshot = snapshotSourceAccessSetTracking(trackingCtx); + std::optional snapshot; + if (trackingCtx) + snapshot = snapshotSourceAccessSetTracking(*trackingCtx); auto elapsedMs = std::chrono::duration_cast(std::chrono::steady_clock::now() - started).count(); printTalkative( @@ -910,7 +1027,13 @@ static TargetDependencyResults evaluateTecnixTargetDependencies( auto evalMiss = [&](size_t i) { auto & target = args.targets[i]; results[i] = evalTargetDependencies( - state, pos, *preparedResolve.resolveFn, preparedResolve.sourceDeps, target, keepTargetValues); + state, + pos, + *preparedResolve.resolveFn, + preparedResolve.sourceDeps, + target, + keepTargetValues, + tecnixSourceTrackingEnabled(state, args)); if (results[i]) results[i]->cacheNeedsUpsert = true; }; @@ -1035,6 +1158,7 @@ static void prim_tecnixTargetNames(EvalState & state, const PosIdx pos, Value ** args, state.symbols.create("includeDependencies"), "while evaluating the 'includeDependencies' argument to builtins.tecnixTargetNames"); + dArgs.requireDependencies = includeDependencies; DependencyFingerprintCache fingerprintCache; auto result = discoverTecnixTargetNames(state, pos, dArgs, fingerprintCache); diff --git a/src/libexpr/tecnix/eval-data.hh b/src/libexpr/tecnix/eval-data.hh index aff4df849..d4140875f 100644 --- a/src/libexpr/tecnix/eval-data.hh +++ b/src/libexpr/tecnix/eval-data.hh @@ -31,6 +31,22 @@ using EvalImportResolutionCache = boost::concurrent_flat_map; using EvalWorldTreeShaCache = boost::concurrent_flat_map; +/** + * A resolver module applied to `args`, together with the source-access set + * recorded while applying it. The label has to be kept alongside the value: + * the accesses happen once, when the module is first built, but every tracking + * context that reuses the module must still record them, or the resolver file + * itself silently drops out of the dependency closure it is supposed to + * invalidate. + */ +struct EvalTecnixModuleCacheEntry +{ + RootValue value; + EvalSourceAccessSetId sourceDeps = emptyEvalSourceAccessSetId; +}; + +using EvalTecnixModuleCache = boost::concurrent_flat_map; + struct EvalState::TecnixEvalData { /** @@ -100,6 +116,28 @@ struct EvalState::TecnixEvalData /** Cache: world path → tree SHA (lazy computed, cached at each path level) */ const ref worldTreeShaCache = make_ref(); + /** + * The resolver module applied to `args`, keyed by resolver path and the + * canonical `args` encoding. + * + * Every Tecnix builtin needs the same thing: the resolver file imported and + * called with `args`. Importing is already cached, but the *application* + * was not, so each builtin got its own copy of the returned attrset and + * therefore its own unevaluated copy of everything hanging off it. A run + * that discovers target names and then resolves them consequently walked + * the zone graph twice, which measured as ~24% more thunks and function + * calls than resolving alone. + * + * `argsKey` is a canonical, injective encoding of `args` (see + * `canonicalJsonFromValue`), so an equal key means the resolver would be + * applied to an equal value and the result is interchangeable. Sharing the + * applied module across tracking contexts is sound for the same reason + * sharing `trackedFileEvalCache` is: whichever context first forces a thunk + * publishes its source-access label onto the finished value, and a later + * force in another context picks that label up via `forceValueTracked`. + */ + const ref tecnixModuleCache = make_ref(); + /** Lazy-initialized set of zone IDs in sparse checkout (thread-safe via once_flag) */ mutable std::once_flag tectonixSparseCheckoutRootsFlag; mutable std::set tectonixSparseCheckoutRoots; diff --git a/src/libexpr/tecnix/source-deps.cc b/src/libexpr/tecnix/source-deps.cc index 278b20f11..3ba129d52 100644 --- a/src/libexpr/tecnix/source-deps.cc +++ b/src/libexpr/tecnix/source-deps.cc @@ -30,6 +30,10 @@ uint32_t * tecnixInstallValueLabelChunk(size_t dirIndex) { static_assert(sizeof(void *) == 8, "the Tecnix value-label table requires a 64-bit address space"); + /* Published before the chunk pointer, and therefore before the label store + that this chunk is being installed for: a reader that can observe any + label can also observe the flag. */ + size_t chunkBytes = (size_t{1} << 32) / 16 * sizeof(uint32_t); int flags = MAP_PRIVATE | MAP_ANONYMOUS; #ifdef MAP_NORESERVE @@ -101,6 +105,43 @@ static uint64_t hashSourceAccessIds(std::span items) return hash; } +/* Hashes a canonical `internAccessSet` input. The direct ids and the child set + ids live in different id spaces, so they are separated by a marker that + cannot occur in either: without it `({a}, {})` and `({}, {a})` would collide + into the same bucket (harmless, but it would cost a chain walk on every + lookup). */ +static uint64_t hashAccessSetInputKey( + std::span directAccesses, std::span children) +{ + uint64_t hash = 1469598103934665603ULL; + auto mix = [&](uint64_t word) { + hash ^= word; + hash *= 1099511628211ULL; + }; + + for (auto access : directAccesses) + mix(access); + mix(~uint64_t{0}); + for (auto child : children) + mix(child); + return hash; +} + +/* Copies the non-empty ids of `in` into `out`, sorted and deduplicated, so that + frames which recorded the same ids in a different order (or more than once, + which the frame appenders only partially suppress) produce the same memo + key. */ +template +static void canonicaliseAccessSetInput(std::vector & out, std::span in) +{ + out.clear(); + for (auto id : in) + if (id != 0) + out.push_back(id); + std::sort(out.begin(), out.end()); + out.erase(std::unique(out.begin(), out.end()), out.end()); +} + EvalSourceAccessSetGraph::EvalSourceAccessSetGraph() = default; void EvalSourceAccessSetGraph::enable() @@ -114,6 +155,7 @@ void EvalSourceAccessSetGraph::enable() accesses.emplace_back(); accessSets.push_back(EvalSourceAccessSetNode{}); + inputKeys.push_back(EvalSourceAccessSetInputNode{}); // index 0 is the "no entry" sentinel enabled.store(true, std::memory_order_release); } @@ -153,40 +195,96 @@ bool EvalSourceAccessSetGraph::accessSetEquals( accessSetItems.begin() + node.first + node.count); } -EvalSourceAccessSetId EvalSourceAccessSetGraph::internAccessSet( - std::span directAccesses, std::span children) +bool EvalSourceAccessSetGraph::inputKeyEquals( + uint32_t id, + std::span directAccesses, + std::span children) const { - if (!enabled.load(std::memory_order_acquire)) + auto node = inputKeys[id]; + if (node.directCount != directAccesses.size() || node.childCount != children.size()) + return false; + + auto * items = inputKeyItems.data() + node.first; + return std::equal(directAccesses.begin(), directAccesses.end(), items) + && std::equal(children.begin(), children.end(), items + node.directCount); +} + +EvalSourceAccessSetId EvalSourceAccessSetGraph::lookupInputKey( + uint64_t hash, + std::span directAccesses, + std::span children, + bool & found) const +{ + found = false; + auto head = inputKeyIdsByHash.find(hash); + if (head == inputKeyIdsByHash.end()) return emptyEvalSourceAccessSetId; - size_t directCount = 0; - EvalSourceAccessId singleDirect = emptyEvalSourceAccessId; - for (auto access : directAccesses) { - if (access == emptyEvalSourceAccessId) + for (auto id = head->second; id != 0; id = inputKeys[id].nextWithSameHash) { + if (!inputKeyEquals(id, directAccesses, children)) continue; - directCount++; - singleDirect = access; + found = true; + return inputKeys[id].accessSet; } + return emptyEvalSourceAccessSetId; +} - size_t childCount = 0; - EvalSourceAccessSetId singleChild = emptyEvalSourceAccessSetId; - EvalSourceAccessSetId pairChildA = emptyEvalSourceAccessSetId; - EvalSourceAccessSetId pairChildB = emptyEvalSourceAccessSetId; - for (auto child : children) { - if (child == emptyEvalSourceAccessSetId) - continue; - childCount++; - singleChild = child; - if (childCount == 1) - pairChildA = child; - else if (childCount == 2) - pairChildB = child; +void EvalSourceAccessSetGraph::rememberInputKey( + uint64_t hash, + std::span directAccesses, + std::span children, + EvalSourceAccessSetId accessSet) +{ + /* The memo is a pure accelerator: past the cap we simply stop adding + entries and fall back to flattening, rather than let a pathological + evaluation grow it without bound. */ + constexpr size_t maxInputKeys = size_t{1} << 20; + if (inputKeys.size() >= maxInputKeys) + return; + + auto first = static_cast(inputKeyItems.size()); + inputKeyItems.insert(inputKeyItems.end(), directAccesses.begin(), directAccesses.end()); + inputKeyItems.insert(inputKeyItems.end(), children.begin(), children.end()); + + auto id = static_cast(inputKeys.size()); + uint32_t next = 0; + if (auto head = inputKeyIdsByHash.find(hash); head != inputKeyIdsByHash.end()) { + next = head->second; + head->second = id; + } else { + inputKeyIdsByHash.emplace(hash, id); } + inputKeys.push_back( + EvalSourceAccessSetInputNode{ + .first = first, + .directCount = static_cast(directAccesses.size()), + .childCount = static_cast(children.size()), + .nextWithSameHash = next, + .hash = hash, + .accessSet = accessSet, + }); +} - if (directCount == 0 && childCount == 0) +EvalSourceAccessSetId EvalSourceAccessSetGraph::internAccessSet( + std::span directAccesses, std::span children) +{ + if (!enabled.load(std::memory_order_acquire)) return emptyEvalSourceAccessSetId; - if (directCount == 0 && childCount == 1) - return singleChild; + + /* Canonicalise the input before anything else: it is what the memo below is + keyed on, and it is tiny (one frame's direct accesses and child edges) + compared to the transitive union those children stand for. */ + static thread_local std::vector keyDirect; + static thread_local std::vector keyChildren; + canonicaliseAccessSetInput(keyDirect, directAccesses); + canonicaliseAccessSetInput(keyChildren, children); + + if (keyDirect.empty()) { + if (keyChildren.empty()) + return emptyEvalSourceAccessSetId; + if (keyChildren.size() == 1) + return keyChildren.front(); + } auto internSingleton = [&](EvalSourceAccessId access) { if (access == emptyEvalSourceAccessId) @@ -206,38 +304,34 @@ EvalSourceAccessSetId EvalSourceAccessSetGraph::internAccessSet( }; std::lock_guard lock(mutex); - if (childCount == 0 && directCount == 1) - return internSingleton(singleDirect); - - bool hasPairUnionKey = false; - uint64_t pairUnionKey = 0; - if (directCount == 0 && childCount == 2) { - auto a = std::min(pairChildA, pairChildB); - auto b = std::max(pairChildA, pairChildB); - if (a == b) - return a; - pairUnionKey = (uint64_t{a} << 32) | uint64_t{b}; - if (auto existing = pairUnionAccessSets.find(pairUnionKey); existing != pairUnionAccessSets.end()) - return existing->second; - hasPairUnionKey = true; - } + if (keyChildren.empty() && keyDirect.size() == 1) + return internSingleton(keyDirect.front()); + + /* Memoise on the canonical input. Publishing the same input tuple over and + over is the normal case — the same thunks and imports are re-forced under + every target — and without this each repeat pays a full flatten, sort and + dedup of the transitive union just to discover, via `accessSetIdsByHash`, + that the resulting set already exists. The union is unbounded (sets here + average ~160 members and target roots are far larger); the input is a + handful of ids. */ + auto inputKeyHash = hashAccessSetInputKey(keyDirect, keyChildren); + bool memoised = false; + if (auto existing = lookupInputKey(inputKeyHash, keyDirect, keyChildren, memoised); memoised) + return existing; static thread_local std::vector items; auto buildItems = [&] { - size_t itemCount = directCount; - for (auto child : children) - if (child != emptyEvalSourceAccessSetId && child < accessSets.size()) + size_t itemCount = keyDirect.size(); + for (auto child : keyChildren) + if (child < accessSets.size()) itemCount += accessSets[child].count; items.clear(); items.reserve(itemCount); - - for (auto access : directAccesses) - if (access != emptyEvalSourceAccessId) - items.push_back(access); - for (auto child : children) { - if (child == emptyEvalSourceAccessSetId || child >= accessSets.size()) + items.insert(items.end(), keyDirect.begin(), keyDirect.end()); + for (auto child : keyChildren) { + if (child >= accessSets.size()) continue; auto node = accessSets[child]; items.insert( @@ -258,13 +352,17 @@ EvalSourceAccessSetId EvalSourceAccessSetGraph::internAccessSet( }; auto hash = buildItems(); - if (items.empty()) + if (items.empty()) { + rememberInputKey(inputKeyHash, keyDirect, keyChildren, emptyEvalSourceAccessSetId); return emptyEvalSourceAccessSetId; - if (items.size() == 1) - return internSingleton(items.front()); + } + if (items.size() == 1) { + auto singleton = internSingleton(items.front()); + rememberInputKey(inputKeyHash, keyDirect, keyChildren, singleton); + return singleton; + } if (auto existing = lookupAccessSet(hash); existing != emptyEvalSourceAccessSetId) { - if (hasPairUnionKey) - pairUnionAccessSets.emplace(pairUnionKey, existing); + rememberInputKey(inputKeyHash, keyDirect, keyChildren, existing); return existing; } @@ -287,8 +385,7 @@ EvalSourceAccessSetId EvalSourceAccessSetGraph::internAccessSet( .nextWithSameHash = next, .hash = hash, }); - if (hasPairUnionKey) - pairUnionAccessSets.emplace(pairUnionKey, id); + rememberInputKey(inputKeyHash, keyDirect, keyChildren, id); return id; } @@ -372,19 +469,31 @@ EvalSourceAccessSetStats EvalSourceAccessSetGraph::stats() const }; } -/* Frames only suppress *consecutive* duplicates: `internAccessSet` sorts and - fully dedupes at publish time, so per-append deduplication would be - redundant work (and quadratic for large scope frames, e.g. the resolver - import scope). The consecutive check catches the common pattern of a loop - re-reading one path or re-forcing one value. */ +/* Frames suppress duplicates against a bounded window of recent appends rather + than against the whole frame: `internAccessSet` sorts and fully dedupes at + publish time, so exhaustive per-append deduplication would be redundant work + (and quadratic for large scope frames, e.g. the resolver import scope). + + The window is deliberately wider than the last entry. Duplicates arrive + interleaved, not just consecutively — a loop that forces a handful of values + round-robin, or a scope that re-reads two or three paths in rotation, defeats + a size-1 check entirely. Every duplicate that survives into the frame inflates + the publish: a larger input to hash and compare, and, on a memo miss, more + member lists to concatenate and sort. Scanning a fixed window keeps the append + O(1) while collapsing the common patterns. */ +/* Duplicate suppression compares against the frame's own last entry only, never + against entries below its watermark: those belong to the parent, and skipping + a push because the *parent* already holds the id would leave this frame with + no dependencies of its own, so its value would be published without a label. */ static void addFrameAccess(TrackedSourceDepsFrame & frame, EvalSourceAccessId access) { if (access == emptyEvalSourceAccessId) return; - auto size = frame.directSourceAccessSetAccesses.size(); - if (size == 0 || frame.directSourceAccessSetAccesses.data()[size - 1] != access) - frame.directSourceAccessSetAccesses.push_back(access); + auto & ids = frame.trackingCtx.frameAccessStack; + if (ids.size() > frame.accessBase && ids.back() == access) + return; + ids.push_back(access); } static void addFrameChild(TrackedSourceDepsFrame & frame, EvalSourceAccessSetId child) @@ -392,17 +501,10 @@ static void addFrameChild(TrackedSourceDepsFrame & frame, EvalSourceAccessSetId if (child == emptyEvalSourceAccessSetId) return; - auto size = frame.childSourceAccessSets.size(); - if (size == 0 || frame.childSourceAccessSets.data()[size - 1] != child) - frame.childSourceAccessSets.push_back(child); -} - -static void addToParentFrame(TrackedSourceDepsFrame & parent, EvalSourceAccessSetId accessSet) -{ - if (accessSet == emptyEvalSourceAccessSetId) + auto & ids = frame.trackingCtx.frameChildStack; + if (ids.size() > frame.childBase && ids.back() == child) return; - - addFrameChild(parent, accessSet); + ids.push_back(child); } static void addToCurrentFrame(TrackingContext & trackingCtx, EvalSourceAccessSetId accessSet) @@ -420,7 +522,8 @@ static void addToCurrentFrame(TrackingContext & trackingCtx, EvalSourceAccessSet static bool frameHasSourceDeps(const TrackedSourceDepsFrame & frame) { - return !frame.directSourceAccessSetAccesses.empty() || !frame.childSourceAccessSets.empty(); + return frame.trackingCtx.frameAccessStack.size() > frame.accessBase + || frame.trackingCtx.frameChildStack.size() > frame.childBase; } static EvalSourceAccessSetId internFrameAccessSet(TrackedSourceDepsFrame & frame) @@ -429,27 +532,52 @@ static EvalSourceAccessSetId internFrameAccessSet(TrackedSourceDepsFrame & frame return emptyEvalSourceAccessSetId; return frame.trackingCtx.sourceAccessSetGraph->internAccessSet( - std::span( - frame.directSourceAccessSetAccesses.data(), frame.directSourceAccessSetAccesses.size()), - std::span(frame.childSourceAccessSets.data(), frame.childSourceAccessSets.size())); + frame.directSourceAccessSetAccesses(), frame.childSourceAccessSets()); } -static void mergeFrameIntoParent(TrackedSourceDepsFrame & frame) +/* Truncate the frame's region and leave `accessSet` in its place, so the frame's + contribution to its parent is exactly the one interned id. Recording the marks + as the new bases lets frame teardown restore the stacks with a plain compare. */ +static void collapseFrameToAccessSet(TrackedSourceDepsFrame & frame, EvalSourceAccessSetId accessSet) { - auto * parent = frame.previous ? frame.previous : &frame.trackingCtx.rootFrame; - if (parent == &frame) - return; + auto & accesses = frame.trackingCtx.frameAccessStack; + auto & children = frame.trackingCtx.frameChildStack; + + if (accesses.size() > frame.accessBase) + accesses.resize(frame.accessBase); + if (children.size() > frame.childBase) + children.resize(frame.childBase); - for (auto access : frame.directSourceAccessSetAccesses) - addFrameAccess(*parent, access); - for (auto child : frame.childSourceAccessSets) - addFrameChild(*parent, child); + if (accessSet != emptyEvalSourceAccessSetId) { + auto * parent = frame.previous ? frame.previous : &frame.trackingCtx.rootFrame; + if (parent != &frame) + addFrameChild(*parent, accessSet); + else + addFrameChild(frame, accessSet); + } + + frame.accessBase = accesses.size(); + frame.childBase = children.size(); } void mergeUnpublishedTrackedSourceDepsFrame(TrackedSourceDepsFrame & frame) { + /* An unpublished frame needs no merge: its entries already sit contiguously + inside the parent's region, so leaving them in place is the merge. + + A published frame has already been collapsed to its interned id, and its + bases moved past it. Anything recorded after the publish is discarded -- + matching the copy-based implementation, where a published frame's entries + were simply never copied out. */ if (!frame.published) - mergeFrameIntoParent(frame); + return; + + auto & accesses = frame.trackingCtx.frameAccessStack; + auto & children = frame.trackingCtx.frameChildStack; + if (accesses.size() > frame.accessBase) + accesses.resize(frame.accessBase); + if (children.size() > frame.childBase) + children.resize(frame.childBase); } void recordTrackedSourceAccessSetAccess(EvalSourceAccessId access) @@ -481,34 +609,24 @@ void publishTrackedValueDependencies(const void * value) if (!frame || frame->published) return; - auto directCount = frame->directSourceAccessSetAccesses.size(); - auto childCount = frame->childSourceAccessSets.size(); + auto directAccesses = frame->directSourceAccessSetAccesses(); + auto children = frame->childSourceAccessSets(); - if (!frameHasSourceDeps(*frame)) { + if (directAccesses.empty() && children.empty()) { frame->published = true; return; } EvalSourceAccessSetId sourceAccessSet = emptyEvalSourceAccessSetId; - if (directCount == 0 && childCount == 1) { - sourceAccessSet = frame->childSourceAccessSets.data()[0]; + if (directAccesses.empty() && children.size() == 1) { + sourceAccessSet = children[0]; if (sourceAccessSet != emptyEvalSourceAccessSetId) frame->value->setTrackedSourceAccessSet(sourceAccessSet); } else { sourceAccessSet = publishTrackedSourceAccessSetDependencies( - *frame->trackingCtx.sourceAccessSetGraph, - *frame->value, - std::span( - frame->directSourceAccessSetAccesses.data(), frame->directSourceAccessSetAccesses.size()), - std::span( - frame->childSourceAccessSets.data(), frame->childSourceAccessSets.size())); - } - if (sourceAccessSet != emptyEvalSourceAccessSetId) { - if (frame->previous) - addToParentFrame(*frame->previous, sourceAccessSet); - else - addToCurrentFrame(frame->trackingCtx, sourceAccessSet); + *frame->trackingCtx.sourceAccessSetGraph, *frame->value, directAccesses, children); } + collapseFrameToAccessSet(*frame, sourceAccessSet); frame->accessSet = sourceAccessSet; frame->published = true; } @@ -557,10 +675,28 @@ void publishCopiedValueDependencies(void * dst, const void * src) std::span(children)); } +std::span TrackedSourceDepsFrame::directSourceAccessSetAccesses() const +{ + const auto & ids = trackingCtx.frameAccessStack; + if (ids.size() <= accessBase) + return {}; + return {ids.data() + accessBase, ids.size() - accessBase}; +} + +std::span TrackedSourceDepsFrame::childSourceAccessSets() const +{ + const auto & ids = trackingCtx.frameChildStack; + if (ids.size() <= childBase) + return {}; + return {ids.data() + childBase, ids.size() - childBase}; +} + TrackedSourceDepsFrame::TrackedSourceDepsFrame( TrackingContext & trackingCtx, Value * value, TrackedSourceDepsFrame * previous) : trackingCtx(trackingCtx) , value(value) + , accessBase(trackingCtx.frameAccessStack.size()) + , childBase(trackingCtx.frameChildStack.size()) , previous(previous) , nearestValueForceFrame( value ? this @@ -573,6 +709,12 @@ TrackingContext::TrackingContext(EvalState & evalState) : sourceAccessSetGraph(trackedSourceAccessSetGraph(evalState)) , rootFrame(*this) { + /* Sized to hold a deep force chain without reallocating mid-evaluation; + frames hold indices, not pointers, so a reallocation would be correct, + just wasteful. */ + frameAccessStack.reserve(4096); + frameChildStack.reserve(4096); + // Establish the invariant every hot path relies on: a live TrackingContext // implies an enabled graph, so forcing and the value hooks never re-check. sourceAccessSetGraph->enable(); @@ -623,12 +765,7 @@ EvalSourceAccessSetId TrackedSourceDepsScope::finish(Value * publishValue) currentTecnixThreadState.sourceDepsFrame = previousFrame; frame.accessSet = internFrameAccessSet(frame); - if (frame.accessSet != emptyEvalSourceAccessSetId) { - if (frame.previous) - addToParentFrame(*frame.previous, frame.accessSet); - else - addToCurrentFrame(frame.trackingCtx, frame.accessSet); - } + collapseFrameToAccessSet(frame, frame.accessSet); if (publishValue && frame.accessSet != emptyEvalSourceAccessSetId) publishValue->setTrackedSourceAccessSet(frame.accessSet); diff --git a/src/libfetchers/filtering-source-accessor.cc b/src/libfetchers/filtering-source-accessor.cc index 3c798f627..03a31ce95 100644 --- a/src/libfetchers/filtering-source-accessor.cc +++ b/src/libfetchers/filtering-source-accessor.cc @@ -96,6 +96,8 @@ struct AllowListSourceAccessorImpl : AllowListSourceAccessor SharedSync> allowedPrefixes; SharedSync> allowedPaths; + SharedSync> knownAllowed; + AllowListSourceAccessorImpl( ref next, std::set && allowedPrefixes, @@ -109,7 +111,15 @@ struct AllowListSourceAccessorImpl : AllowListSourceAccessor bool isAllowed(const CanonPath & path) override { - return allowedPaths.readLock()->contains(path) || path.isAllowed(*allowedPrefixes.readLock()); + if (knownAllowed.readLock()->contains(path)) + return true; + if (!(allowedPaths.readLock()->contains(path) || path.isAllowed(*allowedPrefixes.readLock()))) + return false; + // Only cache allows since allow may be widened later by `allowPrefix` but not narrowed + // and `CanonPath::isAllowed` may do more expensive I/O for a hot path (and this struct + // cannot be inherited/overriden externally) + knownAllowed.lock()->insert(path); + return true; } void allowPrefix(CanonPath prefix) override diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 795f3b373..7c3924624 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -940,16 +940,14 @@ struct GitSourceAccessor : SourceAccessor throw; } sizeCallback(s.s.size()); - StringSource source{s.s}; - source.drainInto(sink); + sink(s.s); return; } } auto view = std::string_view((const char *) git_blob_rawcontent(blob.get()), git_blob_rawsize(blob.get())); sizeCallback(view.size()); - StringSource source{view}; - source.drainInto(sink); + sink(view); } void readFile(const CanonPath & path, Sink & sink, fun sizeCallback) override diff --git a/src/libstore/store-dir-config.cc b/src/libstore/store-dir-config.cc index 61f82029d..f1889c366 100644 --- a/src/libstore/store-dir-config.cc +++ b/src/libstore/store-dir-config.cc @@ -6,20 +6,26 @@ namespace nix { -StorePath StoreDirConfig::parseStorePath(std::string_view path) const +/** + * Canonicalise a candidate store path. + * + * On Windows, `/nix/store` is not a canonical path. More broadly it is unclear + * whether this function should be using the native notion of a canonical path + * at all. For example, it makes to support remote stores whose store dir is a + * non-native path (e.g. Windows <-> Unix ssh-ing). + */ +static std::filesystem::path canonicaliseStorePathCandidate(std::string_view path) { - // On Windows, `/nix/store` is not a canonical path. More broadly it - // is unclear whether this function should be using the native - // notion of a canonical path at all. For example, it makes to - // support remote stores whose store dir is a non-native path (e.g. - // Windows <-> Unix ssh-ing). - auto p = #ifdef _WIN32 - std::filesystem::path(path) + return std::filesystem::path(path); #else - canonPath(std::string(path)) + return canonPath(std::string(path)); #endif - ; +} + +StorePath StoreDirConfig::parseStorePath(std::string_view path) const +{ + auto p = canonicaliseStorePathCandidate(path); if (p.parent_path() != storeDir) throw BadStorePath("path %s is not in the Nix store", PathFmt(p)); return StorePath(p.filename().string()); @@ -28,7 +34,10 @@ StorePath StoreDirConfig::parseStorePath(std::string_view path) const std::optional StoreDirConfig::maybeParseStorePath(std::string_view path) const { try { - return parseStorePath(path); + auto p = canonicaliseStorePathCandidate(path); + if (p.parent_path() != storeDir) + return {}; + return StorePath(p.filename().string()); } catch (Error &) { return {}; }