Add PrepareDepsOfPatternsFunction, use before query

This introduces a single SkyFunction that has the desired side effect
of loading matching targets and their transitive dependencies in the
graph.

It replaces the two calls to buildDriver.evaluate that made sure the
graph loaded the necessary values before query evaluation with just one
call.

--
MOS_MIGRATED_REVID=89864338

4 files changed, 220 insertions, 25 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java
new file mode 100644
index 0000000000..fe90910c63
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java
@@ -0,0 +1,120 @@
+// Copyright 2015 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Predicates;
+import com.google.devtools.build.lib.cmdline.ResolvedTargets;
+import com.google.devtools.build.lib.cmdline.TargetParsingException;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.packages.NoSuchPackageException;
+import com.google.devtools.build.lib.packages.NoSuchTargetException;
+import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.pkgcache.ParseFailureListener;
+import com.google.devtools.build.lib.skyframe.PrepareDepsOfPatternsValue.TargetPatternSequence;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+import com.google.devtools.build.skyframe.ValueOrException;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+import javax.annotation.Nullable;
+
+/**
+ * PrepareDepsOfPatternsFunction ensures the graph loads targets matching the pattern sequence and
+ * their transitive dependencies.
+ */
+public class PrepareDepsOfPatternsFunction implements SkyFunction {
+
+  /**
+   * Given a {@link SkyKey} that contains a sequence of target patterns, when this function returns
+   * {@link PrepareDepsOfPatternsValue}, then all targets matching that sequence, and those targets'
+   * transitive dependencies, have been loaded.
+   */
+  @Nullable
+  @Override
+  public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
+    TargetPatternSequence targetPatternSequence = (TargetPatternSequence) skyKey.argument();
+    Iterable<SkyKey> patternSkyKeys = TargetPatternValue.keys(targetPatternSequence.getPatterns(),
+        targetPatternSequence.getPolicy(), targetPatternSequence.getOffset());
+    Map<SkyKey, ValueOrException<TargetParsingException>> targetPatternValuesByKey =
+        env.getValuesOrThrow(patternSkyKeys, TargetParsingException.class);
+    if (env.valuesMissing()) {
+      return null;
+    }
+
+    EventHandler eventHandler = env.getListener();
+    boolean handlerIsParseFailureListener = eventHandler instanceof ParseFailureListener;
+
+    ResolvedTargets.Builder<Target> builder = ResolvedTargets.builder();
+    for (SkyKey key : patternSkyKeys) {
+      try {
+        // The only exception type throwable by TargetPatternFunction is TargetParsingException.
+        // Therefore all ValueOrException values in the map will either be non-null or throw
+        // TargetParsingException when get is called.
+        TargetPatternValue resultValue = Preconditions.checkNotNull(
+            (TargetPatternValue) targetPatternValuesByKey.get(key).get());
+        ResolvedTargets<Target> results = resultValue.getTargets();
+        if (((TargetPatternValue.TargetPattern) key.argument()).isNegative()) {
+          builder.filter(Predicates.not(Predicates.in(results.getTargets())));
+        } else {
+          builder.merge(results);
+        }
+      } catch (TargetParsingException e) {
+        // If a target pattern can't be evaluated, notify the user of the problem and keep going.
+        handleTargetParsingException(eventHandler, handlerIsParseFailureListener, key, e);
+      }
+    }
+    ResolvedTargets<Target> resolvedTargets = builder.build();
+
+    List<SkyKey> targetKeys = new ArrayList<>();
+    for (Target target : resolvedTargets.getTargets()) {
+      targetKeys.add(TransitiveTargetValue.key(target.getLabel()));
+    }
+
+    // TransitiveTargetFunction can produce exceptions of types NoSuchPackageException and
+    // NoSuchTargetException. However, PrepareDepsOfPatternsFunction doesn't care about any errors
+    // found during transitive target loading--it just wants the graph to have loaded whatever
+    // transitive target values are available.
+    env.getValuesOrThrow(targetKeys, NoSuchPackageException.class, NoSuchTargetException.class);
+    if (env.valuesMissing()) {
+      return null;
+    }
+
+    return PrepareDepsOfPatternsValue.INSTANCE;
+  }
+
+  private static void handleTargetParsingException(EventHandler eventHandler,
+      boolean handlerIsParseFailureListener, SkyKey key, TargetParsingException e) {
+    TargetPatternValue.TargetPattern pattern = (TargetPatternValue.TargetPattern) key.argument();
+    String rawPattern = pattern.getPattern();
+    String errorMessage = e.getMessage();
+    eventHandler.handle(Event.error("Skipping '" + rawPattern + "': " + errorMessage));
+    if (handlerIsParseFailureListener) {
+      ParseFailureListener parseListener = (ParseFailureListener) eventHandler;
+      parseListener.parsingError(rawPattern,  errorMessage);
+    }
+  }
+
+  @Nullable
+  @Override
+  public String extractTag(SkyKey skyKey) {
+    return null;
+  }
+
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsValue.java
new file mode 100644
index 0000000000..88f034cee2
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsValue.java
@@ -0,0 +1,90 @@
+// Copyright 2015 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+
+import java.io.Serializable;
+import java.util.Objects;
+
+/**
+ * The value returned by {@link PrepareDepsOfPatternsFunction}. Because that function is
+ * invoked only for its side effect (i.e. ensuring the graph contains targets matching the
+ * pattern sequence and their transitive dependencies), this value carries no information.
+ */
+@Immutable
+@ThreadSafe
+public final class PrepareDepsOfPatternsValue implements SkyValue {
+  public static final PrepareDepsOfPatternsValue INSTANCE = new PrepareDepsOfPatternsValue();
+
+  private PrepareDepsOfPatternsValue() {
+  }
+
+  @ThreadSafe
+  public static SkyKey key(ImmutableList<String> patterns, FilteringPolicy policy,
+      String offset) {
+    return new SkyKey(SkyFunctions.PREPARE_DEPS_OF_PATTERNS,
+        new TargetPatternSequence(patterns, policy, offset));
+  }
+
+  /** The argument value for SkyKeys of {@link PrepareDepsOfPatternsFunction}. */
+  @ThreadSafe
+  public static class TargetPatternSequence implements Serializable {
+    private final ImmutableList<String> patterns;
+    private final FilteringPolicy policy;
+    private final String offset;
+
+    public TargetPatternSequence(ImmutableList<String> patterns,
+        FilteringPolicy policy, String offset) {
+      this.patterns = patterns;
+      this.policy = policy;
+      this.offset = offset;
+    }
+
+    public ImmutableList<String> getPatterns() {
+      return patterns;
+    }
+
+    public FilteringPolicy getPolicy() {
+      return policy;
+    }
+
+    public String getOffset() {
+      return offset;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) {
+        return true;
+      }
+      if (!(o instanceof TargetPatternSequence)) {
+        return false;
+      }
+      TargetPatternSequence that = (TargetPatternSequence) o;
+      return offset.equals(that.offset) && patterns.equals(that.patterns)
+          && policy.equals(that.policy);
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(patterns, policy, offset);
+    }
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
index 316d27d86f..ba0ea07e84 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
@@ -40,6 +40,8 @@ public final class SkyFunctions {
   public static final SkyFunctionName PACKAGE = SkyFunctionName.computed("PACKAGE");
   public static final SkyFunctionName TARGET_MARKER = SkyFunctionName.computed("TARGET_MARKER");
   public static final SkyFunctionName TARGET_PATTERN = SkyFunctionName.computed("TARGET_PATTERN");
+  public static final SkyFunctionName PREPARE_DEPS_OF_PATTERNS =
+      SkyFunctionName.computed("PREPARE_DEPS_OF_PATTERNS");
   public static final SkyFunctionName RECURSIVE_PKG = SkyFunctionName.computed("RECURSIVE_PKG");
   public static final SkyFunctionName TRANSITIVE_TARGET =
       SkyFunctionName.computed("TRANSITIVE_TARGET");
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index b8a6ba1523..26728347b0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -58,8 +58,6 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFactory;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
-import com.google.devtools.build.lib.cmdline.ResolvedTargets;
-import com.google.devtools.build.lib.cmdline.TargetParsingException;
 import com.google.devtools.build.lib.concurrent.ThreadSafety;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
 import com.google.devtools.build.lib.events.EventHandler;
@@ -298,6 +296,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
         pkgFactory.getRuleClassProvider(), pkgFactory));
     map.put(SkyFunctions.GLOB, new GlobFunction());
     map.put(SkyFunctions.TARGET_PATTERN, new TargetPatternFunction(pkgLocator));
+    map.put(SkyFunctions.PREPARE_DEPS_OF_PATTERNS, new PrepareDepsOfPatternsFunction());
     map.put(SkyFunctions.RECURSIVE_PKG, new RecursivePkgFunction());
     map.put(SkyFunctions.PACKAGE, new PackageFunction(
         reporter, pkgFactory, packageManager, showLoadingProgress, packageFunctionCache,
@@ -1176,12 +1175,12 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   }
 
   /**
-   * For internal use in queries: performs two graph updates to make sure the transitive closure of
+   * For internal use in queries: performs a graph update to make sure the transitive closure of
    * the specified target {@code patterns} is present in the graph, and returns a traversable view
    * of the graph.
    *
-   * <p>The graph updates here are unconditionally done in keep-going mode, so that the query is
-   * guaranteed a complete graph to work on.
+   * <p>The graph update is unconditionally done in keep-going mode, so that the query is guaranteed
+   * a complete graph to work on.
    */
   @Override
   public WalkableGraph prepareAndGet(Collection<String> patterns, int numThreads,
@@ -1190,26 +1189,10 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
         (SkyframeTargetPatternEvaluator) packageManager.getTargetPatternEvaluator();
     String offset = patternEvaluator.getOffset();
     FilteringPolicy policy = TargetPatternEvaluator.DEFAULT_FILTERING_POLICY;
-    Iterable<SkyKey> patternSkyKeys = TargetPatternValue.keys(patterns, policy, offset);
-    ResolvedTargets<Target> result;
-    try {
-      result = patternEvaluator.parseTargetPatternKeys(patternSkyKeys, /*numThreads=*/numThreads,
-        /*keepGoing=*/true, eventHandler);
-    } catch (TargetParsingException e) {
-      // Can't happen, since we ran with keepGoing.
-      throw new IllegalStateException(e);
-    }
-    List<SkyKey> targetKeys = new ArrayList<>();
-    for (Target target : result.getTargets()) {
-      targetKeys.add(TransitiveTargetValue.key(target.getLabel()));
-    }
-    // We request all the keys here, even the ones that were already evaluated, because we want a
-    // single graph that contains all these keys, and if the evaluator keys graphs based on
-    // top-level keys, we must request the union of all our desired keys in a single evaluate call.
-    Iterable<SkyKey> allKeys = ImmutableList.copyOf(Iterables.concat(patternSkyKeys, targetKeys));
-    return Preconditions.checkNotNull(
-        buildDriver.evaluate(allKeys, true, numThreads, eventHandler).getWalkableGraph(),
-        patterns);
+    SkyKey skyKey = PrepareDepsOfPatternsValue.key(ImmutableList.copyOf(patterns), policy, offset);
+    EvaluationResult<SkyValue> evaluationResult =
+        buildDriver.evaluate(ImmutableList.of(skyKey), true, numThreads, eventHandler);
+    return Preconditions.checkNotNull(evaluationResult.getWalkableGraph(), patterns);
   }
 
   /**