From 7b7e555bb827219f47020c3ffea8d80b503f0018 Mon Sep 17 00:00:00 2001 From: Bernardo Meurer Costa Date: Mon, 3 Nov 2025 22:10:29 +0000 Subject: [PATCH] perf(libstore/derivation-builder): pre-compute outputGraph for linear complexity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Build the output reference graph and inverse output map before running topoSort, avoiding quadratic complexity when determining which outputs reference each other. This fixes the FIXME comment about building the inverted map up front. The outputGraph (StorePath → StorePathSet) maps each output to its references, and will be reused in future commits to generate detailed cycle error messages showing which files contain problematic references. Adapted from Lix commit 10c04ce84. Change-Id: Ibdd46e7b2e895bfeeebc173046d1297b41998181 Co-Authored-By: Maximilian Bosch --- src/libstore/unix/build/derivation-builder.cc | 57 +++++++++++-------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 333c4dff8fe..70b039703ec 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -1470,31 +1470,40 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() outputStats.insert_or_assign(outputName, std::move(st)); } + /* Build output graph and inverse map up front to avoid quadratic complexity. */ + std::map outputGraph; + std::map inverseOutputMap; + for (auto & name : outputsToSort) { + inverseOutputMap[scratchOutputs.at(name)] = name; + } + + for (auto & name : outputsToSort) { + auto orifu = get(outputReferencesIfUnregistered, name); + if (!orifu) + throw BuildError( + BuildResult::Failure::OutputRejected, + "no output reference for '%s' in build of '%s'", + name, + store.printStorePath(drvPath)); + + std::visit( + overloaded{ + /* Since we'll use the already installed versions of these, we + can treat them as leaves and ignore any references they have. */ + [&](const AlreadyRegistered &) { outputGraph[scratchOutputs.at(name)] = StorePathSet{}; }, + [&](const PerhapsNeedToRegister & refs) { outputGraph[scratchOutputs.at(name)] = refs.refs; }}, + *orifu); + } + auto topoSortResult = topoSort(outputsToSort, {[&](const std::string & name) { - auto orifu = get(outputReferencesIfUnregistered, name); - if (!orifu) - throw BuildError( - BuildResult::Failure::OutputRejected, - "no output reference for '%s' in build of '%s'", - name, - store.printStorePath(drvPath)); - return std::visit( - overloaded{ - /* 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; - /* FIXME build inverted map up front so no quadratic waste here */ - for (auto & r : refs.refs) - for (auto & [o, p] : scratchOutputs) - if (r == p) - referencedOutputs.insert(o); - return referencedOutputs; - }, - }, - *orifu); + StringSet referencedOutputs; + for (auto & path : outputGraph.at(scratchOutputs.at(name))) { + auto outputName = inverseOutputMap.find(path); + if (outputName != inverseOutputMap.end()) { + referencedOutputs.insert(outputName->second); + } + } + return referencedOutputs; }}); auto sortedOutputNames = std::visit(