Do not expand nested sets in resource merging action construction.

PiperOrigin-RevId: 155900259

2 files changed, 55 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
index d314388fe4..e6f9792637 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.analysis.actions;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -28,6 +29,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -282,6 +284,34 @@ public final class CustomCommandLine extends CommandLine {
     }
   }
 
+  private static final class JoinValuesTransformed<T> extends ArgvFragment {
+
+    private final String delimiter;
+    private final Iterable<T> values;
+    private final Function<T, String> toString;
+
+    private JoinValuesTransformed(
+        String delimiter, Iterable<T> values, Function<T, String> toString) {
+      this.delimiter = delimiter;
+      this.values = CollectionUtils.makeImmutable(values);
+      this.toString = toString;
+    }
+
+    @Override
+    void eval(ImmutableList.Builder<String> builder) {
+      StringBuilder arg = new StringBuilder();
+      Iterator<T> parts = values.iterator();
+      if (parts.hasNext()) {
+        arg.append(toString.apply(parts.next()));
+        while (parts.hasNext()) {
+          arg.append(delimiter);
+          arg.append(toString.apply(parts.next()));
+        }
+      }
+      builder.add(arg.toString());
+    }
+  }
+
   /**
    * Arguments that intersperse strings between the items in a sequence. There are two forms of
    * interspersing, and either may be used by this implementation:
@@ -480,6 +510,27 @@ public final class CustomCommandLine extends CommandLine {
       }
       return this;
     }
+
+    /**
+     * Adds a list of values transformed by a function and delimited by a string.
+     *
+     * <p>Prefer this to transforming nested sets yourself as it is more memory-efficient. By using
+     * this class, expansion of the nested set is deferred until action execution instead of
+     * retained on the heap.
+     *
+     * @param arg The argument
+     * @param delimiter A delimiter string placed in between each transformed value
+     * @param values The values to expand into a list
+     * @param toString A function that transforms a value into a string
+     */
+    public <T> Builder addJoinValues(
+        String arg, String delimiter, Iterable<T> values, Function<T, String> toString) {
+      if (arg != null && arguments != null) {
+        arguments.add(arg);
+        arguments.add(new JoinValuesTransformed<T>(delimiter, values, toString));
+      }
+      return this;
+    }
  
     public Builder addJoinExecPaths(String arg, String delimiter, Iterable<Artifact> artifacts) {
       if (arg != null && artifacts != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainerConverter.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainerConverter.java
index 028c6321a1..a78344077a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainerConverter.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainerConverter.java
@@ -20,7 +20,6 @@ import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Iterators;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
@@ -216,25 +215,13 @@ public class ResourceContainerConverter {
       ToArtifacts toArtifacts) {
 
     if (dependencies != null) {
-      // TODO(bazel-team): Find an appropriately lazy method to deduplicate the dependencies between
-      // the direct and transitive data.
-      // Add transitive data inside an unmodifiableIterable to ensure it won't be expanded until
-      // iteration.
       if (!dependencies.getTransitiveResources().isEmpty()) {
-        cmdBuilder.addJoinStrings(
-            "--data",
-            toArg.listSeparator(),
-            Iterables.unmodifiableIterable(
-                Iterables.transform(dependencies.getTransitiveResources(), toArg)));
+        cmdBuilder.addJoinValues(
+            "--data", toArg.listSeparator(), dependencies.getTransitiveResources(), toArg);
       }
-      // Add direct data inside an unmodifiableIterable to ensure it won't be expanded until
-      // iteration.
       if (!dependencies.getDirectResources().isEmpty()) {
-        cmdBuilder.addJoinStrings(
-            "--directData",
-            toArg.listSeparator(),
-            Iterables.unmodifiableIterable(
-                Iterables.transform(dependencies.getDirectResources(), toArg)));
+        cmdBuilder.addJoinValues(
+            "--directData", toArg.listSeparator(), dependencies.getDirectResources(), toArg);
       }
       // This flattens the nested set. Since each ResourceContainer needs to be transformed into
       // Artifacts, and the NestedSetBuilder.wrap doesn't support lazy Iterator evaluation