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
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; }
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(),
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); + } }
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[]); }