Use a CommandLine to represent executable arguments in SpawnAction
This reduced analysis memory usage for actions that construct complex
argument lists.
PiperOrigin-RevId: 218388661
diff --git a/src/main/java/com/google/devtools/build/lib/actions/CommandLine.java b/src/main/java/com/google/devtools/build/lib/actions/CommandLine.java
index b87e5a6..1516fb0 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/CommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/CommandLine.java
@@ -16,9 +16,9 @@
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.collect.CollectionUtils;
+import com.google.devtools.build.lib.collect.IterablesChain;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.util.Fingerprint;
@@ -94,13 +94,13 @@
@Override
public Iterable<String> arguments() throws CommandLineExpansionException {
- return Iterables.concat(executableArgs, commandLine.arguments());
+ return IterablesChain.concat(executableArgs, commandLine.arguments());
}
@Override
public Iterable<String> arguments(ArtifactExpander artifactExpander)
throws CommandLineExpansionException {
- return Iterables.concat(executableArgs, commandLine.arguments(artifactExpander));
+ return IterablesChain.concat(executableArgs, commandLine.arguments(artifactExpander));
}
}
@@ -118,13 +118,13 @@
@Override
public Iterable<String> arguments() throws CommandLineExpansionException {
- return Iterables.concat(commandLine.arguments(), executableArgs);
+ return IterablesChain.concat(commandLine.arguments(), executableArgs);
}
@Override
public Iterable<String> arguments(ArtifactExpander artifactExpander)
throws CommandLineExpansionException {
- return Iterables.concat(commandLine.arguments(artifactExpander), executableArgs);
+ return IterablesChain.concat(commandLine.arguments(artifactExpander), executableArgs);
}
}
@@ -137,6 +137,9 @@
if (executableArgs.isEmpty()) {
return commandLine;
}
+ if (commandLine == EMPTY) {
+ return CommandLine.of(executableArgs);
+ }
return new PrefixedCommandLine(executableArgs, commandLine);
}
@@ -149,6 +152,9 @@
if (args.isEmpty()) {
return commandLine;
}
+ if (commandLine == EMPTY) {
+ return CommandLine.of(args);
+ }
return new SuffixedCommandLine(args, commandLine);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index e3ba208..c3c268c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
@@ -22,7 +22,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
import com.google.devtools.build.lib.actions.AbstractAction;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionEnvironment;
@@ -76,7 +75,6 @@
import com.google.errorprone.annotations.FormatMethod;
import com.google.errorprone.annotations.FormatString;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@@ -569,9 +567,7 @@
private boolean isShellCommand = false;
private boolean useDefaultShellEnvironment = false;
protected boolean executeUnconditionally;
- private PathFragment executable;
- // executableArgs does not include the executable itself.
- private List<String> executableArgs;
+ private CustomCommandLine.Builder executableArgs;
private List<CommandLineAndParamFileInfo> commandLines = new ArrayList<>();
private CharSequence progressMessage;
@@ -598,10 +594,7 @@
this.executionInfo = other.executionInfo;
this.isShellCommand = other.isShellCommand;
this.useDefaultShellEnvironment = other.useDefaultShellEnvironment;
- this.executable = other.executable;
- this.executableArgs = (other.executableArgs != null)
- ? Lists.newArrayList(other.executableArgs)
- : null;
+ this.executableArgs = other.executableArgs;
this.commandLines = new ArrayList<>(other.commandLines);
this.progressMessage = other.progressMessage;
this.mnemonic = other.mnemonic;
@@ -631,10 +624,9 @@
@VisibleForTesting @CheckReturnValue
public Action[] build(ActionOwner owner, BuildConfiguration configuration) {
- ImmutableList<String> executableArgs = buildExecutableArgs();
Action[] actions = new Action[1];
CommandLines.Builder result = CommandLines.builder();
- result.addCommandLine(CommandLine.of(executableArgs));
+ result.addCommandLine(executableArgs.build());
for (CommandLineAndParamFileInfo pair : this.commandLines) {
result.addCommandLine(pair);
}
@@ -653,8 +645,7 @@
@CheckReturnValue
SpawnAction buildForActionTemplate(ActionOwner owner) {
CommandLines.Builder result = CommandLines.builder();
- ImmutableList<String> executableArgs = buildExecutableArgs();
- result.addCommandLine(CommandLine.of(executableArgs));
+ result.addCommandLine(executableArgs.build());
for (CommandLineAndParamFileInfo pair : commandLines) {
result.addCommandLine(pair.commandLine);
}
@@ -749,16 +740,6 @@
extraActionInfoSupplier);
}
- private ImmutableList<String> buildExecutableArgs() {
- Preconditions.checkNotNull(executable);
- Preconditions.checkNotNull(executableArgs);
-
- return ImmutableList.<String>builder()
- .add(executable.getPathString())
- .addAll(executableArgs)
- .build();
- }
-
/**
* Adds an artifact that is necessary for executing the spawn itself (e.g. a compiler), in
* contrast to an artifact that is necessary for the spawn to do its work (e.g. source code).
@@ -934,8 +915,7 @@
* {@link #setShellCommand(String)}.
*/
public Builder setExecutable(PathFragment executable) {
- this.executable = executable;
- this.executableArgs = Lists.newArrayList();
+ this.executableArgs = CustomCommandLine.builder().addPath(executable);
this.isShellCommand = false;
return this;
}
@@ -982,11 +962,12 @@
private Builder setJavaExecutable(PathFragment javaExecutable, Artifact deployJar,
List<String> jvmArgs, String... launchArgs) {
- this.executable = javaExecutable;
- this.executableArgs = Lists.newArrayList();
- executableArgs.add("-Xverify:none");
- executableArgs.addAll(jvmArgs);
- Collections.addAll(executableArgs, launchArgs);
+ this.executableArgs =
+ CustomCommandLine.builder()
+ .addPath(javaExecutable)
+ .add("-Xverify:none")
+ .addAll(ImmutableList.copyOf(jvmArgs))
+ .addAll(ImmutableList.copyOf(launchArgs));
toolsBuilder.add(deployJar);
this.isShellCommand = false;
return this;
@@ -1032,9 +1013,9 @@
* #setExecutable(Artifact)}, {@link #setJavaExecutable}, or {@link #setShellCommand(String)}.
*/
public Builder setShellCommand(PathFragment shExecutable, String command) {
- this.executable = shExecutable;
// 0=shell command switch, 1=command
- this.executableArgs = Lists.newArrayList("-c", command);
+ this.executableArgs =
+ CustomCommandLine.builder().addPath(shExecutable).add("-c").addDynamicString(command);
this.isShellCommand = true;
return this;
}
@@ -1044,9 +1025,7 @@
* commands to be executed.
*/
public Builder setShellCommand(Iterable<String> command) {
- this.executable = PathFragment.create(Iterables.getFirst(command, null));
- // The first item of the commands is the shell executable that should be used.
- this.executableArgs = ImmutableList.copyOf(Iterables.skip(command, 1));
+ this.executableArgs = CustomCommandLine.builder().addAll(ImmutableList.copyOf(command));
this.isShellCommand = true;
return this;
}
@@ -1062,22 +1041,26 @@
return this;
}
- /**
- * Appends the arguments to the list of executable arguments.
- */
+ /** Returns a {@link CustomCommandLine.Builder} for executable arguments. */
+ public CustomCommandLine.Builder executableArguments() {
+ Preconditions.checkState(executableArgs != null);
+ return this.executableArgs;
+ }
+
+ /** Appends the arguments to the list of executable arguments. */
public Builder addExecutableArguments(String... arguments) {
Preconditions.checkState(executableArgs != null);
- Collections.addAll(executableArgs, arguments);
+ this.executableArgs.addAll(ImmutableList.copyOf(arguments));
return this;
}
/**
- * Add multiple arguments in the order they are returned by the collection
- * to the list of executable arguments.
+ * Add multiple arguments in the order they are returned by the collection to the list of
+ * executable arguments.
*/
public Builder addExecutableArguments(Iterable<String> arguments) {
Preconditions.checkState(executableArgs != null);
- Iterables.addAll(executableArgs, arguments);
+ this.executableArgs.addAll(ImmutableList.copyOf(arguments));
return this;
}
