Stream TargetPattern#eval implementations' results to a callback rather than returning a ResolvedTargets set.

This is the second step in a series to allow processing large sets of targets in query target patterns via streaming batches rather than all at once. This should also be a functional no-op.

--
MOS_MIGRATED_REVID=111611858

4 files changed, 92 insertions, 44 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
index 60515979ff..6a23a64386 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
@@ -148,7 +148,16 @@ public abstract class TargetPattern implements Serializable {
    */
   public <T> ResolvedTargets<T> eval(TargetPatternResolver<T> resolver)
       throws TargetParsingException, InterruptedException {
-    return eval(resolver, ImmutableSet.<String>of());
+    final Set<T> results = CompactHashSet.create();
+    BatchCallback<T, RuntimeException> callback =
+        new BatchCallback<T, RuntimeException>() {
+          @Override
+          public void process(Iterable<T> partialResult) {
+            Iterables.addAll(results, partialResult);
+          }
+        };
+    eval(resolver, ImmutableSet.<String>of(), callback);
+    return ResolvedTargets.<T>builder().addAll(results).build();
   }
 
   /**
@@ -158,9 +167,11 @@ public abstract class TargetPattern implements Serializable {
    * @throws IllegalArgumentException if {@code excludedSubdirectories} is nonempty and this
    *      pattern does not have type {@code Type.TARGETS_BELOW_DIRECTORY}.
    */
-  public abstract <T> ResolvedTargets<T> eval(TargetPatternResolver<T> resolver,
-      ImmutableSet<String> excludedSubdirectories)
-      throws TargetParsingException, InterruptedException;
+  public abstract <T, E extends Exception> void eval(
+      TargetPatternResolver<T> resolver,
+      ImmutableSet<String> excludedSubdirectories,
+      BatchCallback<T, E> callback)
+      throws TargetParsingException, E, InterruptedException;
 
   /**
    * Returns {@code true} iff this pattern has type {@code Type.TARGETS_BELOW_DIRECTORY} and
@@ -208,13 +219,15 @@ public abstract class TargetPattern implements Serializable {
     }
 
     @Override
-    public <T> ResolvedTargets<T> eval(TargetPatternResolver<T> resolver,
-        ImmutableSet<String> excludedSubdirectories)
-        throws TargetParsingException, InterruptedException {
+    public <T, E extends Exception> void eval(
+        TargetPatternResolver<T> resolver,
+        ImmutableSet<String> excludedSubdirectories,
+        BatchCallback<T, E> callback)
+        throws TargetParsingException, E, InterruptedException {
       Preconditions.checkArgument(excludedSubdirectories.isEmpty(),
           "Target pattern \"%s\" of type %s cannot be evaluated with excluded subdirectories: %s.",
           getOriginalPattern(), getType(), excludedSubdirectories);
-      return resolver.getExplicitTarget(label(targetName));
+      callback.process(resolver.getExplicitTarget(label(targetName)).getTargets());
     }
 
     @Override
@@ -260,30 +273,37 @@ public abstract class TargetPattern implements Serializable {
     }
 
     @Override
-    public <T> ResolvedTargets<T> eval(TargetPatternResolver<T> resolver,
-        ImmutableSet<String> excludedSubdirectories)
-        throws TargetParsingException, InterruptedException {
+    public <T, E extends Exception> void eval(
+        TargetPatternResolver<T> resolver,
+        ImmutableSet<String> excludedSubdirectories,
+        BatchCallback<T, E> callback)
+        throws TargetParsingException, E, InterruptedException {
       Preconditions.checkArgument(excludedSubdirectories.isEmpty(),
           "Target pattern \"%s\" of type %s cannot be evaluated with excluded subdirectories: %s.",
           getOriginalPattern(), getType(), excludedSubdirectories);
       if (resolver.isPackage(PackageIdentifier.createInDefaultRepo(path))) {
         // User has specified a package name. lookout for default target.
-        return resolver.getExplicitTarget(label("//" + path));
-      }
-
-      List<String> pieces = SLASH_SPLITTER.splitToList(path);
-
-      // Interprets the label as a file target.  This loop stops as soon as the
-      // first BUILD file is found (i.e. longest prefix match).
-      for (int i = pieces.size() - 1; i > 0; i--) {
-        String packageName = SLASH_JOINER.join(pieces.subList(0, i));
-        if (resolver.isPackage(PackageIdentifier.createInDefaultRepo(packageName))) {
-          String targetName = SLASH_JOINER.join(pieces.subList(i, pieces.size()));
-          return resolver.getExplicitTarget(label("//" + packageName + ":" + targetName));
+        callback.process(resolver.getExplicitTarget(label("//" + path)).getTargets());
+      } else {
+
+        List<String> pieces = SLASH_SPLITTER.splitToList(path);
+
+        // Interprets the label as a file target.  This loop stops as soon as the
+        // first BUILD file is found (i.e. longest prefix match).
+        for (int i = pieces.size() - 1; i > 0; i--) {
+          String packageName = SLASH_JOINER.join(pieces.subList(0, i));
+          if (resolver.isPackage(PackageIdentifier.createInDefaultRepo(packageName))) {
+            String targetName = SLASH_JOINER.join(pieces.subList(i, pieces.size()));
+            callback.process(
+                resolver
+                    .getExplicitTarget(label("//" + packageName + ":" + targetName))
+                    .getTargets());
+            return;
+          }
         }
-      }
 
-      throw new TargetParsingException("couldn't determine target from filename '" + path + "'");
+        throw new TargetParsingException("couldn't determine target from filename '" + path + "'");
+      }
     }
 
     @Override
@@ -342,20 +362,26 @@ public abstract class TargetPattern implements Serializable {
     }
 
     @Override
-    public <T> ResolvedTargets<T> eval(TargetPatternResolver<T> resolver,
-        ImmutableSet<String> excludedSubdirectories)
-        throws TargetParsingException, InterruptedException {
+    public <T, E extends Exception> void eval(
+        TargetPatternResolver<T> resolver,
+        ImmutableSet<String> excludedSubdirectories,
+        BatchCallback<T, E> callback)
+        throws TargetParsingException, E, InterruptedException {
       Preconditions.checkArgument(excludedSubdirectories.isEmpty(),
           "Target pattern \"%s\" of type %s cannot be evaluated with excluded subdirectories: %s.",
           getOriginalPattern(), getType(), excludedSubdirectories);
       if (checkWildcardConflict) {
         ResolvedTargets<T> targets = getWildcardConflict(resolver);
         if (targets != null) {
-          return targets;
+          callback.process(targets.getTargets());
+          return;
         }
       }
 
-      return resolver.getTargetsInPackage(getOriginalPattern(), packageIdentifier, rulesOnly);
+      callback.process(
+          resolver
+              .getTargetsInPackage(getOriginalPattern(), packageIdentifier, rulesOnly)
+              .getTargets());
     }
 
     @Override
@@ -446,17 +472,11 @@ public abstract class TargetPattern implements Serializable {
     }
 
     @Override
-    public <T> ResolvedTargets<T> eval(TargetPatternResolver<T> resolver,
-        ImmutableSet<String> excludedSubdirectories)
-        throws TargetParsingException, InterruptedException {
-      final Set<T> results = CompactHashSet.create();
-      BatchCallback<T, RuntimeException> callback =
-          new BatchCallback<T, RuntimeException>() {
-            @Override
-            public void process(Iterable<T> partialResult) {
-              Iterables.addAll(results, partialResult);
-            }
-          };
+    public <T, E extends Exception> void eval(
+        TargetPatternResolver<T> resolver,
+        ImmutableSet<String> excludedSubdirectories,
+        BatchCallback<T, E> callback)
+        throws TargetParsingException, E, InterruptedException {
       resolver.findTargetsBeneathDirectory(
           directory.getRepository(),
           getOriginalPattern(),
@@ -464,7 +484,6 @@ public abstract class TargetPattern implements Serializable {
           rulesOnly,
           excludedSubdirectories,
           callback);
-      return ResolvedTargets.<T>builder().addAll(results).build();
     }
 
     @Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
index 5b02ecc041..cc12e3d785 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
@@ -34,6 +34,7 @@ import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
 import com.google.devtools.build.lib.pkgcache.TargetPatternResolverUtil;
 import com.google.devtools.build.lib.skyframe.EnvironmentBackedRecursivePackageProvider.MissingDepException;
 import com.google.devtools.build.lib.util.BatchCallback;
+import com.google.devtools.build.lib.util.BatchCallback.NullCallback;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -81,7 +82,8 @@ public class PrepareDepsOfPatternFunction implements SkyFunction {
       TargetPattern parsedPattern = patternKey.getParsedPattern();
       DepsOfPatternPreparer preparer = new DepsOfPatternPreparer(env, pkgPath.get());
       ImmutableSet<String> excludedSubdirectories = patternKey.getExcludedSubdirectories();
-      parsedPattern.eval(preparer, excludedSubdirectories);
+      parsedPattern.<Void, RuntimeException>eval(
+          preparer, excludedSubdirectories, NullCallback.<Void>instance());
     } catch (TargetParsingException e) {
       throw new PrepareDepsOfPatternFunctionException(e);
     } catch (MissingDepException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
index 8c4963b2e3..127d9b3898 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
@@ -14,18 +14,23 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.ResolvedTargets;
 import com.google.devtools.build.lib.cmdline.TargetParsingException;
 import com.google.devtools.build.lib.cmdline.TargetPattern;
+import com.google.devtools.build.lib.collect.CompactHashSet;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.skyframe.EnvironmentBackedRecursivePackageProvider.MissingDepException;
+import com.google.devtools.build.lib.util.BatchCallback;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 
+import java.util.Set;
+
 import javax.annotation.Nullable;
 
 /**
@@ -51,7 +56,16 @@ public class TargetPatternFunction implements SkyFunction {
               patternKey.getPolicy());
       TargetPattern parsedPattern = patternKey.getParsedPattern();
       ImmutableSet<String> excludedSubdirectories = patternKey.getExcludedSubdirectories();
-      resolvedTargets = parsedPattern.eval(resolver, excludedSubdirectories);
+      final Set<Target> results = CompactHashSet.create();
+      BatchCallback<Target, RuntimeException> callback =
+          new BatchCallback<Target, RuntimeException>() {
+            @Override
+            public void process(Iterable<Target> partialResult) {
+              Iterables.addAll(results, partialResult);
+            }
+          };
+      parsedPattern.eval(resolver, excludedSubdirectories, callback);
+      resolvedTargets = ResolvedTargets.<Target>builder().addAll(results).build();
     } catch (TargetParsingException e) {
       throw new TargetPatternFunctionException(e);
     } catch (MissingDepException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/util/BatchCallback.java b/src/main/java/com/google/devtools/build/lib/util/BatchCallback.java
index 70cc5bff50..bc74b7a161 100644
--- a/src/main/java/com/google/devtools/build/lib/util/BatchCallback.java
+++ b/src/main/java/com/google/devtools/build/lib/util/BatchCallback.java
@@ -29,4 +29,17 @@ public interface BatchCallback<T, E extends Exception> {
    * across calls.
    */
   void process(Iterable<T> partialResult) throws E, InterruptedException;
+
+  /** {@link BatchCallback} that does precisely nothing. */
+  class NullCallback<T> implements BatchCallback<T, RuntimeException> {
+    private static final NullCallback<Object> INSTANCE = new NullCallback<>();
+
+    @Override
+    public void process(Iterable<T> partialResult) {}
+
+    @SuppressWarnings("unchecked")
+    public static <T> NullCallback<T> instance() {
+      return (NullCallback<T>) INSTANCE;
+    }
+  }
 }