From c33b2c583466b14d23bc85caa35f737886842e21 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 25 Nov 2025 17:01:23 -0500 Subject: [PATCH] perf(libstore/derivation-builder): Futher simplify / maybe optimize We can precompute the exact information we need for topo sorting and store it in `PerhapsNeedToRegister`. Depending on how `topoSort` works, this is easy a performance improvement or just completely harmless. Co-Authored-By: Bernardo Meurer Costa --- src/libstore/unix/build/derivation-builder.cc | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index ebd53472b..2d66eb405 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -1396,6 +1396,11 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() struct PerhapsNeedToRegister { StorePathSet refs; + /** + * References to other outputs. Built by looking up in + * `scratchOutputsInverse`. + */ + StringSet otherOutputs; }; /* inverse map of scratchOutputs for efficient lookup */ @@ -1471,12 +1476,24 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() references = scanForReferences(blank, actualPath, referenceablePaths); } - outputReferencesIfUnregistered.insert_or_assign(outputName, PerhapsNeedToRegister{.refs = references}); + StringSet referencedOutputs; + for (auto & r : references) + if (auto * o = get(scratchOutputsInverse, r)) + referencedOutputs.insert(*o); + + outputReferencesIfUnregistered.insert_or_assign( + outputName, + PerhapsNeedToRegister{ + .refs = references, + .otherOutputs = referencedOutputs, + }); outputStats.insert_or_assign(outputName, std::move(st)); } - auto topoSortResult = topoSort(outputsToSort, [&](const std::string & name) { - auto orifu = get(outputReferencesIfUnregistered, name); + StringSet emptySet; + + auto topoSortResult = topoSort(outputsToSort, [&](const std::string & name) -> const StringSet & { + auto * orifu = get(outputReferencesIfUnregistered, name); if (!orifu) throw BuildError( BuildResult::Failure::OutputRejected, @@ -1488,14 +1505,8 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() /* Since we'll use the already installed versions of these, we can treat them as leaves and ignore any references they have. */ - [&](const AlreadyRegistered &) { return StringSet{}; }, - [&](const PerhapsNeedToRegister & refs) { - StringSet referencedOutputs; - for (auto & r : refs.refs) - if (auto * o = get(scratchOutputsInverse, r)) - referencedOutputs.insert(*o); - return referencedOutputs; - }, + [&](const AlreadyRegistered &) -> const StringSet & { return emptySet; }, + [&](const PerhapsNeedToRegister & refs) -> const StringSet & { return refs.otherOutputs; }, }, *orifu); });