Skip to content

Commit

Permalink
Prevent nested set flattening in ActionSpawn if there are no input fi…
Browse files Browse the repository at this point in the history
…lesets.

This is always the case in Bazel. The nested set structure can be exploited to perform optimizations, notably by --experimental_remote_merkle_tree_cache.

PiperOrigin-RevId: 585657594
Change-Id: I83042b5c40b3b9e6ce511a230a0cbd0677dda5b3
  • Loading branch information
tjgq authored and copybara-github committed Nov 27, 2023
1 parent 6072a14 commit 67021ff
Showing 1 changed file with 22 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -520,14 +520,7 @@ private ActionSpawn(
parent.getRunfilesSupplier(),
parent,
parent.resourceSetOrBuilder);
NestedSetBuilder<ActionInput> inputsBuilder = NestedSetBuilder.stableOrder();
for (Artifact input : inputs.toList()) {
if (!input.isFileset()) {
inputsBuilder.add(input);
}
}
inputsBuilder.addAll(additionalInputs);
this.inputs = inputsBuilder.build();
this.inputs = getNonFilesetInputs(inputs).addAll(additionalInputs).build();
this.filesetMappings = filesetMappings;
this.pathMapper = pathMapper;

Expand All @@ -542,6 +535,27 @@ private ActionSpawn(
this.reportOutputs = reportOutputs;
}

/** Returns a {@link NestedSetBuilder} containing only the non-fileset inputs. */
private static NestedSetBuilder<ActionInput> getNonFilesetInputs(NestedSet<Artifact> inputs) {
NestedSetBuilder<ActionInput> builder = NestedSetBuilder.stableOrder();
// TODO(tjgq): Investigate whether we can avoid flattening when filesetMappings is empty.
// This requires auditing getSpawnForExtraAction(), which doesn't appear to propagate the
// filesetMappings from the shadowed action.
boolean hasFilesets = false;
for (Artifact input : inputs.toList()) {
if (!input.isFileset()) {
builder.add(input);
} else {
hasFilesets = true;
}
}
// If possible, keep the original nested set. This aids callers that exploit the nested set
// structure to perform optimizations (see SpawnInputExpander#walkInputs and its callers).
return hasFilesets
? builder
: NestedSetBuilder.<ActionInput>stableOrder().addTransitive(inputs);
}

@Override
public PathMapper getPathMapper() {
return pathMapper;
Expand Down

0 comments on commit 67021ff

Please sign in to comment.