Change Spawn.get{Tool,Input}Files to NestedSet

I think this should basically be a no-op. In a few places, it delays
flattening of the nested set, and could potentially reduce peak memory
use / reduce the time we're holding onto a flattened copy of the inputs.

PiperOrigin-RevId: 288673175

src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java

@@ -18,6 +18,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
 import java.util.List;
@@ -102,12 +103,12 @@ private PathFragment getRunfilesRoot() {
   }
 
   @Override
-  public Iterable<? extends ActionInput> getToolFiles() {
+  public NestedSet<? extends ActionInput> getToolFiles() {
     return action.getTools();
   }
 
   @Override
-  public Iterable<? extends ActionInput> getInputFiles() {
+  public NestedSet<? extends ActionInput> getInputFiles() {
     return action.getInputs();
   }
 

src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java

@@ -17,6 +17,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import java.util.Collection;
 import javax.annotation.Nullable;
 
@@ -58,12 +59,12 @@ public ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> getFilesetMap
   }
 
   @Override
-  public Iterable<? extends ActionInput> getToolFiles() {
+  public NestedSet<? extends ActionInput> getToolFiles() {
     return spawn.getToolFiles();
   }
 
   @Override
-  public Iterable<? extends ActionInput> getInputFiles() {
+  public NestedSet<? extends ActionInput> getInputFiles() {
     return spawn.getInputFiles();
   }
 

src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java

@@ -17,7 +17,11 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
 import java.util.Map;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
@@ -32,8 +36,8 @@ public final class SimpleSpawn implements Spawn {
   private final ImmutableList<String> arguments;
   private final ImmutableMap<String, String> environment;
   private final ImmutableMap<String, String> executionInfo;
-  private final ImmutableList<? extends ActionInput> inputs;
-  private final ImmutableList<? extends ActionInput> tools;
+  private final NestedSet<? extends ActionInput> inputs;
+  private final NestedSet<? extends ActionInput> tools;
   private final RunfilesSupplier runfilesSupplier;
   private final Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings;
   private final ImmutableList<? extends ActionInput> outputs;
@@ -46,9 +50,9 @@ public SimpleSpawn(
       ImmutableMap<String, String> executionInfo,
       RunfilesSupplier runfilesSupplier,
       Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings,
-      ImmutableList<? extends ActionInput> inputs,
-      ImmutableList<? extends ActionInput> tools,
-      ImmutableList<? extends ActionInput> outputs,
+      NestedSet<? extends ActionInput> inputs,
+      NestedSet<? extends ActionInput> tools,
+      ImmutableSet<? extends ActionInput> outputs,
       ResourceSet localResources) {
     this.owner = Preconditions.checkNotNull(owner);
     this.arguments = Preconditions.checkNotNull(arguments);
@@ -59,7 +63,7 @@ public SimpleSpawn(
     this.runfilesSupplier =
         runfilesSupplier == null ? EmptyRunfilesSupplier.INSTANCE : runfilesSupplier;
     this.filesetMappings = filesetMappings;
-    this.outputs = Preconditions.checkNotNull(outputs);
+    this.outputs = Preconditions.checkNotNull(outputs).asList();
     this.localResources = Preconditions.checkNotNull(localResources);
   }
 
@@ -68,8 +72,8 @@ public SimpleSpawn(
       ImmutableList<String> arguments,
       ImmutableMap<String, String> environment,
       ImmutableMap<String, String> executionInfo,
-      ImmutableList<? extends ActionInput> inputs,
-      ImmutableList<? extends ActionInput> outputs,
+      NestedSet<? extends ActionInput> inputs,
+      ImmutableSet<? extends ActionInput> outputs,
       ResourceSet localResources) {
     this(
         owner,
@@ -79,7 +83,7 @@ public SimpleSpawn(
         null,
         ImmutableMap.of(),
         inputs,
-        ImmutableList.<Artifact>of(),
+        NestedSetBuilder.emptySet(Order.STABLE_ORDER),
         outputs,
         localResources);
   }
@@ -110,12 +114,12 @@ public ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> getFilesetMap
   }
 
   @Override
-  public ImmutableList<? extends ActionInput> getInputFiles() {
+  public NestedSet<? extends ActionInput> getInputFiles() {
     return inputs;
   }
 
   @Override
-  public ImmutableList<? extends ActionInput> getToolFiles() {
+  public NestedSet<? extends ActionInput> getToolFiles() {
     return tools;
   }
 

src/main/java/com/google/devtools/build/lib/actions/Spawn.java

@@ -17,6 +17,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import java.util.Collection;
 import javax.annotation.Nullable;
 
@@ -65,26 +66,26 @@ public interface Spawn {
    *
    * <p>The returned set of files is a subset of what getInputFiles() returns.
    *
-   * <p>This method explicitly does not expand middleman artifacts. Pass the result
-   * to an appropriate utility method on {@link com.google.devtools.build.lib.actions.Artifact} to
-   * expand the middlemen.
+   * <p>This method explicitly does not expand middleman artifacts. Pass the result to an
+   * appropriate utility method on {@link com.google.devtools.build.lib.actions.Artifact} to expand
+   * the middlemen.
    *
-   * <p>This is for use with persistent workers, so we can restart workers when their binaries
-   * have changed.
+   * <p>This is for use with persistent workers, so we can restart workers when their binaries have
+   * changed.
    */
-  Iterable<? extends ActionInput> getToolFiles();
+  NestedSet<? extends ActionInput> getToolFiles();
 
   /**
    * Returns the list of files that this command may read.
    *
-   * <p>This method explicitly does not expand middleman artifacts. Pass the result
-   * to an appropriate utility method on {@link com.google.devtools.build.lib.actions.Artifact} to
-   * expand the middlemen.
+   * <p>This method explicitly does not expand middleman artifacts. Pass the result to an
+   * appropriate utility method on {@link com.google.devtools.build.lib.actions.Artifact} to expand
+   * the middlemen.
    *
-   * <p>This is for use with remote execution, so we can ship inputs before starting the
-   * command. Order stability across multiple calls should be upheld for performance reasons.
+   * <p>This is for use with remote execution, so we can ship inputs before starting the command.
+   * Order stability across multiple calls should be upheld for performance reasons.
    */
-  Iterable<? extends ActionInput> getInputFiles();
+  NestedSet<? extends ActionInput> getInputFiles();
 
   /**
    * Returns the collection of files that this command must write.  Callers should not mutate

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

@@ -383,7 +383,7 @@ public final Spawn getSpawn() throws CommandLineExpansionException {
     return getSpawn(getInputs());
   }
 
-  final Spawn getSpawn(Iterable<Artifact> inputs) throws CommandLineExpansionException {
+  final Spawn getSpawn(NestedSet<Artifact> inputs) throws CommandLineExpansionException {
     return new ActionSpawn(
         commandLines.allArguments(),
         ImmutableMap.of(),
@@ -547,8 +547,7 @@ public Map<String, String> getExecutionInfo() {
 
   /** A spawn instance that is tied to a specific SpawnAction. */
   private class ActionSpawn extends BaseSpawn {
-
-    private final ImmutableList<ActionInput> inputs;
+    private final NestedSet<ActionInput> inputs;
     private final Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings;
     private final ImmutableMap<String, String> effectiveEnvironment;
 
@@ -561,7 +560,7 @@ private class ActionSpawn extends BaseSpawn {
     private ActionSpawn(
         ImmutableList<String> arguments,
         Map<String, String> clientEnv,
-        Iterable<Artifact> inputs,
+        NestedSet<Artifact> inputs,
         Iterable<? extends ActionInput> additionalInputs,
         Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings) {
       super(
@@ -571,7 +570,7 @@ private ActionSpawn(
           SpawnAction.this.getRunfilesSupplier(),
           SpawnAction.this,
           resourceSet);
-      ImmutableList.Builder<ActionInput> inputsBuilder = ImmutableList.builder();
+      NestedSetBuilder<ActionInput> inputsBuilder = NestedSetBuilder.stableOrder();
       ImmutableList<Artifact> manifests = getRunfilesSupplier().getManifests();
       for (Artifact input : inputs) {
         if (!input.isFileset() && !manifests.contains(input)) {
@@ -597,7 +596,7 @@ public ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> getFilesetMap
     }
 
     @Override
-    public Iterable<? extends ActionInput> getInputFiles() {
+    public NestedSet<? extends ActionInput> getInputFiles() {
       return inputs;
     }
   }

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

@@ -206,8 +206,7 @@ Spawn getSpawnForExtraAction() throws CommandLineExpansionException {
 
   @Override
   public Iterable<Artifact> getInputFilesForExtraAction(
-      ActionExecutionContext actionExecutionContext)
-      throws ActionExecutionException, InterruptedException {
+      ActionExecutionContext actionExecutionContext) {
     return allInputs;
   }
 

src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetBuilder.java

@@ -274,7 +274,7 @@ public static <E> NestedSetBuilder<E> naiveLinkOrder() {
     return new NestedSetBuilder<>(Order.NAIVE_LINK_ORDER);
   }
 
-  public static <E> NestedSetBuilder<E> fromNestedSet(NestedSet<E> set) {
+  public static <E> NestedSetBuilder<E> fromNestedSet(NestedSet<? extends E> set) {
     return new NestedSetBuilder<E>(set.getOrder()).addTransitive(set);
   }
 

src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java

@@ -17,12 +17,12 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.google.common.io.ByteStreams;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionInputHelper;
-import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ArtifactPathResolver;
 import com.google.devtools.build.lib.actions.EnvironmentalExecException;
 import com.google.devtools.build.lib.actions.ExecException;
@@ -43,6 +43,8 @@
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.TestResult.ExecutionInfo;
 import com.google.devtools.build.lib.buildeventstream.TestFileNameConstants;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.util.Pair;
@@ -132,9 +134,9 @@ public TestRunnerSpawn createTestRunnerSpawn(
             ImmutableMap.copyOf(executionInfo),
             action.getRunfilesSupplier(),
             ImmutableMap.of(),
-            /*inputs=*/ action.getInputs().toList(),
-            /*tools=*/ ImmutableList.<Artifact>of(),
-            ImmutableList.copyOf(action.getSpawnOutputs()),
+            /*inputs=*/ action.getInputs(),
+            /*tools=*/ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
+            ImmutableSet.copyOf(action.getSpawnOutputs()),
             localResourceUsage);
     return new StandaloneTestRunnerSpawn(
         action, actionExecutionContext, spawn, tmpDir, workingDirectory, execRoot);
@@ -398,9 +400,10 @@ private Spawn createXmlGeneratingSpawn(TestRunnerAction action, SpawnResult resu
         ImmutableMap.copyOf(action.getExecutionInfo()),
         null,
         ImmutableMap.of(),
-        /*inputs=*/ ImmutableList.of(action.getTestXmlGeneratorScript(), action.getTestLog()),
-        /*tools=*/ ImmutableList.<Artifact>of(),
-        /*outputs=*/ ImmutableList.of(ActionInputHelper.fromPath(action.getXmlOutputPath())),
+        /*inputs=*/ NestedSetBuilder.create(
+            Order.STABLE_ORDER, action.getTestXmlGeneratorScript(), action.getTestLog()),
+        /*tools=*/ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
+        /*outputs=*/ ImmutableSet.of(ActionInputHelper.fromPath(action.getXmlOutputPath())),
         SpawnAction.DEFAULT_RESOURCE_SET);
   }
 

src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java

@@ -44,6 +44,8 @@
 import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.includescanning.IncludeParser.GrepIncludesFileType;
 import com.google.devtools.build.lib.includescanning.IncludeParser.Inclusion;
 import com.google.devtools.build.lib.util.io.FileOutErr;
@@ -340,8 +342,9 @@ static InputStream spawnGrep(
       GrepIncludesFileType fileType)
       throws ExecException, InterruptedException {
     ActionInput output = ActionInputHelper.fromPath(outputExecPath);
-    ImmutableList<? extends ActionInput> inputs = ImmutableList.of(grepIncludes, input);
-    ImmutableList<ActionInput> outputs = ImmutableList.of(output);
+    NestedSet<? extends ActionInput> inputs =
+        NestedSetBuilder.create(Order.STABLE_ORDER, grepIncludes, input);
+    ImmutableSet<ActionInput> outputs = ImmutableSet.of(output);
     ImmutableList<String> command =
         ImmutableList.of(
             grepIncludes.getExecPathString(),
@@ -459,8 +462,9 @@ private static ListenableFuture<InputStream> spawnGrepAsync(
       Artifact grepIncludes,
       GrepIncludesFileType fileType) {
     ActionInput output = ActionInputHelper.fromPath(outputExecPath);
-    ImmutableList<? extends ActionInput> inputs = ImmutableList.of(grepIncludes, input);
-    ImmutableList<ActionInput> outputs = ImmutableList.of(output);
+    NestedSet<? extends ActionInput> inputs =
+        NestedSetBuilder.create(Order.STABLE_ORDER, grepIncludes, input);
+    ImmutableSet<ActionInput> outputs = ImmutableSet.of(output);
     ImmutableList<String> command =
         ImmutableList.of(
             grepIncludes.getExecPathString(),

src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java

@@ -380,7 +380,7 @@ public ImmutableSet<Artifact> getMandatoryOutputs() {
    * {@link #discoverInputs(ActionExecutionContext)} must be called before this method is called on
    * each action execution.
    */
-  public Iterable<Artifact> getAdditionalInputs() {
+  public NestedSet<Artifact> getAdditionalInputs() {
     return Preconditions.checkNotNull(additionalInputs);
   }
 
@@ -1420,19 +1420,16 @@ protected byte[] getDotDContents(SpawnResult spawnResult) throws EnvironmentalEx
   protected Spawn createSpawn(Map<String, String> clientEnv) throws ActionExecutionException {
     // Intentionally not adding {@link CppCompileAction#inputsForInvalidation}, those are not needed
     // for execution.
-    ImmutableList.Builder<ActionInput> inputsBuilder =
-        new ImmutableList.Builder<ActionInput>()
-            .addAll(getMandatoryInputs())
-            .addAll(getAdditionalInputs());
+    NestedSetBuilder<ActionInput> inputsBuilder =
+        NestedSetBuilder.<ActionInput>stableOrder()
+            .addTransitive(getMandatoryInputs())
+            .addTransitive(getAdditionalInputs());
     if (getParamFileActionInput() != null) {
       inputsBuilder.add(getParamFileActionInput());
     }
-    ImmutableList<ActionInput> inputs = inputsBuilder.build();
+    NestedSet<ActionInput> inputs = inputsBuilder.build();
 
     ImmutableMap<String, String> executionInfo = getExecutionInfo();
-    ImmutableList.Builder<ActionInput> outputs =
-        ImmutableList.builderWithExpectedSize(getOutputs().size() + 1);
-    outputs.addAll(getOutputs());
     if (getDotdFile() != null && useInMemoryDotdFiles()) {
       /*
        * CppCompileAction does dotd file scanning locally inside the Bazel process and thus
@@ -1458,7 +1455,7 @@ protected Spawn createSpawn(Map<String, String> clientEnv) throws ActionExecutio
           getEnvironment(clientEnv),
           executionInfo,
           inputs,
-          outputs.build(),
+          getOutputs(),
           estimateResourceConsumptionLocal());
     } catch (CommandLineExpansionException e) {
       throw new ActionExecutionException(

src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java

@@ -329,8 +329,8 @@ private Spawn createSpawn(ActionExecutionContext actionExecutionContext)
           ImmutableList.copyOf(getCommandLine(actionExecutionContext.getArtifactExpander())),
           getEnvironment(actionExecutionContext.getClientEnv()),
           getExecutionInfo(),
-          ImmutableList.copyOf(getInputs()),
-          getOutputs().asList(),
+          getInputs(),
+          getOutputs(),
           estimateResourceConsumptionLocal());
     } catch (CommandLineExpansionException e) {
       throw new ActionExecutionException(