Avoid expansion of NestedSets in MiddlemanFactory.

MiddlemanFactory wants to check if the middleman inputs set is empty
or singleton. A NestedSet can answer both queries efficiently if the
right APIs are used.

My ultimate goal here is to avoid the expansion of runfiles artifacts
nested sets until execution.

Change-Id: I29a269df757ef41b1410bbb492cf24c926df6114
PiperOrigin-RevId: 178600943

4 files changed, 22 insertions, 7 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java
index 5093344464..fbbc10016f 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java
@@ -17,6 +17,8 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.collect.CollectionUtils;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -96,6 +98,9 @@ public final class MiddlemanFactory {
   }
 
   private <T> boolean hasExactlyOneInput(Iterable<T> iterable) {
+    if (iterable instanceof NestedSet) {
+      return ((NestedSet) iterable).isSingleton();
+    }
     Iterator<T> it = iterable.iterator();
     if (!it.hasNext()) {
       return false;
@@ -150,7 +155,7 @@ public final class MiddlemanFactory {
   private Pair<Artifact, Action> createMiddleman(
       ActionOwner owner, String middlemanName, String purpose, Iterable<Artifact> inputs,
       Root middlemanDir, MiddlemanType middlemanType) {
-    if (inputs == null || Iterables.isEmpty(inputs)) {
+    if (inputs == null || CollectionUtils.isEmpty(inputs)) {
       return null;
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
index acc30fa783..84bb2e63ff 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
@@ -25,6 +25,7 @@ import com.google.devtools.build.lib.analysis.actions.SymlinkTreeAction;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.RunUnder;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.syntax.Type;
@@ -264,8 +265,8 @@ public final class RunfilesSupport {
     return sourcesManifest;
   }
 
-  private Artifact createArtifactsMiddleman(ActionConstructionContext context,
-      Iterable<Artifact> allRunfilesArtifacts) {
+  private Artifact createArtifactsMiddleman(
+      ActionConstructionContext context, NestedSet<Artifact> allRunfilesArtifacts) {
     return context.getAnalysisEnvironment().getMiddlemanFactory().createRunfilesMiddleman(
         context.getActionOwner(), owningExecutable, allRunfilesArtifacts,
         context.getMiddlemanDirectory(),
diff --git a/src/main/java/com/google/devtools/build/lib/collect/CollectionUtils.java b/src/main/java/com/google/devtools/build/lib/collect/CollectionUtils.java
index 5c2f92e0fe..369ef52274 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/CollectionUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/collect/CollectionUtils.java
@@ -20,6 +20,7 @@ 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.Iterables;
 import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import java.util.ArrayList;
@@ -199,4 +200,14 @@ public final class CollectionUtils {
       Map<KEY_1, ? extends Map<KEY_2, VALUE>> map) {
     return new HashMap<>(Maps.transformValues(map, HashMap::new));
   }
+
+  /**
+   * A variant of {@link com.google.common.collect.Iterables.isEmpty} that avoids expanding nested
+   * sets.
+   */
+  public static <T> boolean isEmpty(Iterable<T> iterable) {
+    return (iterable instanceof NestedSet)
+        ? ((NestedSet) iterable).isEmpty()
+        : Iterables.isEmpty(iterable);
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java
index 30d580580f..062d24d67c 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java
+++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java
@@ -166,10 +166,8 @@ public final class NestedSet<E> implements Iterable<E> {
     return children == EMPTY_CHILDREN;
   }
 
-  /**
-   * Returns true if the set has exactly one element.
-   */
-  private boolean isSingleton() {
+  /** Returns true if the set has exactly one element. */
+  public boolean isSingleton() {
     return !(children instanceof Object[]);
   }