Remove reverse deps lazily, only when the node has finished building and we discover that it no longer has certain deps.

In the common case, where a node's deps do not change in the end, this reduces lock contention and CPU.

The downside of this is that we now create a set of the previous reverse deps during each evaluation of a node. We don't store this set in order to conserve memory, so we pay for it in CPU. We will probably only construct it two or three times (most SkyFunctions don't have so many groups), so the cost shouldn't be so high, but we can try to mitigate if it shows up in profiling.

--
MOS_MIGRATED_REVID=122566267

1 files changed, 15 insertions, 1 deletions

diff --git a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
index c18f07666d..c4f0047f3b 100644
--- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
@@ -1936,7 +1936,11 @@ public class MemoizingEvaluatorTest {
                 new StringValue("second"),
                 ImmutableList.<SkyKey>of()));
     // And mid is independently marked as modified,
-    tester.getOrCreate(midKey, /*markAsModified=*/ true);
+    tester
+        .getOrCreate(midKey, /*markAsModified=*/ true)
+        .removeDependency(changedKey)
+        .setComputedValue(null)
+        .setConstantValue(new StringValue("mid"));
     tester.invalidate();
     SkyKey newTopKey = GraphTester.skyKey("newTop");
     // And changed will start rebuilding independently of midKey, because it's requested directly by
@@ -4127,6 +4131,16 @@ public class MemoizingEvaluatorTest {
     assertThat(tester.evalAndGet(/*keepGoing=*/ true, inactiveKey)).isEqualTo(val);
   }
 
+  @Test
+  public void errorChanged() throws Exception {
+    SkyKey error = GraphTester.skyKey("error");
+    tester.getOrCreate(error).setHasTransientError(true);
+    assertThatErrorInfo(tester.evalAndGetError(error)).hasExceptionThat().isNotNull();
+    tester.getOrCreate(error, /*markAsModified=*/ true);
+    tester.invalidate();
+    assertThatErrorInfo(tester.evalAndGetError(error)).hasExceptionThat().isNotNull();
+  }
+
   /** A graph tester that is specific to the memoizing evaluator, with some convenience methods. */
   protected class MemoizingEvaluatorTester extends GraphTester {
     private RecordingDifferencer differencer = new RecordingDifferencer();