Refactor EvaluationProgressReceiver#invalidated into separate metho…

…ds for dirty and deleted.

Many receivers treat these situations differently. Additionally, instead of `InvalidationState`, this allows us to pass in the `DirtyType` to provide the receiver with more detail. `InvalidationState` is kept around as a test utility because some tests make significant use of it.

PiperOrigin-RevId: 571944169
Change-Id: I9ba341648e621ca1fbbb3e585677e844bc93ad9a

src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java

@@ -84,6 +84,7 @@
 import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator;
 import com.google.devtools.build.skyframe.Injectable;
 import com.google.devtools.build.skyframe.MemoizingEvaluator;
+import com.google.devtools.build.skyframe.NodeEntry.DirtyType;
 import com.google.devtools.build.skyframe.RecordingDifferencer;
 import com.google.devtools.build.skyframe.SequencedRecordingDifferencer;
 import com.google.devtools.build.skyframe.SkyFunction;
@@ -230,9 +231,9 @@ protected SkyframeProgressReceiver newSkyframeProgressReceiver() {
   /** A {@link SkyframeProgressReceiver} tracks dirty {@link FileValue.Key}s. */
   protected class SequencedSkyframeProgressReceiver extends SkyframeProgressReceiver {
     @Override
-    public void invalidated(SkyKey skyKey, InvalidationState state) {
-      super.invalidated(skyKey, state);
-      if (state == InvalidationState.DIRTY && skyKey instanceof FileValue.Key) {
+    public void dirtied(SkyKey skyKey, DirtyType dirtyType) {
+      super.dirtied(skyKey, dirtyType);
+      if (skyKey instanceof FileValue.Key) {
         incrementalBuildMonitor.reportInvalidatedFileValue();
       }
     }

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java

@@ -106,6 +106,7 @@
 import com.google.devtools.build.skyframe.EvaluationProgressReceiver;
 import com.google.devtools.build.skyframe.EvaluationResult;
 import com.google.devtools.build.skyframe.GroupedDeps;
+import com.google.devtools.build.skyframe.NodeEntry.DirtyType;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
@@ -1371,8 +1372,8 @@ private final class ActionLookupValueProgressReceiver implements EvaluationProgr
     private final AtomicInteger configuredTargetActionCount = new AtomicInteger();
 
     @Override
-    public void invalidated(SkyKey skyKey, InvalidationState state) {
-      if (skyKey instanceof ActionLookupKey && state != InvalidationState.DELETED) {
+    public void dirtied(SkyKey skyKey, DirtyType dirtyType) {
+      if (skyKey instanceof ActionLookupKey) {
         // If the value was just dirtied and not deleted, then it may not be truly invalid, since
         // it may later get re-validated. Therefore adding the key to dirtiedConfiguredTargetKeys
         // is provisional--if the key is later evaluated and the value found to be clean, then we

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java

@@ -230,6 +230,7 @@
 import com.google.devtools.build.skyframe.Injectable;
 import com.google.devtools.build.skyframe.MemoizingEvaluator;
 import com.google.devtools.build.skyframe.NodeEntry;
+import com.google.devtools.build.skyframe.NodeEntry.DirtyType;
 import com.google.devtools.build.skyframe.RecordingDifferencer;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -2807,11 +2808,19 @@ protected class SkyframeProgressReceiver implements EvaluationProgressReceiver {
     private Map<GlobDescriptor, ImmutableList<GlobDescriptor>> globDeps = new ConcurrentHashMap<>();
 
     @Override
-    public void invalidated(SkyKey skyKey, InvalidationState state) {
+    public void dirtied(SkyKey skyKey, DirtyType dirtyType) {
       if (ignoreInvalidations) {
         return;
       }
-      skyframeBuildView.getProgressReceiver().invalidated(skyKey, state);
+      skyframeBuildView.getProgressReceiver().dirtied(skyKey, dirtyType);
+    }
+
+    @Override
+    public void deleted(SkyKey skyKey) {
+      if (ignoreInvalidations) {
+        return;
+      }
+      skyframeBuildView.getProgressReceiver().deleted(skyKey);
     }
 
     @Override

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

@@ -897,9 +897,7 @@ private void dirtyRewindGraphAndResetEntry(SkyKey key, NodeEntry entry, Reset re
                 key);
 
         if (childEntry.markDirty(DirtyType.REWIND) != null) {
-          evaluatorContext
-              .getProgressReceiver()
-              .invalidated(childToRestart, EvaluationProgressReceiver.InvalidationState.DIRTY);
+          evaluatorContext.getProgressReceiver().dirtied(childToRestart, DirtyType.REWIND);
         }
       }
     }

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

@@ -14,6 +14,7 @@
 package com.google.devtools.build.skyframe;
 
 import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.skyframe.NodeEntry.DirtyType;
 import java.util.function.Supplier;
 import javax.annotation.Nullable;
 
@@ -30,9 +31,16 @@ protected CompoundEvaluationProgressReceiverBase(
   }
 
   @Override
-  public void invalidated(SkyKey skyKey, InvalidationState state) {
+  public void dirtied(SkyKey skyKey, DirtyType dirtyType) {
     for (EvaluationProgressReceiver receiver : receivers) {
-      receiver.invalidated(skyKey, state);
+      receiver.dirtied(skyKey, dirtyType);
+    }
+  }
+
+  @Override
+  public void deleted(SkyKey skyKey) {
+    for (EvaluationProgressReceiver receiver : receivers) {
+      receiver.deleted(skyKey);
     }
   }
 

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

@@ -15,6 +15,7 @@
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
+import com.google.devtools.build.skyframe.NodeEntry.DirtyType;
 import java.util.Set;
 import java.util.function.Supplier;
 import javax.annotation.Nullable;
@@ -41,22 +42,20 @@ protected void injected(SkyKey skyKey) {
   }
 
   @Override
-  public void invalidated(SkyKey skyKey, InvalidationState state) {
+  public void dirtied(SkyKey skyKey, DirtyType dirtyType) {
     if (progressReceiver != null) {
-      progressReceiver.invalidated(skyKey, state);
+      progressReceiver.dirtied(skyKey, dirtyType);
     }
+    addToDirtySet(skyKey);
+  }
 
-    switch (state) {
-      case DELETED:
-        // This key was removed from the graph, so no longer needs to be marked as dirty.
-        removeFromDirtySet(skyKey);
-        break;
-      case DIRTY:
-        addToDirtySet(skyKey);
-        break;
-      default:
-        throw new IllegalStateException(state.toString());
+  @Override
+  public void deleted(SkyKey skyKey) {
+    if (progressReceiver != null) {
+      progressReceiver.deleted(skyKey);
     }
+    // This key was removed from the graph, so no longer needs to be marked as dirty.
+    removeFromDirtySet(skyKey);
   }
 
   @Override
@@ -87,7 +86,7 @@ private void enqueueing(SkyKey skyKey, boolean afterError) {
    * Called when a node was requested to be enqueued but wasn't because either an interrupt or an
    * error (in nokeep_going mode) had occurred.
    */
-  protected void enqueueAfterError(SkyKey skyKey) {
+  void enqueueAfterError(SkyKey skyKey) {
     enqueueing(skyKey, true);
   }
 
@@ -145,7 +144,7 @@ public Set<SkyKey> getAndClearInflightKeys() {
    * collection, where we would not want to remove dirty nodes that are needed for evaluation (in
    * the downward transitive closure of the set of the evaluation's top level nodes).
    */
-  protected Set<SkyKey> getUnenqueuedDirtyKeys() {
+  Set<SkyKey> getUnenqueuedDirtyKeys() {
     return ImmutableSet.copyOf(dirtyKeys);
   }
 

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

@@ -14,6 +14,7 @@
 package com.google.devtools.build.skyframe;
 
 import com.google.devtools.build.lib.concurrent.ThreadSafety;
+import com.google.devtools.build.skyframe.NodeEntry.DirtyType;
 import java.util.function.Supplier;
 import javax.annotation.Nullable;
 
@@ -52,14 +53,6 @@ public Supplier<EvaluationSuccessState> supplier() {
     }
   }
 
-  /** New state of the value entry after invalidation. */
-  enum InvalidationState {
-    /** The value is dirty, although it might get re-validated again. */
-    DIRTY,
-    /** The value is dirty and got deleted, cannot get re-validated again. */
-    DELETED,
-  }
-
   /** Overall state of the node while it is being evaluated. */
   enum NodeState {
     /** The node is undergoing a dirtiness check and may be re-validated. */
@@ -73,16 +66,18 @@ enum NodeState {
   }
 
   /**
-   * Notifies that the node for {@code key} has been invalidated.
-   *
-   * <p>{@code state} indicates the new state of the value.
+   * Notifies that the node for {@code skyKey} has been {@linkplain NodeEntry#markDirty marked
+   * dirty} with the given {@link DirtyType}.
    *
    * <p>May be called concurrently from multiple threads.
    *
-   * <p>If {@code state} is {@link InvalidationState#DIRTY}, should only be called after a
-   * successful {@link NodeEntry#markDirty} call: a call that returns a non-null value.
+   * <p>Only called after a successful {@link NodeEntry#markDirty} call: a call that returns a
+   * non-null value.
    */
-  default void invalidated(SkyKey skyKey, InvalidationState state) {}
+  default void dirtied(SkyKey skyKey, DirtyType dirtyType) {}
+
+  /** Notifies that the node for {@code skyKey} was deleted. */
+  default void deleted(SkyKey skyKey) {}
 
   /**
    * Notifies that {@code skyKey} is about to get queued for evaluation.

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

@@ -416,8 +416,7 @@ public void visit(Collection<SkyKey> keys, InvalidationType invalidationType) {
               }
 
               // Allow custom key-specific logic to update dirtiness status.
-              progressReceiver.invalidated(
-                  key, EvaluationProgressReceiver.InvalidationState.DELETED);
+              progressReceiver.deleted(key);
               // Actually remove the node.
               graph.remove(key);
 
@@ -560,10 +559,12 @@ private void dirtyKeyAndVisitParents(
         return;
       }
 
+      var dirtyType = isChanged ? DirtyType.CHANGE : DirtyType.DIRTY;
+
       // This entry remains in the graph in this dirty state until it is re-evaluated.
       MarkedDirtyResult markedDirtyResult;
       try {
-        markedDirtyResult = entry.markDirty(isChanged ? DirtyType.CHANGE : DirtyType.DIRTY);
+        markedDirtyResult = entry.markDirty(dirtyType);
       } catch (InterruptedException e) {
         Thread.currentThread().interrupt();
         // This can only happen if the main thread has been interrupted, and so the
@@ -580,7 +581,7 @@ private void dirtyKeyAndVisitParents(
         return;
       }
 
-      progressReceiver.invalidated(key, EvaluationProgressReceiver.InvalidationState.DIRTY);
+      progressReceiver.dirtied(key, dirtyType);
       pendingVisitations.remove(Pair.of(key, invalidationType));
 
       // Propagate dirtiness upwards and mark this node dirty/changed. Reverse deps should

src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java

@@ -85,6 +85,7 @@
 import com.google.devtools.build.skyframe.GroupedDeps;
 import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator;
 import com.google.devtools.build.skyframe.MemoizingEvaluator;
+import com.google.devtools.build.skyframe.NodeEntry.DirtyType;
 import com.google.devtools.build.skyframe.RecordingDifferencer;
 import com.google.devtools.build.skyframe.SequencedRecordingDifferencer;
 import com.google.devtools.build.skyframe.SkyFunction;
@@ -479,7 +480,12 @@ void clear() {
     }
 
     @Override
-    public void invalidated(SkyKey skyKey, InvalidationState state) {
+    public void dirtied(SkyKey skyKey, DirtyType dirtyType) {
+      invalidations.add(skyKey);
+    }
+
+    @Override
+    public void deleted(SkyKey skyKey) {
       invalidations.add(skyKey);
     }
 

src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java

@@ -44,9 +44,9 @@
 import com.google.devtools.build.lib.vfs.Root;
 import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.devtools.build.skyframe.ErrorInfo;
-import com.google.devtools.build.skyframe.EvaluationProgressReceiver;
 import com.google.devtools.build.skyframe.EvaluationProgressReceiver.EvaluationState;
 import com.google.devtools.build.skyframe.GroupedDeps;
+import com.google.devtools.build.skyframe.InvalidationProgressReceiver;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import com.google.devtools.build.skyframe.SkyframeLookupResult;
@@ -83,7 +83,7 @@ public final void createExecutor() {
   }
 
   private static final class TrackingEvaluationProgressReceiver
-      implements EvaluationProgressReceiver {
+      extends InvalidationProgressReceiver {
 
     public static final class InvalidatedKey {
       public final SkyKey skyKey;
@@ -161,7 +161,7 @@ public EvaluatedEntry getEvaluatedEntry(SkyKey forKey) {
     }
 
     @Override
-    public void invalidated(SkyKey skyKey, InvalidationState state) {
+    protected void invalidated(SkyKey skyKey, InvalidationState state) {
       invalidated.add(new InvalidatedKey(skyKey, state));
     }
 

src/test/java/com/google/devtools/build/skyframe/BUILD

@@ -20,6 +20,7 @@ TESTUTIL_FILES = [
     "TrackingAwaiter.java",
     "GraphTester.java",
     "GenericFunctionException.java",
+    "InvalidationProgressReceiver.java",
     "SimpleSkyframeLookupResult.java",
     "SomeErrorException.java",
     "TrackingProgressReceiver.java",
@@ -59,7 +60,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/events",
-        "//src/main/java/com/google/devtools/build/lib/skyframe:package_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils",
         "//src/main/java/com/google/devtools/build/lib/util",