Skip to content

Commit 7b7e555

Browse files
lovesegfaultMa27
andcommitted
perf(libstore/derivation-builder): pre-compute outputGraph for linear complexity
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 <[email protected]>
1 parent 87a2ce4 commit 7b7e555

File tree

1 file changed

+33
-24
lines changed

1 file changed

+33
-24
lines changed

src/libstore/unix/build/derivation-builder.cc

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,31 +1470,40 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
14701470
outputStats.insert_or_assign(outputName, std::move(st));
14711471
}
14721472

1473+
/* Build output graph and inverse map up front to avoid quadratic complexity. */
1474+
std::map<StorePath, StorePathSet> outputGraph;
1475+
std::map<StorePath, std::string> inverseOutputMap;
1476+
for (auto & name : outputsToSort) {
1477+
inverseOutputMap[scratchOutputs.at(name)] = name;
1478+
}
1479+
1480+
for (auto & name : outputsToSort) {
1481+
auto orifu = get(outputReferencesIfUnregistered, name);
1482+
if (!orifu)
1483+
throw BuildError(
1484+
BuildResult::Failure::OutputRejected,
1485+
"no output reference for '%s' in build of '%s'",
1486+
name,
1487+
store.printStorePath(drvPath));
1488+
1489+
std::visit(
1490+
overloaded{
1491+
/* Since we'll use the already installed versions of these, we
1492+
can treat them as leaves and ignore any references they have. */
1493+
[&](const AlreadyRegistered &) { outputGraph[scratchOutputs.at(name)] = StorePathSet{}; },
1494+
[&](const PerhapsNeedToRegister & refs) { outputGraph[scratchOutputs.at(name)] = refs.refs; }},
1495+
*orifu);
1496+
}
1497+
14731498
auto topoSortResult = topoSort(outputsToSort, {[&](const std::string & name) {
1474-
auto orifu = get(outputReferencesIfUnregistered, name);
1475-
if (!orifu)
1476-
throw BuildError(
1477-
BuildResult::Failure::OutputRejected,
1478-
"no output reference for '%s' in build of '%s'",
1479-
name,
1480-
store.printStorePath(drvPath));
1481-
return std::visit(
1482-
overloaded{
1483-
/* Since we'll use the already installed versions of these, we
1484-
can treat them as leaves and ignore any references they
1485-
have. */
1486-
[&](const AlreadyRegistered &) { return StringSet{}; },
1487-
[&](const PerhapsNeedToRegister & refs) {
1488-
StringSet referencedOutputs;
1489-
/* FIXME build inverted map up front so no quadratic waste here */
1490-
for (auto & r : refs.refs)
1491-
for (auto & [o, p] : scratchOutputs)
1492-
if (r == p)
1493-
referencedOutputs.insert(o);
1494-
return referencedOutputs;
1495-
},
1496-
},
1497-
*orifu);
1499+
StringSet referencedOutputs;
1500+
for (auto & path : outputGraph.at(scratchOutputs.at(name))) {
1501+
auto outputName = inverseOutputMap.find(path);
1502+
if (outputName != inverseOutputMap.end()) {
1503+
referencedOutputs.insert(outputName->second);
1504+
}
1505+
}
1506+
return referencedOutputs;
14981507
}});
14991508

15001509
auto sortedOutputNames = std::visit(

0 commit comments

Comments
 (0)