Allow NodeEntry implementations to keep just deps, as opposed to all edges or no edges. Also add option to disable checks in MemoizingEvaluatorTest that don't make sense for implementations that don't keep track of dirty nodes. Also extract RecordingDifferencer to an interface. And add a test for the situation that a node changes during a build that it's not requested, and which fails, necessitating cleanup.

PiperOrigin-RevId: 171616817
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
index 007cfef..67e1867 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
@@ -73,7 +73,7 @@
 public class InMemoryNodeEntry implements NodeEntry {
 
   /** Actual data stored in this entry when it is done. */
-  private SkyValue value = null;
+  protected SkyValue value = null;
 
   /**
    * The last version of the graph at which this node's value was changed. In {@link #setValue} it
@@ -95,13 +95,13 @@
 
   /**
    * This object represents the direct deps of the node, in groups if the {@code SkyFunction}
-   * requested them that way. It contains either the in-progress direct deps, stored as a
-   * {@code GroupedList<SkyKey>} before the node is finished building, or the full direct deps,
-   * compressed in a memory-efficient way (via {@link GroupedList#compress}, after the node is done.
+   * requested them that way. It contains either the in-progress direct deps, stored as a {@code
+   * GroupedList<SkyKey>} before the node is finished building, or the full direct deps, compressed
+   * in a memory-efficient way (via {@link GroupedList#compress}, after the node is done.
    *
    * <p>It is initialized lazily in getTemporaryDirectDeps() to save a little bit more memory.
    */
-  private Object directDeps = null;
+  protected Object directDeps = null;
 
   /**
    * This list stores the reverse dependencies of this node that have been declared so far.
@@ -175,8 +175,12 @@
   }
 
   @Override
-  public boolean keepEdges() {
-    return true;
+  public KeepEdgesPolicy keepEdges() {
+    return KeepEdgesPolicy.ALL;
+  }
+
+  private boolean keepReverseDeps() {
+    return keepEdges() == KeepEdgesPolicy.ALL;
   }
 
   @Override
@@ -224,7 +228,7 @@
    * added in {@link #addTemporaryDirectDeps}.
    */
   public synchronized GroupedList<SkyKey> getGroupedDirectDeps() {
-    assertKeepEdges();
+    assertKeepDeps();
     Preconditions.checkState(isDone(), "no deps until done. NodeEntry: %s", this);
     return GroupedList.create(directDeps);
   }
@@ -236,7 +240,7 @@
     return ValueWithMetadata.getMaybeErrorInfo(value);
   }
 
-  private DirtyBuildingState getDirtyBuildingState() {
+  protected DirtyBuildingState getDirtyBuildingState() {
     return Preconditions.checkNotNull(dirtyBuildingState, "Didn't have state: %s", this);
   }
 
@@ -249,20 +253,18 @@
     signaledDeps = NOT_EVALUATING_SENTINEL;
   }
 
-  protected synchronized Set<SkyKey> setStateFinishedAndReturnReverseDepsToSignal() {
+  protected final synchronized Set<SkyKey> setStateFinishedAndReturnReverseDepsToSignal() {
     Set<SkyKey> reverseDepsToSignal =
         ReverseDepsUtility.consolidateDataAndReturnNewElements(this, getOpToStoreBare());
     this.directDeps = getTemporaryDirectDeps().compress();
 
     markDone();
-
-    if (!keepEdges()) {
-      this.directDeps = null;
-      this.reverseDeps = null;
-    }
+    postProcessAfterDone();
     return reverseDepsToSignal;
   }
 
+  protected void postProcessAfterDone() {}
+
   @Override
   public synchronized Set<SkyKey> getInProgressReverseDeps() {
     Preconditions.checkState(!isDone(), this);
@@ -298,7 +300,7 @@
   public synchronized DependencyState addReverseDepAndCheckIfDone(SkyKey reverseDep) {
     if (reverseDep != null) {
       if (isDone()) {
-        if (keepEdges()) {
+        if (keepReverseDeps()) {
           ReverseDepsUtility.addReverseDeps(this, ImmutableList.of(reverseDep));
         }
       } else {
@@ -356,7 +358,13 @@
   @Override
   public synchronized DependencyState checkIfDoneForDirtyReverseDep(SkyKey reverseDep) {
     Preconditions.checkNotNull(reverseDep, this);
-    Preconditions.checkState(keepEdges(), "%s %s", reverseDep, this);
+    // Note that implementations of InMemoryNodeEntry that have
+    // #keepEdges == KeepEdgesPolicy.JUST_DEPS may override this entire method.
+    Preconditions.checkState(
+        keepEdges() == KeepEdgesPolicy.ALL,
+        "Incremental means keeping edges %s %s",
+        reverseDep,
+        this);
     if (isDone()) {
       ReverseDepsUtility.checkReverseDep(this, reverseDep);
     } else {
@@ -367,7 +375,7 @@
 
   @Override
   public synchronized void removeReverseDep(SkyKey reverseDep) {
-    if (!keepEdges()) {
+    if (!keepReverseDeps()) {
       return;
     }
     if (isDone()) {
@@ -386,14 +394,14 @@
 
   @Override
   public synchronized Set<SkyKey> getReverseDepsForDoneEntry() {
-    assertKeepEdges();
+    assertKeepRdeps();
     Preconditions.checkState(isDone(), "Called on not done %s", this);
     return ReverseDepsUtility.getReverseDeps(this);
   }
 
   @Override
   public synchronized Set<SkyKey> getAllReverseDepsForNodeBeingDeleted() {
-    assertKeepEdges();
+    assertKeepRdeps();
     if (!isDone()) {
       // This consolidation loses information about pending reverse deps to signal, but that is
       // unimportant since this node is being deleted.
@@ -429,13 +437,19 @@
   }
 
   /** Checks that a caller is not trying to access not-stored graph edges. */
-  private void assertKeepEdges() {
-    Preconditions.checkState(keepEdges(), "Graph edges not stored. %s", this);
+  private void assertKeepDeps() {
+    Preconditions.checkState(keepEdges() != KeepEdgesPolicy.NONE, "Not keeping deps: %s", this);
+  }
+
+  /** Checks that a caller is not trying to access not-stored graph edges. */
+  private void assertKeepRdeps() {
+    Preconditions.checkState(keepEdges() == KeepEdgesPolicy.ALL, "Not keeping rdeps: %s", this);
   }
 
   @Override
   public synchronized MarkedDirtyResult markDirty(boolean isChanged) {
-    assertKeepEdges();
+    // Can't process a dirty node without its deps.
+    assertKeepDeps();
     if (isDone()) {
       dirtyBuildingState =
           DirtyBuildingState.create(isChanged, GroupedList.<SkyKey>create(directDeps), value);
@@ -601,21 +615,24 @@
         .toString();
   }
 
+  protected synchronized InMemoryNodeEntry cloneNodeEntry(InMemoryNodeEntry newEntry) {
+    // As this is temporary, for now let's limit to done nodes.
+    Preconditions.checkState(isDone(), "Only done nodes can be copied: %s", this);
+    newEntry.value = value;
+    newEntry.lastChangedVersion = this.lastChangedVersion;
+    newEntry.lastEvaluatedVersion = this.lastEvaluatedVersion;
+    ReverseDepsUtility.addReverseDeps(newEntry, ReverseDepsUtility.getReverseDeps(this));
+    newEntry.directDeps = directDeps;
+    newEntry.dirtyBuildingState = null;
+    return newEntry;
+  }
+
   /**
    * Do not use except in custom evaluator implementations! Added only temporarily.
    *
    * <p>Clones a InMemoryMutableNodeEntry iff it is a done node. Otherwise it fails.
    */
   public synchronized InMemoryNodeEntry cloneNodeEntry() {
-    // As this is temporary, for now let's limit to done nodes.
-    Preconditions.checkState(isDone(), "Only done nodes can be copied: %s", this);
-    InMemoryNodeEntry nodeEntry = new InMemoryNodeEntry();
-    nodeEntry.value = value;
-    nodeEntry.lastChangedVersion = this.lastChangedVersion;
-    nodeEntry.lastEvaluatedVersion = this.lastEvaluatedVersion;
-    ReverseDepsUtility.addReverseDeps(nodeEntry, ReverseDepsUtility.getReverseDeps(this));
-    nodeEntry.directDeps = directDeps;
-    nodeEntry.dirtyBuildingState = null;
-    return nodeEntry;
+    return cloneNodeEntry(new InMemoryNodeEntry());
   }
 }