Model the TopLevelArtifactContext as an argument to the CompletionFunction rather than a PRECOMPUTED value.

Having a stale TopLevelArtifactContext leads to invalidation of all the top level target nodes, causing time wasted due to a lot of cache hits for a null build.

--
MOS_MIGRATED_REVID=127585059

1 files changed, 51 insertions, 5 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
index f19a572f98..1f48c84f3b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
@@ -28,7 +28,9 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.skyframe.AspectCompletionValue.AspectCompletionKey;
 import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
+import com.google.devtools.build.lib.skyframe.TargetCompletionValue.TargetCompletionKey;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -57,7 +59,21 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
     TValue getValueFromSkyKey(SkyKey skyKey, Environment env);
 
     /**
-     * Returns all artefacts that need to be built to complete the {@code value}
+     * Returns the options which determine the artifacts to build for the top-level targets.
+     * <p>
+     * For the Top level targets we made a conscious decision to include the TopLevelArtifactContext
+     * within the SkyKey as an argument to the CompletionFunction rather than a separate SkyKey.
+     * As a result we do have <num top level targets> extra SkyKeys for every unique
+     * TopLevelArtifactContexts used over the lifetime of Blaze. This is a minor tradeoff,
+     * since it significantly improves null build times when we're switching the
+     * TopLevelArtifactContexts frequently (common for IDEs), by reusing existing SkyKeys
+     * from earlier runs, instead of causing an eager invalidation
+     * were the TopLevelArtifactContext modeled as a separate SkyKey.
+     */
+    TopLevelArtifactContext getTopLevelArtifactContext(SkyKey skyKey);
+
+    /**
+     * Returns all artifacts that need to be built to complete the {@code value}
      */
     ArtifactsToBuild getAllArtifactsToBuild(TValue value, TopLevelArtifactContext context);
 
@@ -80,18 +96,30 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
      * Creates a failed completion value.
      */
     SkyValue createFailed(TValue value, NestedSet<Label> rootCauses);
+
+    /**
+     * Extracts a tag given the {@link SkyKey}.
+     */
+    String extractTag(SkyKey skyKey);
   }
 
   private static class TargetCompletor
       implements Completor<ConfiguredTargetValue, TargetCompletionValue> {
     @Override
     public ConfiguredTargetValue getValueFromSkyKey(SkyKey skyKey, Environment env) {
-      LabelAndConfiguration lac = (LabelAndConfiguration) skyKey.argument();
+      TargetCompletionKey tcKey = (TargetCompletionKey) skyKey.argument();
+      LabelAndConfiguration lac = tcKey.labelAndConfiguration();
       return (ConfiguredTargetValue)
           env.getValue(ConfiguredTargetValue.key(lac.getLabel(), lac.getConfiguration()));
     }
 
     @Override
+    public TopLevelArtifactContext getTopLevelArtifactContext(SkyKey skyKey) {
+      TargetCompletionKey tcKey = (TargetCompletionKey) skyKey.argument();
+      return tcKey.topLevelArtifactContext();
+    }
+
+    @Override
     public ArtifactsToBuild getAllArtifactsToBuild(
         ConfiguredTargetValue value, TopLevelArtifactContext topLevelContext) {
       return TopLevelArtifactHelper.getAllArtifactsToBuild(
@@ -126,16 +154,29 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
     public SkyValue createFailed(ConfiguredTargetValue value, NestedSet<Label> rootCauses) {
       return TargetCompleteEvent.createFailed(value.getConfiguredTarget(), rootCauses);
     }
+
+    @Override
+    public String extractTag(SkyKey skyKey) {
+      return Label.print(
+          ((TargetCompletionKey) skyKey.argument()).labelAndConfiguration().getLabel());
+    }
   }
 
   private static class AspectCompletor implements Completor<AspectValue, AspectCompletionValue> {
     @Override
     public AspectValue getValueFromSkyKey(SkyKey skyKey, Environment env) {
-      AspectKey aspectKey = (AspectKey) skyKey.argument();
+      AspectCompletionKey acKey = (AspectCompletionKey) skyKey.argument();
+      AspectKey aspectKey = acKey.aspectKey();
       return (AspectValue) env.getValue(AspectValue.key(aspectKey));
     }
 
     @Override
+    public TopLevelArtifactContext getTopLevelArtifactContext(SkyKey skyKey) {
+      AspectCompletionKey acKey = (AspectCompletionKey) skyKey.argument();
+      return acKey.topLevelArtifactContext();
+    }
+
+    @Override
     public ArtifactsToBuild getAllArtifactsToBuild(
         AspectValue value, TopLevelArtifactContext topLevelArtifactContext) {
       return TopLevelArtifactHelper.getAllArtifactsToBuild(value, topLevelArtifactContext);
@@ -172,6 +213,11 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
     public SkyValue createFailed(AspectValue value, NestedSet<Label> rootCauses) {
       return AspectCompleteEvent.createFailed(value, rootCauses);
     }
+
+    @Override
+    public String extractTag(SkyKey skyKey) {
+      return Label.print(((AspectCompletionKey) skyKey.argument()).aspectKey().getLabel());
+    }
   }
 
   public static SkyFunction targetCompletionFunction(AtomicReference<EventBus> eventBusRef) {
@@ -195,7 +241,7 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
   @Override
   public SkyValue compute(SkyKey skyKey, Environment env) throws CompletionFunctionException {
     TValue value = completor.getValueFromSkyKey(skyKey, env);
-    TopLevelArtifactContext topLevelContext = PrecomputedValue.TOP_LEVEL_CONTEXT.get(env);
+    TopLevelArtifactContext topLevelContext = completor.getTopLevelArtifactContext(skyKey);
     if (env.valuesMissing()) {
       return null;
     }
@@ -250,7 +296,7 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
 
   @Override
   public String extractTag(SkyKey skyKey) {
-    return Label.print(((LabelAndConfiguration) skyKey.argument()).getLabel());
+    return completor.extractTag(skyKey);
   }
 
   private static final class CompletionFunctionException extends SkyFunctionException {