commit 09e22056d9c84bf6e024a705b5d3d3549fd13873
author Benjamin Peterson <bp@benjamin.pe> Mon Dec 11 04:31:37 2017 -0800
committer Copybara-Service <copybara-piper@google.com> Mon Dec 11 04:33:27 2017 -0800
tree 66fc104cd8d473530bf97c788563b05437643a48
parent e7707580aa47b1dea03c2c6af2438219ee8d9bd6

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

src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java

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 5093344..fbbc100 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.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 @@
   }
 
   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 @@
   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;
     }
 

src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java

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 acc30fa..84bb2e6 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.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 @@
     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(),

src/main/java/com/google/devtools/build/lib/collect/CollectionUtils.java

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 5c2f92e..369ef52 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.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 @@
       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);
+  }
 }

src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java

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 30d5805..062d24d 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 @@
     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[]);
   }