Skip to content

Commit 41ca39a

Browse files
tjgqcopybara-github
authored andcommitted
Use filesetMappings to determine whether filesets must be filtered out of the inputs.
This avoids a potentially costly flattening in the likely case (100% likely for Bazel) that there are no input filesets. PiperOrigin-RevId: 586995584 Change-Id: Icce6d4e2e61d2c3808f7b467faf545a2e932b759
1 parent 68362d0 commit 41ca39a

File tree

1 file changed

+15
-15
lines changed

1 file changed

+15
-15
lines changed

src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,11 @@ private ActionSpawn(
520520
parent.getRunfilesSupplier(),
521521
parent,
522522
parent.resourceSetOrBuilder);
523-
this.inputs = getNonFilesetInputs(inputs).addAll(additionalInputs).build();
523+
NestedSetBuilder<ActionInput> inputsBuilder = NestedSetBuilder.stableOrder();
524+
addNonFilesetInputs(inputsBuilder, inputs, filesetMappings);
525+
inputsBuilder.addAll(additionalInputs);
526+
527+
this.inputs = inputsBuilder.build();
524528
this.filesetMappings = filesetMappings;
525529
this.pathMapper = pathMapper;
526530

@@ -535,25 +539,21 @@ private ActionSpawn(
535539
this.reportOutputs = reportOutputs;
536540
}
537541

538-
/** Returns a {@link NestedSetBuilder} containing only the non-fileset inputs. */
539-
private static NestedSetBuilder<ActionInput> getNonFilesetInputs(NestedSet<Artifact> inputs) {
540-
NestedSetBuilder<ActionInput> builder = NestedSetBuilder.stableOrder();
541-
// TODO(tjgq): Investigate whether we can avoid flattening when filesetMappings is empty.
542-
// This requires auditing getSpawnForExtraAction(), which doesn't appear to propagate the
543-
// filesetMappings from the shadowed action.
544-
boolean hasFilesets = false;
542+
private static void addNonFilesetInputs(
543+
NestedSetBuilder<ActionInput> builder,
544+
NestedSet<Artifact> inputs,
545+
Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings) {
546+
if (filesetMappings.isEmpty()) {
547+
// Keep the original nested set intact. This aids callers that exploit the nested set
548+
// structure to perform optimizations (see SpawnInputExpander#walkInputs and its callers).
549+
builder.addTransitive(inputs);
550+
return;
551+
}
545552
for (Artifact input : inputs.toList()) {
546553
if (!input.isFileset()) {
547554
builder.add(input);
548-
} else {
549-
hasFilesets = true;
550555
}
551556
}
552-
// If possible, keep the original nested set. This aids callers that exploit the nested set
553-
// structure to perform optimizations (see SpawnInputExpander#walkInputs and its callers).
554-
return hasFilesets
555-
? builder
556-
: NestedSetBuilder.<ActionInput>stableOrder().addTransitive(inputs);
557557
}
558558

559559
@Override

0 commit comments

Comments
 (0)