Skip to content

Commit 4b5a1ca

Browse files
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 <[email protected]>
1 parent 8a3a773 commit 4b5a1ca

File tree

1 file changed

+37
-25
lines changed

1 file changed

+37
-25
lines changed

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

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1396,6 +1396,11 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
13961396
struct PerhapsNeedToRegister
13971397
{
13981398
StorePathSet refs;
1399+
/**
1400+
* References to other outputs. Built by looking up in
1401+
* `scratchOutputsInverse`.
1402+
*/
1403+
StringSet otherOutputs;
13991404
};
14001405

14011406
/* inverse map of scratchOutputs for efficient lookup */
@@ -1471,34 +1476,41 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
14711476
references = scanForReferences(blank, actualPath, referenceablePaths);
14721477
}
14731478

1474-
outputReferencesIfUnregistered.insert_or_assign(outputName, PerhapsNeedToRegister{.refs = references});
1479+
StringSet referencedOutputs;
1480+
for (auto & r : references)
1481+
if (auto * o = get(scratchOutputsInverse, r))
1482+
referencedOutputs.insert(*o);
1483+
1484+
outputReferencesIfUnregistered.insert_or_assign(
1485+
outputName,
1486+
PerhapsNeedToRegister{
1487+
.refs = references,
1488+
.otherOutputs = referencedOutputs,
1489+
});
14751490
outputStats.insert_or_assign(outputName, std::move(st));
14761491
}
14771492

1478-
auto topoSortResult = topoSort(outputsToSort, {[&](const std::string & name) {
1479-
auto orifu = get(outputReferencesIfUnregistered, name);
1480-
if (!orifu)
1481-
throw BuildError(
1482-
BuildResult::Failure::OutputRejected,
1483-
"no output reference for '%s' in build of '%s'",
1484-
name,
1485-
store.printStorePath(drvPath));
1486-
return std::visit(
1487-
overloaded{
1488-
/* Since we'll use the already installed versions of these, we
1489-
can treat them as leaves and ignore any references they
1490-
have. */
1491-
[&](const AlreadyRegistered &) { return StringSet{}; },
1492-
[&](const PerhapsNeedToRegister & refs) {
1493-
StringSet referencedOutputs;
1494-
for (auto & r : refs.refs)
1495-
if (auto * o = get(scratchOutputsInverse, r))
1496-
referencedOutputs.insert(*o);
1497-
return referencedOutputs;
1498-
},
1499-
},
1500-
*orifu);
1501-
}});
1493+
StringSet emptySet;
1494+
1495+
auto topoSortResult =
1496+
topoSort(outputsToSort, {[&](const std::string & name) {
1497+
auto orifu = get(outputReferencesIfUnregistered, name);
1498+
if (!orifu)
1499+
throw BuildError(
1500+
BuildResult::Failure::OutputRejected,
1501+
"no output reference for '%s' in build of '%s'",
1502+
name,
1503+
store.printStorePath(drvPath));
1504+
return std::visit(
1505+
overloaded{
1506+
/* Since we'll use the already installed versions of these, we
1507+
can treat them as leaves and ignore any references they
1508+
have. */
1509+
[&](const AlreadyRegistered &) -> const StringSet & { return emptySet; },
1510+
[&](const PerhapsNeedToRegister & refs) -> const StringSet & { return refs.otherOutputs; },
1511+
},
1512+
*orifu);
1513+
}});
15021514

15031515
auto sortedOutputNames = std::visit(
15041516
overloaded{

0 commit comments

Comments
 (0)