Stop storing reverse deps to signal in BuildingState. Instead, re-use the reverseDepsToConsolidate field in InMemoryNodeEntry. As part of that, revamp our logic of how we store pending operations: store adds bare on initial evaluations, and checks bare on incremental evaluations and operations on done nodes.

This should improve performance in two ways: BuildingState loses two fields, saving working memory intra-build. Storing pending reverse dep operations bare also saves memory intra-build. Note that neither of these changes helps resting memory state, only while a node is still evaluating.

Because of this, we can simplify ReverseDepsUtil a bit, making ReverseDepsUtilImpl a static class, which it always wanted to be (what it really wants to be is a superclass of InMemoryNodeEntry, but I don't want to spend the object alignment bits).

Finally, this makes it pretty tempting to get rid of BuildingState altogether on initial evaluations. We'd still keep DirtyBuildingState, but we could save another ~24 bytes by storing BuildingState's one remaining field, signaledDeps, directly inside InMemoryNodeEntry.

--
PiperOrigin-RevId: 151048879
MOS_MIGRATED_REVID=151048879

1 files changed, 3 insertions, 12 deletions

diff --git a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
index 577639a618..d7bb2d2cab 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
@@ -271,16 +271,7 @@ public abstract class InvalidatingNodeVisitor<TGraph extends QueryableGraph> {
 
                 if (traverseGraph) {
                   // Propagate deletion upwards.
-                  try {
-                    visit(entry.getReverseDeps(), InvalidationType.DELETED);
-                  } catch (InterruptedException e) {
-                    throw new IllegalStateException(
-                        "Deletion cannot happen on a graph that may have blocking operations: "
-                            + key
-                            + ", "
-                            + entry,
-                        e);
-                  }
+                  visit(entry.getAllReverseDepsForNodeBeingDeleted(), InvalidationType.DELETED);
 
                   // Unregister this node as an rdep from its direct deps, since reverse dep
                   // edges cannot point to non-existent nodes. To know whether the child has this
@@ -339,8 +330,8 @@ public abstract class InvalidatingNodeVisitor<TGraph extends QueryableGraph> {
                 }
 
                 // Allow custom key-specific logic to update dirtiness status.
-                progressReceiver.invalidated(key,
-                    EvaluationProgressReceiver.InvalidationState.DELETED);
+                progressReceiver.invalidated(
+                    key, EvaluationProgressReceiver.InvalidationState.DELETED);
                 // Actually remove the node.
                 graph.remove(key);