commit 0bd89dc476357fe411fed5cf46772249869ebdd5
author janakr <janakr@google.com> Tue Aug 28 09:53:01 2018 -0700
committer Copybara-Service <copybara-piper@google.com> Tue Aug 28 09:55:39 2018 -0700
tree ce98657ff1629733ba9125c8c9328ade4a1bf001
parent 0ec9d783909e2131d228a95dc3d2591c59487fb7

Don't use SkyFunction.Environment#registerDependencies optimization when using MAX_CHILD_VERSIONS evaluation behavior: the versions must be known to avoid over-evaluating.

PiperOrigin-RevId: 210557138

src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java

diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
index fc808b0..9b99377 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
@@ -17,6 +17,7 @@
 import com.google.common.base.Predicates;
 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.common.collect.Sets;
@@ -103,9 +104,10 @@
   private final Map<SkyKey, SkyValue> newlyRequestedDepsValues = new HashMap<>();
 
   /**
-   * Keys of dependencies registered via {@link #registerDependencies}.
+   * Keys of dependencies registered via {@link #registerDependencies} if not using {@link
+   * EvaluationVersionBehavior#MAX_CHILD_VERSIONS}.
    *
-   * <p>The {@link #registerDependencies} method is hacky. Deps registered through it will not have
+   * <p>The {@link #registerDependencies} method is hacky. Deps registered through it may not have
    * entries in {@link #newlyRequestedDepsValues}, but they are expected to be done. This set tracks
    * those keys so that they aren't removed when {@link #removeUndoneNewlyRequestedDeps} is called.
    */
@@ -814,15 +816,28 @@
   }
 
   @Override
-  public void registerDependencies(Iterable<SkyKey> keys) {
+  public void registerDependencies(Iterable<SkyKey> keys) throws InterruptedException {
+    if (EvaluationVersionBehavior.MAX_CHILD_VERSIONS.equals(
+        evaluatorContext.getEvaluationVersionBehavior())) {
+      // Need versions when doing MAX_CHILD_VERSIONS, so can't use optimization. To use the
+      // optimization, the caller would have to know the versions of the passed-in keys. Extensions
+      // of the SkyFunction.Environment interface to make that possible could happen.
+      Map<SkyKey, SkyValue> checkSizeMap = getValues(keys);
+      ImmutableSet<SkyKey> keysSet = ImmutableSet.copyOf(keys);
+      if (checkSizeMap.size() != keysSet.size()) {
+        throw new IllegalStateException(
+            "Missing keys when checking dependencies for "
+                + skyKey
+                + ": "
+                + Sets.difference(keysSet, checkSizeMap.keySet()));
+      }
+      return;
+    }
     newlyRequestedDeps.startGroup();
     for (SkyKey key : keys) {
       if (!previouslyRequestedDepsValues.containsKey(key)) {
         newlyRequestedDeps.add(key);
         newlyRegisteredDeps.add(key);
-        // Be conservative with these value-not-retrieved deps: assume they have the highest
-        // possible version.
-        maxChildVersion = evaluatorContext.getGraphVersion();
       }
     }
     newlyRequestedDeps.endGroup();