Improve safety of NestedSetFingerprintCache by detecting multiple instances of the same mapFn class.

This code tries to add protection against the user creating new mapFn instances per-rule. This would cause the nested set cache to be computed per-rule instead of shared across rule instances, causing memory bloat and slowdowns.

Since this can only happen in native code, we can get away with detecting this and crashing blaze. I think this is a better choice than silently allowing it / falling back to slow computations.

The user can override this behaviour by inheriting from CommandLineItem.CapturingMapFn, in which case the user is explicitly saying they assume responsibility for the number of instances of the mapFn the application will use.

PiperOrigin-RevId: 184061642

9 files changed, 409 insertions, 168 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 3c43f0874b..270c8a8340 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -910,6 +910,7 @@ java_library(
         "rules/java/proto/GeneratedExtensionRegistryProvider.java",
     ],
     deps = [
+        ":commandline_item",
         "//src/main/java/com/google/devtools/build/lib:build-base",
         "//src/main/java/com/google/devtools/build/lib:build-info",
         "//src/main/java/com/google/devtools/build/lib:events",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/CommandLineItem.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/CommandLineItem.java
index c228226e19..f1418d9c10 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/CommandLineItem.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/CommandLineItem.java
@@ -24,6 +24,43 @@ public interface CommandLineItem {
     String expandToCommandLine(T object);
   }
 
+  /**
+   * Use this map function when parametrizing over a limited set of values.
+   *
+   * <p>The user promises that the number of distinct instances constructed is closer to O(rule
+   * class count) than O(rule count).
+   *
+   * <p>Without this, {@link
+   * com.google.devtools.build.lib.collect.nestedset.NestedSetFingerprintCache} will refuse to cache
+   * your {@link MapFn} computations.
+   */
+  abstract class ParametrizedMapFn<T> implements MapFn<T> {
+    @Override
+    public abstract boolean equals(Object obj);
+
+    @Override
+    public abstract int hashCode();
+
+    /**
+     * This method controls the max number of distinct instances allowed. If the system sees any
+     * more than this, it will throw.
+     *
+     * <p>Override and set this to something low. You want this to represent the small number of
+     * preallocated static instances used in this blaze instance. 3 is an OK number, 100 is a bad
+     * number.
+     */
+    public abstract int maxInstancesAllowed();
+  }
+
+  /**
+   * Use this map function when your map function needs to capture per-rule information.
+   *
+   * <p>Use of this class prevents sharing sub-computations over shared NestedSets, since the map
+   * function is per-target. This will make your action key computations become O(N^2). Please avoid
+   * if possible.
+   */
+  interface CapturingMapFn<T> extends MapFn<T> {}
+
   /** Expands the object into the command line as a string. */
   String expandToCommandLine();
 
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetFingerprintCache.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetFingerprintCache.java
index b48d4a2298..083ef06269 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetFingerprintCache.java
+++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetFingerprintCache.java
@@ -15,9 +15,14 @@
 package com.google.devtools.build.lib.collect.nestedset;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.HashMultiset;
+import com.google.common.collect.Multiset;
 import com.google.devtools.build.lib.analysis.actions.CommandLineItem;
+import com.google.devtools.build.lib.analysis.actions.CommandLineItem.MapFn;
 import com.google.devtools.build.lib.util.Fingerprint;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
 /** Computes fingerprints for nested sets, reusing sub-computations from children. */
@@ -26,7 +31,10 @@ public class NestedSetFingerprintCache {
   private static final int EMPTY_SET_DIGEST = 104_395_303;
 
   /** Memoize the subresults. We have to have one cache per type of command item map function. */
-  private Map<CommandLineItem.MapFn<?>, DigestMap> mapFnToFingerprints = createMap();
+  private Map<CommandLineItem.MapFn<?>, DigestMap> mapFnToDigestMap = createMap();
+
+  private final Set<Class<?>> seenMapFns = new HashSet<>();
+  private final Multiset<Class<?>> seenParametrizedMapFns = HashMultiset.create();
 
   public <T> void addNestedSetToFingerprint(Fingerprint fingerprint, NestedSet<T> nestedSet) {
     addNestedSetToFingerprint(CommandLineItem.MapFn.DEFAULT, fingerprint, nestedSet);
@@ -34,20 +42,32 @@ public class NestedSetFingerprintCache {
 
   public <T> void addNestedSetToFingerprint(
       CommandLineItem.MapFn<? super T> mapFn, Fingerprint fingerprint, NestedSet<T> nestedSet) {
+    if (mapFn instanceof CommandLineItem.CapturingMapFn) {
+      addNestedSetToFingerprintSlow(mapFn, fingerprint, nestedSet);
+      return;
+    }
     // Only top-level nested sets can be empty, so we can bail here
     if (nestedSet.isEmpty()) {
       fingerprint.addInt(EMPTY_SET_DIGEST);
       return;
     }
-    DigestMap digestMap =
-        mapFnToFingerprints.computeIfAbsent(mapFn, k -> new DigestMap(DIGEST_SIZE, 1024));
+    DigestMap digestMap = mapFnToDigestMap.computeIfAbsent(mapFn, this::newDigestMap);
     fingerprint.addInt(nestedSet.getOrder().ordinal());
     Object children = nestedSet.rawChildren();
     addToFingerprint(mapFn, fingerprint, digestMap, children);
   }
 
+  private <T> void addNestedSetToFingerprintSlow(
+      MapFn<? super T> mapFn, Fingerprint fingerprint, NestedSet<T> nestedSet) {
+    for (T object : nestedSet) {
+      fingerprint.addString(mapFn.expandToCommandLine(object));
+    }
+  }
+
   public void clear() {
-    mapFnToFingerprints = createMap();
+    mapFnToDigestMap = createMap();
+    seenMapFns.clear();
+    seenParametrizedMapFns.clear();
   }
 
   @SuppressWarnings("unchecked")
@@ -78,4 +98,29 @@ public class NestedSetFingerprintCache {
   private static Map<CommandLineItem.MapFn<?>, DigestMap> createMap() {
     return new ConcurrentHashMap<>();
   }
+
+  private DigestMap newDigestMap(CommandLineItem.MapFn<?> mapFn) {
+    Class<?> mapFnClass = mapFn.getClass();
+    if (mapFn instanceof CommandLineItem.ParametrizedMapFn) {
+      int occurrences = seenParametrizedMapFns.add(mapFnClass, 1) + 1;
+      if (occurrences > ((CommandLineItem.ParametrizedMapFn) mapFn).maxInstancesAllowed()) {
+        throw new IllegalArgumentException(
+            String.format(
+                "Too many instances of CommandLineItem.ParametrizedMapFn '%s' detected. "
+                    + "Please construct fewer instances or use CommandLineItem.CapturingMapFn.",
+                mapFnClass.getName()));
+      }
+    } else {
+      if (!seenMapFns.add(mapFnClass)) {
+        throw new IllegalArgumentException(
+            String.format(
+                "Illegal mapFn implementation: '%s'. "
+                    + "CommandLineItem.MapFn instances must be singletons. "
+                    + "Please see CommandLineItem.ParametrizedMapFn or "
+                    + "CommandLineItem.CapturingMapFn for alternatives.",
+                mapFnClass.getName()));
+      }
+    }
+    return new DigestMap(DIGEST_SIZE, 1024);
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
index 6dbab850f3..71612341af 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
@@ -25,7 +25,8 @@ import com.google.devtools.build.lib.analysis.actions.ParamFileInfo;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.Builder.SeparatorType;
+import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg;
+import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg.Includes;
 import com.google.devtools.build.lib.util.OS;
 import java.util.ArrayList;
 import java.util.List;
@@ -41,18 +42,18 @@ public class AndroidResourceMergingActionBuilder {
 
   private static final ResourceContainerConverter.ToArg RESOURCE_CONTAINER_TO_ARG =
       ResourceContainerConverter.builder()
-          .includeResourceRoots()
-          .includeLabel()
-          .includeSymbolsBin()
-          .withSeparator(SeparatorType.SEMICOLON_AMPERSAND)
+          .include(Includes.ResourceRoots)
+          .include(Includes.Label)
+          .include(Includes.SymbolsBin)
+          .withSeparator(ToArg.SeparatorType.SEMICOLON_AMPERSAND)
           .toArgConverter();
 
   private static final ResourceContainerConverter.ToArg RESOURCE_CONTAINER_TO_ARG_FOR_COMPILED =
       ResourceContainerConverter.builder()
-          .includeResourceRoots()
-          .includeLabel()
-          .includeCompiledSymbols()
-          .withSeparator(SeparatorType.SEMICOLON_AMPERSAND)
+          .include(Includes.ResourceRoots)
+          .include(Includes.Label)
+          .include(Includes.CompiledSymbols)
+          .withSeparator(ToArg.SeparatorType.SEMICOLON_AMPERSAND)
           .toArgConverter();
 
   private final RuleContext ruleContext;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
index 16ebb20876..2f97a4b2a5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
@@ -27,7 +27,8 @@ import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion;
-import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.Builder.SeparatorType;
+import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg;
+import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg.Includes;
 import com.google.devtools.build.lib.util.OS;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -38,37 +39,37 @@ public class AndroidResourcesProcessorBuilder {
 
   private static final ResourceContainerConverter.ToArg AAPT2_RESOURCE_DEP_TO_ARG =
       ResourceContainerConverter.builder()
-          .includeResourceRoots()
-          .includeManifest()
-          .includeAapt2RTxt()
-          .includeSymbolsBin()
-          .includeCompiledSymbols()
-          .withSeparator(SeparatorType.COLON_COMMA)
+          .include(Includes.ResourceRoots)
+          .include(Includes.Manifest)
+          .include(Includes.Aapt2RTxt)
+          .include(Includes.SymbolsBin)
+          .include(Includes.CompiledSymbols)
+          .withSeparator(ToArg.SeparatorType.COLON_COMMA)
           .toArgConverter();
 
   private static final ResourceContainerConverter.ToArg AAPT2_RESOURCE_DEP_TO_ARG_NO_PARSE =
       ResourceContainerConverter.builder()
-          .includeResourceRoots()
-          .includeManifest()
-          .includeAapt2RTxt()
-          .includeCompiledSymbols()
-          .withSeparator(SeparatorType.COLON_COMMA)
+          .include(Includes.ResourceRoots)
+          .include(Includes.Manifest)
+          .include(Includes.Aapt2RTxt)
+          .include(Includes.CompiledSymbols)
+          .withSeparator(ToArg.SeparatorType.COLON_COMMA)
           .toArgConverter();
 
   private static final ResourceContainerConverter.ToArg RESOURCE_CONTAINER_TO_ARG =
       ResourceContainerConverter.builder()
-          .includeResourceRoots()
-          .includeManifest()
-          .withSeparator(SeparatorType.COLON_COMMA)
+          .include(Includes.ResourceRoots)
+          .include(Includes.Manifest)
+          .withSeparator(ToArg.SeparatorType.COLON_COMMA)
           .toArgConverter();
 
   private static final ResourceContainerConverter.ToArg RESOURCE_DEP_TO_ARG =
       ResourceContainerConverter.builder()
-          .includeResourceRoots()
-          .includeManifest()
-          .includeRTxt()
-          .includeSymbolsBin()
-          .withSeparator(SeparatorType.COLON_COMMA)
+          .include(Includes.ResourceRoots)
+          .include(Includes.Manifest)
+          .include(Includes.RTxt)
+          .include(Includes.SymbolsBin)
+          .withSeparator(ToArg.SeparatorType.COLON_COMMA)
           .toArgConverter();
 
   private ResourceContainer primary;
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 fd9ba71a94..e653fb6015 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
@@ -17,13 +17,18 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Functions;
 import com.google.common.base.Joiner;
+import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterators;
+import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.analysis.actions.CommandLineItem;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
 import com.google.devtools.build.lib.rules.android.ResourceContainer.ResourceType;
+import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg.Includes;
+import java.util.HashSet;
+import java.util.Set;
 
 /**
  * Factory for functions to convert a {@link ResourceContainer} to a commandline argument, or a
@@ -37,152 +42,150 @@ public class ResourceContainerConverter {
     return new Builder();
   }
 
-  interface ToArg extends CommandLineItem.MapFn<ResourceContainer> {
-
-    String listSeparator();
-  }
-
-  static class Builder {
-
-    private boolean includeResourceRoots;
-    private boolean includeLabel;
-    private boolean includeManifest;
-    private boolean includeRTxt;
-    private boolean includeSymbolsBin;
-    private boolean includeCompiledSymbols;
-    private boolean includeStaticLibrary;
-    private boolean includeAapt2RTxt;
-    private SeparatorType separatorType;
-    private Joiner argJoiner;
-    private Function<String, String> escaper = Functions.identity();
+  static class ToArg extends CommandLineItem.ParametrizedMapFn<ResourceContainer> {
+
+    private final Set<Includes> includes;
+    private final SeparatorType separatorType;
+    private final Joiner argJoiner;
+    private final Function<String, String> escaper;
+
+    enum Includes {
+      ResourceRoots,
+      Label,
+      Manifest,
+      RTxt,
+      SymbolsBin,
+      CompiledSymbols,
+      StaticLibrary,
+      Aapt2RTxt
+    }
 
     enum SeparatorType {
       COLON_COMMA,
       SEMICOLON_AMPERSAND
     }
 
-    Builder() {}
+    ToArg(Builder builder) {
+      this.includes = Sets.immutableEnumSet(builder.includes);
+      this.separatorType = builder.separatorType;
 
-    Builder includeAapt2RTxt() {
-      includeAapt2RTxt = true;
-      return this;
+      switch (separatorType) {
+        case COLON_COMMA:
+          argJoiner = Joiner.on(":");
+          // We currently use ":" to separate components of an argument and "," to separate
+          // arguments in a list of arguments. Those characters require escaping if used in a label
+          // (part of the set of allowed characters in a label).
+          if (includes.contains(Includes.Label)) {
+            escaper = (String input) -> input.replace(":", "\\:").replace(",", "\\,");
+          } else {
+            escaper = Functions.identity();
+          }
+          break;
+        case SEMICOLON_AMPERSAND:
+          argJoiner = Joiner.on(";");
+          escaper = Functions.identity();
+          break;
+        default:
+          throw new IllegalStateException("Unknown separator type " + separatorType);
+      }
     }
 
-    Builder includeStaticLibrary() {
-      includeStaticLibrary = true;
-      return this;
+    @Override
+    public String expandToCommandLine(ResourceContainer container) {
+      ImmutableList.Builder<String> cmdPieces = ImmutableList.builder();
+      if (includes.contains(Includes.ResourceRoots)) {
+        cmdPieces.add(convertRoots(container, ResourceType.RESOURCES));
+        cmdPieces.add(convertRoots(container, ResourceType.ASSETS));
+      }
+      if (includes.contains(Includes.Label)) {
+        cmdPieces.add(escaper.apply(container.getLabel().toString()));
+      }
+      if (includes.contains(Includes.Manifest)) {
+        cmdPieces.add(container.getManifest().getExecPathString());
+      }
+      if (includes.contains(Includes.RTxt)) {
+        cmdPieces.add(container.getRTxt() == null ? "" : container.getRTxt().getExecPathString());
+      }
+      if (includes.contains(Includes.Aapt2RTxt)) {
+        cmdPieces.add(
+            container.getAapt2RTxt() == null ? "" : container.getAapt2RTxt().getExecPathString());
+      }
+      if (includes.contains(Includes.StaticLibrary)) {
+        cmdPieces.add(
+            container.getStaticLibrary() == null
+                ? ""
+                : container.getStaticLibrary().getExecPathString());
+      }
+      if (includes.contains(Includes.CompiledSymbols)) {
+        cmdPieces.add(
+            container.getCompiledSymbols() == null
+                ? ""
+                : container.getCompiledSymbols().getExecPathString());
+      }
+      if (includes.contains(Includes.SymbolsBin)) {
+        cmdPieces.add(
+            container.getSymbols() == null ? "" : container.getSymbols().getExecPathString());
+      }
+      return argJoiner.join(cmdPieces.build());
     }
 
-    Builder includeResourceRoots() {
-      includeResourceRoots = true;
-      return this;
+    String listSeparator() {
+      switch (separatorType) {
+        case COLON_COMMA:
+          return ",";
+        case SEMICOLON_AMPERSAND:
+          return "&";
+        default:
+          Preconditions.checkState(false, "Unknown separator type " + separatorType);
+          return null;
+      }
     }
 
-    Builder includeLabel() {
-      includeLabel = true;
-      return this;
+    @Override
+    public int maxInstancesAllowed() {
+      // This is the max number of resource converters we expect to statically
+      // construct for any given blaze instance.
+      // Do not increase recklessly.
+      return 10;
     }
 
-    Builder includeManifest() {
-      includeManifest = true;
-      return this;
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) {
+        return true;
+      }
+      if (o == null || getClass() != o.getClass()) {
+        return false;
+      }
+      ToArg toArg = (ToArg) o;
+      return includes.equals(toArg.includes) && separatorType == toArg.separatorType;
     }
 
-    Builder includeRTxt() {
-      includeRTxt = true;
-      return this;
+    @Override
+    public int hashCode() {
+      return Objects.hashCode(includes, separatorType);
     }
+  }
 
-    Builder includeSymbolsBin() {
-      includeSymbolsBin = true;
-      return this;
-    }
+  static class Builder {
+
+    private final Set<Includes> includes = new HashSet<>();
+    private ToArg.SeparatorType separatorType;
 
-    Builder includeCompiledSymbols() {
-      includeCompiledSymbols = true;
+    Builder() {}
+
+    Builder include(Includes include) {
+      includes.add(include);
       return this;
     }
 
-    Builder withSeparator(SeparatorType type) {
+    Builder withSeparator(ToArg.SeparatorType type) {
       separatorType = type;
       return this;
     }
 
     ToArg toArgConverter() {
-      switch (separatorType) {
-        case COLON_COMMA:
-          argJoiner = Joiner.on(":");
-          // We currently use ":" to separate components of an argument and "," to separate
-          // arguments in a list of arguments. Those characters require escaping if used in a label
-          // (part of the set of allowed characters in a label).
-          if (includeLabel) {
-            escaper = (String input) -> input.replace(":", "\\:").replace(",", "\\,");
-          }
-          break;
-        case SEMICOLON_AMPERSAND:
-          argJoiner = Joiner.on(";");
-          break;
-        default:
-          Preconditions.checkState(false, "Unknown separator type " + separatorType);
-          break;
-      }
-
-      return new ToArg() {
-        @Override
-        public String expandToCommandLine(ResourceContainer container) {
-          ImmutableList.Builder<String> cmdPieces = ImmutableList.builder();
-          if (includeResourceRoots) {
-            cmdPieces.add(convertRoots(container, ResourceType.RESOURCES));
-            cmdPieces.add(convertRoots(container, ResourceType.ASSETS));
-          }
-          if (includeLabel) {
-            cmdPieces.add(escaper.apply(container.getLabel().toString()));
-          }
-          if (includeManifest) {
-            cmdPieces.add(container.getManifest().getExecPathString());
-          }
-          if (includeRTxt) {
-            cmdPieces.add(
-                container.getRTxt() == null ? "" : container.getRTxt().getExecPathString());
-          }
-          if (includeAapt2RTxt) {
-            cmdPieces.add(
-                container.getAapt2RTxt() == null
-                    ? ""
-                    : container.getAapt2RTxt().getExecPathString());
-          }
-          if (includeStaticLibrary) {
-            cmdPieces.add(
-                container.getStaticLibrary() == null
-                    ? ""
-                    : container.getStaticLibrary().getExecPathString());
-          }
-          if (includeCompiledSymbols) {
-            cmdPieces.add(
-                container.getCompiledSymbols() == null
-                    ? ""
-                    : container.getCompiledSymbols().getExecPathString());
-          }
-          if (includeSymbolsBin) {
-            cmdPieces.add(
-                container.getSymbols() == null ? "" : container.getSymbols().getExecPathString());
-          }
-          return argJoiner.join(cmdPieces.build());
-        }
-
-        @Override
-        public String listSeparator() {
-          switch (separatorType) {
-            case COLON_COMMA:
-              return ",";
-            case SEMICOLON_AMPERSAND:
-              return "&";
-            default:
-              Preconditions.checkState(false, "Unknown separator type " + separatorType);
-              return null;
-          }
-        }
-      };
+      return new ToArg(this);
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java
index ea84eeede7..fe0568defc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java
@@ -25,8 +25,8 @@ import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTa
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion;
-import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.Builder.SeparatorType;
 import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg;
+import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg.Includes;
 import com.google.devtools.build.lib.util.OS;
 
 /**
@@ -39,20 +39,20 @@ public class RobolectricResourceSymbolsActionBuilder {
 
   private static final ResourceContainerConverter.ToArg RESOURCE_CONTAINER_TO_ARG =
       ResourceContainerConverter.builder()
-          .includeResourceRoots()
-          .includeManifest()
-          .includeRTxt()
-          .includeSymbolsBin()
-          .withSeparator(SeparatorType.COLON_COMMA)
+          .include(Includes.ResourceRoots)
+          .include(Includes.Manifest)
+          .include(Includes.RTxt)
+          .include(Includes.SymbolsBin)
+          .withSeparator(ToArg.SeparatorType.COLON_COMMA)
           .toArgConverter();
 
   private static final ResourceContainerConverter.ToArg RESOURCE_CONTAINER_TO_AAPT2_ARG =
       ResourceContainerConverter.builder()
-          .includeResourceRoots()
-          .includeManifest()
-          .includeAapt2RTxt()
-          .includeSymbolsBin()
-          .withSeparator(SeparatorType.COLON_COMMA)
+          .include(Includes.ResourceRoots)
+          .include(Includes.Manifest)
+          .include(Includes.Aapt2RTxt)
+          .include(Includes.SymbolsBin)
+          .withSeparator(ToArg.SeparatorType.COLON_COMMA)
           .toArgConverter();
 
   private Artifact classJarOut;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java
index 85759c25b4..29470d125f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.rules.java;
 
 import static java.util.Objects.requireNonNull;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -22,6 +23,7 @@ import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.actions.CommandLine;
+import com.google.devtools.build.lib.analysis.actions.CommandLineItem;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
 import com.google.devtools.build.lib.analysis.actions.ParamFileInfo;
@@ -160,16 +162,50 @@ public final class SingleJarActionBuilder {
     args.addExecPaths("--sources", resourceJars);
     if (!resources.isEmpty()) {
       args.add("--resources");
-      args.addAll(VectorArg.of(resources).mapped(resource -> getResourceArg(semantics, resource)));
+      args.addAll(VectorArg.of(resources).mapped(new ResourceArgMapFn(semantics)));
     }
     return args.build();
   }
 
-  private static String getResourceArg(JavaSemantics semantics, Artifact resource) {
-    return String.format(
-        "%s:%s",
-        resource.getExecPathString(),
-        semantics.getDefaultJavaResourcePath(resource.getRootRelativePath()));
+  private static class ResourceArgMapFn extends CommandLineItem.ParametrizedMapFn<Artifact> {
+    private final JavaSemantics semantics;
+
+    ResourceArgMapFn(JavaSemantics semantics) {
+      this.semantics = Preconditions.checkNotNull(semantics);
+    }
+
+    @Override
+    public String expandToCommandLine(Artifact resource) {
+      String execPath = resource.getExecPathString();
+      String resourcePath =
+          semantics.getDefaultJavaResourcePath(resource.getRootRelativePath()).getPathString();
+      StringBuilder sb = new StringBuilder(execPath.length() + resourcePath.length() + 1);
+      sb.append(execPath).append(":").append(resourcePath);
+      return sb.toString();
+    }
+
+    @Override
+    public int maxInstancesAllowed() {
+      // Expect only one semantics object.
+      return 1;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) {
+        return true;
+      }
+      if (o == null || getClass() != o.getClass()) {
+        return false;
+      }
+      ResourceArgMapFn that = (ResourceArgMapFn) o;
+      return semantics.equals(that.semantics);
+    }
+
+    @Override
+    public int hashCode() {
+      return semantics.hashCode();
+    }
   }
 }
 
diff --git a/src/test/java/com/google/devtools/build/lib/collect/nestedset/NestedSetFingerprintCacheTest.java b/src/test/java/com/google/devtools/build/lib/collect/nestedset/NestedSetFingerprintCacheTest.java
index 975f97c972..99bc4bbd9c 100644
--- a/src/test/java/com/google/devtools/build/lib/collect/nestedset/NestedSetFingerprintCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/collect/nestedset/NestedSetFingerprintCacheTest.java
@@ -15,10 +15,13 @@ package com.google.devtools.build.lib.collect.nestedset;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.common.base.Objects;
 import com.google.common.collect.HashMultiset;
 import com.google.common.collect.Multiset;
 import com.google.devtools.build.lib.analysis.actions.CommandLineItem;
+import com.google.devtools.build.lib.analysis.actions.CommandLineItem.CapturingMapFn;
 import com.google.devtools.build.lib.analysis.actions.CommandLineItem.MapFn;
+import com.google.devtools.build.lib.testutil.MoreAsserts;
 import com.google.devtools.build.lib.util.Fingerprint;
 import org.junit.Before;
 import org.junit.Test;
@@ -112,4 +115,118 @@ public class NestedSetFingerprintCacheTest {
       assertThat(entry.getCount()).isEqualTo(2);
     }
   }
+
+  @Test
+  public void testMultipleInstancesOfMapFnThrows() {
+    NestedSet<String> nestedSet =
+        NestedSetBuilder.<String>stableOrder().add("a0").add("a1").build();
+
+    // Make sure a normal method reference doesn't get blacklisted.
+    for (int i = 0; i < 2; ++i) {
+      cache.addNestedSetToFingerprint(
+          NestedSetFingerprintCacheTest::simpleMapFn, new Fingerprint(), nestedSet);
+    }
+
+    // Try again to make sure Java synthesizes a new class for a second method reference.
+    for (int i = 0; i < 2; ++i) {
+      cache.addNestedSetToFingerprint(
+          NestedSetFingerprintCacheTest::simpleMapFn2, new Fingerprint(), nestedSet);
+    }
+
+    // Make sure a non-capturing lambda doesn't get blacklisted
+    for (int i = 0; i < 2; ++i) {
+      cache.addNestedSetToFingerprint(s -> s + "_mapped", new Fingerprint(), nestedSet);
+    }
+
+    // Make sure a CapturingMapFn doesn't get blacklisted
+    for (int i = 0; i < 2; ++i) {
+      cache.addNestedSetToFingerprint(
+          (CapturingMapFn<String>) object -> object, new Fingerprint(), nestedSet);
+    }
+
+    // Make sure a ParametrizedMapFn doesn't get blacklisted until it exceeds its instance count
+    cache.addNestedSetToFingerprint(new IntParametrizedMapFn(1), new Fingerprint(), nestedSet);
+    cache.addNestedSetToFingerprint(new IntParametrizedMapFn(2), new Fingerprint(), nestedSet);
+    MoreAsserts.expectThrows(
+        IllegalArgumentException.class,
+        () ->
+            cache.addNestedSetToFingerprint(
+                new IntParametrizedMapFn(3), new Fingerprint(), nestedSet));
+
+    // Make sure a capturing method reference gets blacklisted
+    MoreAsserts.expectThrows(
+        IllegalArgumentException.class,
+        () -> {
+          for (int i = 0; i < 2; ++i) {
+            StringJoiner str = new StringJoiner("hello");
+            cache.addNestedSetToFingerprint(str::join, new Fingerprint(), nestedSet);
+          }
+        });
+
+    // Do make sure that a capturing lambda gets blacklisted
+    MoreAsserts.expectThrows(
+        IllegalArgumentException.class,
+        () -> {
+          for (int i = 0; i < 2; ++i) {
+            final int capturedVariable = i;
+            cache.addNestedSetToFingerprint(
+                s -> s + capturedVariable, new Fingerprint(), nestedSet);
+          }
+        });
+  }
+
+  private static class IntParametrizedMapFn extends CommandLineItem.ParametrizedMapFn<String> {
+    private final int i;
+
+    private IntParametrizedMapFn(int i) {
+      this.i = i;
+    }
+
+    @Override
+    public String expandToCommandLine(String object) {
+      return object + i;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) {
+        return true;
+      }
+      if (o == null || getClass() != o.getClass()) {
+        return false;
+      }
+      IntParametrizedMapFn that = (IntParametrizedMapFn) o;
+      return i == that.i;
+    }
+
+    @Override
+    public int maxInstancesAllowed() {
+      return 2;
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hashCode(i);
+    }
+  }
+
+  private static class StringJoiner {
+    private final String str;
+
+    private StringJoiner(String str) {
+      this.str = str;
+    }
+
+    private String join(String other) {
+      return str + other;
+    }
+  }
+
+  private static String simpleMapFn(String o) {
+    return o + "_mapped";
+  }
+
+  private static String simpleMapFn2(String o) {
+    return o + "_mapped2";
+  }
 }