Remove ConfiguredTarget from TargetCompletionValue, since it is no longer needed: we can get the ConfiguredTargetKey directly from the TargetCompletionKey. Since that was the only use of the actual value in EvaluationProgressReceiver#evaluated, remove it, instead just provide a boolean enum that gives whether or not evaluation succeeded.

PiperOrigin-RevId: 199178047

12 files changed, 95 insertions, 68 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
index 644f34e448..b0447fd945 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
@@ -13,17 +13,14 @@
 // limitations under the License.
 package com.google.devtools.build.lib.buildtool;
 
-import com.google.common.base.Supplier;
 import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType;
 import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter;
 import com.google.devtools.build.lib.actions.ActionLookupData;
-import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.clock.BlazeClock;
 import com.google.devtools.build.lib.skyframe.ActionExecutionInactivityWatchdog;
 import com.google.devtools.build.lib.skyframe.AspectCompletionValue;
-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.ConfiguredTargetKey;
 import com.google.devtools.build.lib.skyframe.SkyFunctions;
@@ -32,12 +29,12 @@ import com.google.devtools.build.lib.skyframe.TargetCompletionValue;
 import com.google.devtools.build.skyframe.EvaluationProgressReceiver;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
-import com.google.devtools.build.skyframe.SkyValue;
 import java.text.NumberFormat;
 import java.util.Collections;
 import java.util.Locale;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Supplier;
 
 /**
  * Listener for executed actions and built artifacts. We use a listener so that we have an
@@ -101,22 +98,20 @@ public final class ExecutionProgressReceiver
   }
 
   @Override
-  public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier, EvaluationState state) {
+  public void evaluated(
+      SkyKey skyKey,
+      Supplier<EvaluationSuccessState> evaluationSuccessState,
+      EvaluationState state) {
     SkyFunctionName type = skyKey.functionName();
     if (type.equals(SkyFunctions.TARGET_COMPLETION)) {
-      TargetCompletionValue value = (TargetCompletionValue) skyValueSupplier.get();
-      if (value == null) {
-        return;
+      if (evaluationSuccessState.get().succeeded()) {
+        builtTargets.add(
+            ((TargetCompletionValue.TargetCompletionKey) skyKey).configuredTargetKey());
       }
-      ConfiguredTarget target = value.getConfiguredTarget();
-      builtTargets.add(ConfiguredTargetKey.inTargetConfig(target));
     } else if (type.equals(SkyFunctions.ASPECT_COMPLETION)) {
-      AspectCompletionValue value = (AspectCompletionValue) skyValueSupplier.get();
-      if (value == null) {
-        return;
+      if (evaluationSuccessState.get().succeeded()) {
+        builtAspects.add(((AspectCompletionValue.AspectCompletionKey) skyKey).aspectKey());
       }
-      AspectKey aspectKey = ((AspectCompletionKey) skyKey).aspectKey();
-      builtAspects.add(aspectKey);
     } else if (type.equals(SkyFunctions.ACTION_EXECUTION)) {
       // Remember all completed actions, even those in error, regardless of having been cached or
       // really executed.
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 83e879994f..de70aa1af0 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
@@ -79,10 +79,8 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
     MissingInputFileException getMissingFilesException(
         TValue value, int missingCount, Environment env) throws InterruptedException;
 
-    /**
-     * Creates a successful completion value.
-     */
-    TResult createResult(TValue value);
+    /** Provides a successful completion value. */
+    TResult getResult();
 
     /** Creates a failed completion value. */
     ExtendedEventHandler.Postable createFailed(
@@ -157,8 +155,8 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
     }
 
     @Override
-    public TargetCompletionValue createResult(ConfiguredTargetValue value) {
-      return new TargetCompletionValue(value.getConfiguredTarget());
+    public TargetCompletionValue getResult() {
+      return TargetCompletionValue.INSTANCE;
     }
 
     @Override
@@ -251,7 +249,7 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
     }
 
     @Override
-    public AspectCompletionValue createResult(AspectValue value) {
+    public AspectCompletionValue getResult() {
       return AspectCompletionValue.INSTANCE;
     }
 
@@ -369,7 +367,7 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
       return null;
     }
     env.getListener().post(postable);
-    return completor.createResult(value);
+    return completor.getResult();
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index c4491ff6a0..fefb8fb6cf 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -15,7 +15,6 @@ package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
-import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -75,12 +74,12 @@ import com.google.devtools.build.skyframe.EvaluationProgressReceiver;
 import com.google.devtools.build.skyframe.EvaluationResult;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.SkyKey;
-import com.google.devtools.build.skyframe.SkyValue;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Supplier;
 import javax.annotation.Nullable;
 
 /**
@@ -675,12 +674,14 @@ public final class SkyframeBuildView {
     }
 
     @Override
-    public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier,
+    public void evaluated(
+        SkyKey skyKey,
+        Supplier<EvaluationSuccessState> evaluationSuccessState,
         EvaluationState state) {
       if (skyKey.functionName().equals(SkyFunctions.CONFIGURED_TARGET)) {
         switch (state) {
           case BUILT:
-            if (skyValueSupplier.get() != null) {
+            if (evaluationSuccessState.get().succeeded()) {
               evaluatedConfiguredTargets.add(skyKey);
               // During multithreaded operation, this is only set to true, so no concurrency issues.
               someConfiguredTargetEvaluated = true;
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 c424bd44a3..c0bb40456d 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
@@ -21,7 +21,6 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.base.Stopwatch;
-import com.google.common.base.Supplier;
 import com.google.common.base.Throwables;
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
@@ -179,6 +178,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BooleanSupplier;
+import java.util.function.Supplier;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -1319,7 +1319,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     try {
       progressReceiver.executionProgressReceiver = executionProgressReceiver;
       Iterable<SkyKey> artifactKeys = ArtifactSkyKey.mandatoryKeys(artifactsToBuild);
-      Iterable<SkyKey> targetKeys =
+      Iterable<TargetCompletionValue.TargetCompletionKey> targetKeys =
           TargetCompletionValue.keys(targetsToBuild, topLevelArtifactContext, targetsToTest);
       Iterable<SkyKey> aspectKeys = AspectCompletionValue.keys(aspects, topLevelArtifactContext);
       Iterable<SkyKey> testKeys =
@@ -2320,13 +2320,16 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     }
 
     @Override
-    public void evaluated(SkyKey skyKey, Supplier<SkyValue> valueSupplier, EvaluationState state) {
+    public void evaluated(
+        SkyKey skyKey,
+        Supplier<EvaluationSuccessState> evaluationSuccessState,
+        EvaluationState state) {
       if (ignoreInvalidations) {
         return;
       }
-      skyframeBuildView.getProgressReceiver().evaluated(skyKey, valueSupplier, state);
+      skyframeBuildView.getProgressReceiver().evaluated(skyKey, evaluationSuccessState, state);
       if (executionProgressReceiver != null) {
-        executionProgressReceiver.evaluated(skyKey, valueSupplier, state);
+        executionProgressReceiver.evaluated(skyKey, evaluationSuccessState, state);
       }
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
index f15e6fbf49..dbbb6a70d4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
@@ -28,24 +28,18 @@ import java.util.Set;
  * The value of a TargetCompletion. Currently this just stores a ConfiguredTarget.
  */
 public class TargetCompletionValue implements SkyValue {
-  private final ConfiguredTarget ct;
+  @AutoCodec static final TargetCompletionValue INSTANCE = new TargetCompletionValue();
 
-  TargetCompletionValue(ConfiguredTarget ct) {
-    this.ct = ct;
-  }
-
-  public ConfiguredTarget getConfiguredTarget() {
-    return ct;
-  }
+  private TargetCompletionValue() {}
 
-  public static SkyKey key(
+  public static TargetCompletionKey key(
       ConfiguredTargetKey configuredTargetKey,
       TopLevelArtifactContext topLevelArtifactContext,
       boolean willTest) {
     return TargetCompletionKey.create(configuredTargetKey, topLevelArtifactContext, willTest);
   }
 
-  public static Iterable<SkyKey> keys(
+  public static Iterable<TargetCompletionKey> keys(
       Collection<ConfiguredTarget> targets,
       final TopLevelArtifactContext ctx,
       final Set<ConfiguredTarget> targetsToTest) {
@@ -63,7 +57,7 @@ public class TargetCompletionValue implements SkyValue {
   /** {@link SkyKey} for {@link TargetCompletionValue}. */
   @AutoCodec
   @AutoValue
-  abstract static class TargetCompletionKey implements SkyKey {
+  public abstract static class TargetCompletionKey implements SkyKey {
     @AutoCodec.Instantiator
     static TargetCompletionKey create(
         ConfiguredTargetKey configuredTargetKey,
@@ -78,9 +72,10 @@ public class TargetCompletionValue implements SkyValue {
       return SkyFunctions.TARGET_COMPLETION;
     }
 
-    abstract ConfiguredTargetKey configuredTargetKey();
+    public abstract ConfiguredTargetKey configuredTargetKey();
+
+    abstract TopLevelArtifactContext topLevelArtifactContext();
 
-    public abstract TopLevelArtifactContext topLevelArtifactContext();
-    public abstract boolean willTest();
+    abstract boolean willTest();
   }
 }
diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractExceptionalParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/AbstractExceptionalParallelEvaluator.java
index 43068fe839..7771d833ec 100644
--- a/src/main/java/com/google/devtools/build/skyframe/AbstractExceptionalParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/AbstractExceptionalParallelEvaluator.java
@@ -14,7 +14,6 @@
 package com.google.devtools.build.skyframe;
 
 import com.google.common.base.Preconditions;
-import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -25,6 +24,7 @@ import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 import com.google.devtools.build.lib.util.GroupedList;
 import com.google.devtools.build.skyframe.EvaluationProgressReceiver.EvaluationState;
+import com.google.devtools.build.skyframe.EvaluationProgressReceiver.EvaluationSuccessState;
 import com.google.devtools.build.skyframe.MemoizingEvaluator.EmittedEventState;
 import com.google.devtools.build.skyframe.NodeEntry.DependencyState;
 import com.google.devtools.build.skyframe.QueryableGraph.Reason;
@@ -156,7 +156,9 @@ public abstract class AbstractExceptionalParallelEvaluator<E extends Exception>
           .getProgressReceiver()
           .evaluated(
               key,
-              Suppliers.ofInstance(value),
+              value != null
+                  ? EvaluationSuccessState.SUCCESS.supplier()
+                  : EvaluationSuccessState.FAILURE.supplier(),
               valueVersion.equals(evaluatorContext.getGraphVersion())
                   ? EvaluationState.BUILT
                   : EvaluationState.CLEAN);
diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
index 5cb810f699..2373959857 100644
--- a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
@@ -286,7 +286,7 @@ public abstract class AbstractParallelEvaluator {
           // Tell the receiver that the value was not actually changed this run.
           evaluatorContext
               .getProgressReceiver()
-              .evaluated(skyKey, new SkyValueSupplier(state), EvaluationState.CLEAN);
+              .evaluated(skyKey, new EvaluationSuccessStateSupplier(state), EvaluationState.CLEAN);
           if (!evaluatorContext.keepGoing() && state.getErrorInfo() != null) {
             if (!evaluatorContext.getVisitor().preventNewEvaluations()) {
               return DirtyOutcome.ALREADY_PROCESSED;
diff --git a/src/main/java/com/google/devtools/build/skyframe/CompoundEvaluationProgressReceiver.java b/src/main/java/com/google/devtools/build/skyframe/CompoundEvaluationProgressReceiver.java
index 894df66fda..9f54ca0f57 100644
--- a/src/main/java/com/google/devtools/build/skyframe/CompoundEvaluationProgressReceiver.java
+++ b/src/main/java/com/google/devtools/build/skyframe/CompoundEvaluationProgressReceiver.java
@@ -13,8 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.skyframe;
 
-import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
+import java.util.function.Supplier;
 
 /**
  * An {@link EvaluationProgressReceiver} that delegates to a bunch of other
@@ -61,9 +61,12 @@ public class CompoundEvaluationProgressReceiver implements EvaluationProgressRec
   }
 
   @Override
-  public void evaluated(SkyKey skyKey, Supplier<SkyValue> valueSupplier, EvaluationState state) {
+  public void evaluated(
+      SkyKey skyKey,
+      Supplier<EvaluationSuccessState> evaluationSuccessState,
+      EvaluationState state) {
     for (EvaluationProgressReceiver receiver : receivers) {
-      receiver.evaluated(skyKey, valueSupplier, state);
+      receiver.evaluated(skyKey, evaluationSuccessState, state);
     }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/skyframe/DirtyTrackingProgressReceiver.java b/src/main/java/com/google/devtools/build/skyframe/DirtyTrackingProgressReceiver.java
index 22f9dfba1d..820fb51cfe 100644
--- a/src/main/java/com/google/devtools/build/skyframe/DirtyTrackingProgressReceiver.java
+++ b/src/main/java/com/google/devtools/build/skyframe/DirtyTrackingProgressReceiver.java
@@ -13,10 +13,10 @@
 // limitations under the License.
 package com.google.devtools.build.skyframe;
 
-import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import java.util.Set;
+import java.util.function.Supplier;
 import javax.annotation.Nullable;
 
 /**
@@ -106,10 +106,12 @@ public class DirtyTrackingProgressReceiver implements EvaluationProgressReceiver
   }
 
   @Override
-  public void evaluated(SkyKey skyKey, Supplier<SkyValue> valueSupplier,
+  public void evaluated(
+      SkyKey skyKey,
+      Supplier<EvaluationSuccessState> evaluationSuccessState,
       EvaluationState state) {
     if (progressReceiver != null) {
-      progressReceiver.evaluated(skyKey, valueSupplier, state);
+      progressReceiver.evaluated(skyKey, evaluationSuccessState, state);
     }
 
     // This key was either built or marked clean, so we can remove it from both the dirty and
diff --git a/src/main/java/com/google/devtools/build/skyframe/EvaluationProgressReceiver.java b/src/main/java/com/google/devtools/build/skyframe/EvaluationProgressReceiver.java
index 2844ec4f45..d93812b139 100644
--- a/src/main/java/com/google/devtools/build/skyframe/EvaluationProgressReceiver.java
+++ b/src/main/java/com/google/devtools/build/skyframe/EvaluationProgressReceiver.java
@@ -13,8 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.skyframe;
 
-import com.google.common.base.Supplier;
 import com.google.devtools.build.lib.concurrent.ThreadSafety;
+import java.util.function.Supplier;
 
 /** Receiver for various stages of the lifetime of a skyframe node evaluation. */
 @ThreadSafety.ThreadSafe
@@ -29,6 +29,26 @@ public interface EvaluationProgressReceiver {
     CLEAN,
   }
 
+  /** Whether or not evaluation of this node succeeded. */
+  enum EvaluationSuccessState {
+    SUCCESS(true),
+    FAILURE(false);
+
+    EvaluationSuccessState(boolean succeeded) {
+      this.succeeded = succeeded;
+    }
+
+    private final boolean succeeded;
+
+    public boolean succeeded() {
+      return succeeded;
+    }
+
+    public Supplier<EvaluationSuccessState> supplier() {
+      return () -> this;
+    }
+  }
+
   /**
    * New state of the value entry after invalidation.
    */
@@ -97,10 +117,13 @@ public interface EvaluationProgressReceiver {
    *
    * <p>{@code state} indicates the new state of the node.
    *
-   * <p>If the value builder threw an error when building this node, then
-   * {@code valueSupplier.get()} evaluates to null.
+   * <p>If the value builder threw an error when building this node, then {@code
+   * valueSupplier.get()} evaluates to null.
    */
-  void evaluated(SkyKey skyKey, Supplier<SkyValue> valueSupplier, EvaluationState state);
+  void evaluated(
+      SkyKey skyKey,
+      Supplier<EvaluationSuccessState> evaluationSuccessState,
+      EvaluationState state);
 
   /** An {@link EvaluationProgressReceiver} that does nothing. */
   class NullEvaluationProgressReceiver implements EvaluationProgressReceiver {
@@ -121,7 +144,9 @@ public interface EvaluationProgressReceiver {
     }
 
     @Override
-    public void evaluated(SkyKey skyKey, Supplier<SkyValue> valueSupplier, EvaluationState state) {
-    }
+    public void evaluated(
+        SkyKey skyKey,
+        Supplier<EvaluationSuccessState> evaluationSuccessState,
+        EvaluationState state) {}
   }
 }
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyValueSupplier.java b/src/main/java/com/google/devtools/build/skyframe/EvaluationSuccessStateSupplier.java
index d05dbd729f..f4c320ca6e 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyValueSupplier.java
+++ b/src/main/java/com/google/devtools/build/skyframe/EvaluationSuccessStateSupplier.java
@@ -14,22 +14,25 @@
 package com.google.devtools.build.skyframe;
 
 import com.google.common.base.Supplier;
+import com.google.devtools.build.skyframe.EvaluationProgressReceiver.EvaluationSuccessState;
 
 /**
- * Supplier of {@link SkyValue} that crashes if its contained {@link NodeEntry} throws an {@link
- * InterruptedException} on value retrieval.
+ * Supplier of {@link EvaluationSuccessState} that crashes if its contained {@link NodeEntry} throws
+ * an {@link InterruptedException} on value retrieval.
  */
-class SkyValueSupplier implements Supplier<SkyValue> {
+class EvaluationSuccessStateSupplier implements Supplier<EvaluationSuccessState> {
   private final NodeEntry state;
 
-  SkyValueSupplier(NodeEntry state) {
+  EvaluationSuccessStateSupplier(NodeEntry state) {
     this.state = state;
   }
 
   @Override
-  public SkyValue get() {
+  public EvaluationSuccessState get() {
     try {
-      return state.getValue();
+      return state.getValue() != null
+          ? EvaluationSuccessState.SUCCESS
+          : EvaluationSuccessState.FAILURE;
     } catch (InterruptedException e) {
       throw new IllegalStateException(
           "Graph implementations in which value retrieval can block should not be used in "
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 905e822c57..67b7176193 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
@@ -562,7 +562,7 @@ class SkyFunctionEnvironment extends AbstractSkyFunctionEnvironment {
         .getProgressReceiver()
         .evaluated(
             skyKey,
-            new SkyValueSupplier(primaryEntry),
+            new EvaluationSuccessStateSupplier(primaryEntry),
             valueVersion.equals(evaluatorContext.getGraphVersion())
                 ? EvaluationState.BUILT
                 : EvaluationState.CLEAN);