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
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 56bcd05..414525e 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
@@ -417,15 +417,30 @@
if (!isDirty()) {
return Iterables.concat(getTemporaryDirectDeps());
} else {
- return Iterables.concat(
- Iterables.concat(getTemporaryDirectDeps()),
- buildingState.getAllRemainingDirtyDirectDeps());
+ // There may be duplicates here. Make sure everything is unique.
+ ImmutableSet.Builder<SkyKey> result = ImmutableSet.builder();
+ for (Iterable<SkyKey> group : getTemporaryDirectDeps()) {
+ result.addAll(group);
+ }
+ result.addAll(buildingState.getAllRemainingDirtyDirectDeps(/*preservePosition=*/ false));
+ return result.build();
}
}
@Override
- public synchronized Collection<SkyKey> markRebuildingAndGetAllRemainingDirtyDirectDeps() {
- return buildingState.markRebuildingAndGetAllRemainingDirtyDirectDeps();
+ public synchronized Set<SkyKey> getAllRemainingDirtyDirectDeps() {
+ if (isDirty()) {
+ Preconditions.checkState(buildingState.getDirtyState() == DirtyState.REBUILDING, this);
+ return buildingState.getAllRemainingDirtyDirectDeps(/*preservePosition=*/ true);
+ } else {
+ Preconditions.checkState(buildingState.evaluating, this);
+ return ImmutableSet.of();
+ }
+ }
+
+ @Override
+ public synchronized void markRebuilding() {
+ buildingState.markRebuilding();
}
@Override
