Improve CustomCommandLine.

Instead of having custom ArgvFragments for every combination of desired things, we make a combined "interpreter" of argvs. This saves memory and simplifies things as we do not have to allocate a strategy instance per call to args (instead pushing a single shared instance, followed by the args).

The generic interpreter does have a lot of branching compared to the bespoke implementations, but because the branch is always the same for long stretches the branch predictor should easily be able to handle it with minimal overhead (~1 cycle per branch IIRC).

This CL also elevates that we either want a NestedSet or an ImmutableCollection to the surface of the API, so consumers understand the cost when they call it with a non-immutable collection. Most of the changes in clients is due to this.

To cut down on CL churn, @Deprecated forwarding methods are added to CustomCommandLine. These will be removed in a separate CL using IDE inlining.

RELNOTES: None
PiperOrigin-RevId: 164725370

1 files changed, 7 insertions, 6 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
index 61e99ec6d9..1c17394a30 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
@@ -461,7 +461,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
         j2objcSourceJarTranslatedHeaderFiles(ruleContext));
   }
 
-  private static List<String> sourceJarFlags(RuleContext ruleContext) {
+  private static ImmutableList<String> sourceJarFlags(RuleContext ruleContext) {
     return ImmutableList.of(
         "--output_gen_source_dir",
         j2ObjcSourceJarTranslatedSourceFiles(ruleContext).getExecPathString(),
@@ -492,14 +492,14 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
     ImmutableList.Builder<Artifact> sourceJarOutputFiles = ImmutableList.builder();
     if (!Iterables.isEmpty(sourceJars)) {
       sourceJarOutputFiles.addAll(sourceJarOutputs(ruleContext));
-      argBuilder.addJoinExecPaths("--src_jars", ",", sourceJars);
+      argBuilder.addJoinExecPaths("--src_jars", ",", ImmutableList.copyOf(sourceJars));
       argBuilder.add(sourceJarFlags(ruleContext));
     }
 
     Iterable<String> translationFlags = ruleContext
         .getFragment(J2ObjcConfiguration.class)
         .getTranslationFlags();
-    argBuilder.add(translationFlags);
+    argBuilder.add(ImmutableList.copyOf(translationFlags));
 
     NestedSet<Artifact> depsHeaderMappingFiles =
         depJ2ObjcMappingFileProvider.getHeaderMappingFiles();
@@ -541,7 +541,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
       argBuilder.addJoinExecPaths("-classpath", ":", compileTimeJars);
     }
 
-    argBuilder.addExecPaths(sources);
+    argBuilder.addExecPaths(ImmutableList.copyOf(sources));
 
     Artifact paramFile = j2ObjcOutputParamFile(ruleContext);
     ruleContext.registerAction(new ParameterFileWriteAction(
@@ -584,10 +584,11 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
     if (experimentalJ2ObjcHeaderMap) {
       CustomCommandLine.Builder headerMapCommandLine = CustomCommandLine.builder();
       if (!Iterables.isEmpty(sources)) {
-        headerMapCommandLine.addJoinExecPaths("--source_files", ",", sources);
+        headerMapCommandLine.addJoinExecPaths("--source_files", ",", ImmutableList.copyOf(sources));
       }
       if (!Iterables.isEmpty(sourceJars)) {
-        headerMapCommandLine.addJoinExecPaths("--source_jars", ",", sourceJars);
+        headerMapCommandLine.addJoinExecPaths(
+            "--source_jars", ",", ImmutableList.copyOf(sourceJars));
       }
       headerMapCommandLine.addExecPath("--output_mapping_file", outputHeaderMappingFile);
       ruleContext.registerAction(new SpawnAction.Builder()