Do not expand depset twice and store expanded in ctx.actions.run inputs.
Previously, when inputs is a depset, we'd:
* Expand it to a collection, then
* Expand it again and add it as a flat list to the input builder
This would be square on both performance and analysis time memory.
Note that we _still_ expand the depset once to try to see if any input is a tool. This will have to be fixed in a future CL.
RELNOTES: None
PiperOrigin-RevId: 186566243
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
index 4ba8102..d991f49 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
@@ -599,7 +599,6 @@
throws EvalException {
context.checkMutable("actions.run_shell");
- // TODO(bazel-team): builder still makes unnecessary copies of inputs, outputs and args.
SkylarkList argumentList = (SkylarkList) arguments;
SpawnAction.Builder builder = new SpawnAction.Builder();
buildCommandLine(builder, argumentList);
@@ -705,14 +704,14 @@
Object inputManifestsUnchecked,
SpawnAction.Builder builder)
throws EvalException {
- // TODO(bazel-team): builder still makes unnecessary copies of inputs, outputs and args.
Iterable<Artifact> inputArtifacts;
if (inputs instanceof SkylarkList) {
inputArtifacts = ((SkylarkList) inputs).getContents(Artifact.class, "inputs");
builder.addInputs(inputArtifacts);
} else {
- inputArtifacts = ((SkylarkNestedSet) inputs).toCollection(Artifact.class);
- builder.addInputs(((SkylarkNestedSet) inputs).getSet(Artifact.class));
+ NestedSet<Artifact> inputSet = ((SkylarkNestedSet) inputs).getSet(Artifact.class);
+ builder.addTransitiveInputs(inputSet);
+ inputArtifacts = inputSet;
}
builder.addOutputs(outputs.getContents(Artifact.class, "outputs"));
