Extract ReverseDepsUtil interface so that InMemoryNodeEntry can be partially isolated from implementation details.
--
MOS_MIGRATED_REVID=108523104
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 8c1fa7f..261c836 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
@@ -98,14 +98,14 @@
* are O(N). Originally reverseDeps was a HashSet, but because of memory consumption we switched
* to a list.
*
- * <p>Internally, ReverseDepsUtil consolidates this data periodically, and when the set of reverse
- * deps is requested. While this operation is not free, it can be done more effectively than
- * trying to remove/check each dirty reverse dependency individually (O(N) each time).
+ * <p>Internally, ReverseDepsUtilImpl consolidates this data periodically, and when the set of
+ * reverse deps is requested. While this operation is not free, it can be done more effectively
+ * than trying to remove/check each dirty reverse dependency individually (O(N) each time).
*/
private List<Object> reverseDepsDataToConsolidate = null;
protected static final ReverseDepsUtil<InMemoryNodeEntry> REVERSE_DEPS_UTIL =
- new ReverseDepsUtil<InMemoryNodeEntry>() {
+ new ReverseDepsUtilImpl<InMemoryNodeEntry>() {
@Override
void setReverseDepsObject(InMemoryNodeEntry container, Object object) {
container.reverseDeps = object;
@@ -209,16 +209,23 @@
return ValueWithMetadata.getMaybeErrorInfo(value);
}
+ /**
+ * Puts entry in "done" state, as checked by {@link #isDone}. Subclasses that override one should
+ * override the other.
+ */
+ protected void markDone() {
+ buildingState = null;
+ }
+
protected synchronized Set<SkyKey> setStateFinishedAndReturnReverseDeps() {
// Get reverse deps that need to be signaled.
ImmutableSet<SkyKey> reverseDepsToSignal = buildingState.getReverseDepsToSignal();
- REVERSE_DEPS_UTIL.addReverseDeps(this, reverseDepsToSignal);
+ getReverseDepsUtil().addReverseDeps(this, reverseDepsToSignal);
// Force consistency check.
- REVERSE_DEPS_UTIL.getReverseDeps(this);
+ getReverseDepsUtil().getReverseDeps(this);
this.directDeps = buildingState.getFinishedDirectDeps().compress();
- // Set state of entry to done.
- buildingState = null;
+ markDone();
if (!keepEdges()) {
this.directDeps = null;
@@ -257,15 +264,19 @@
return setStateFinishedAndReturnReverseDeps();
}
+ protected ReverseDepsUtil<InMemoryNodeEntry> getReverseDepsUtil() {
+ return REVERSE_DEPS_UTIL;
+ }
+
@Override
public synchronized DependencyState addReverseDepAndCheckIfDone(SkyKey reverseDep) {
if (reverseDep != null) {
if (keepEdges()) {
- REVERSE_DEPS_UTIL.maybeCheckReverseDepNotPresent(this, reverseDep);
+ getReverseDepsUtil().maybeCheckReverseDepNotPresent(this, reverseDep);
}
if (isDone()) {
if (keepEdges()) {
- REVERSE_DEPS_UTIL.addReverseDeps(this, ImmutableList.of(reverseDep));
+ getReverseDepsUtil().addReverseDeps(this, ImmutableList.of(reverseDep));
}
} else {
// Parent should never register itself twice in the same build.
@@ -284,10 +295,10 @@
Preconditions.checkNotNull(reverseDep, this);
Preconditions.checkState(keepEdges(), "%s %s", reverseDep, this);
if (!isDone()) {
- REVERSE_DEPS_UTIL.removeReverseDep(this, reverseDep);
+ getReverseDepsUtil().removeReverseDep(this, reverseDep);
buildingState.addReverseDepToSignal(reverseDep);
} else {
- REVERSE_DEPS_UTIL.checkReverseDep(this, reverseDep);
+ getReverseDepsUtil().checkReverseDep(this, reverseDep);
}
return addReverseDepAndCheckIfDone(null);
}
@@ -297,7 +308,7 @@
if (!keepEdges()) {
return;
}
- REVERSE_DEPS_UTIL.removeReverseDep(this, reverseDep);
+ getReverseDepsUtil().removeReverseDep(this, reverseDep);
}
@Override
@@ -308,7 +319,7 @@
@Override
public synchronized Iterable<SkyKey> getReverseDeps() {
assertKeepEdges();
- Iterable<SkyKey> reverseDeps = REVERSE_DEPS_UTIL.getReverseDeps(this);
+ Iterable<SkyKey> reverseDeps = getReverseDepsUtil().getReverseDeps(this);
if (isDone()) {
return reverseDeps;
} else {
@@ -349,7 +360,7 @@
buildingState =
BuildingState.newDirtyState(isChanged, GroupedList.<SkyKey>create(directDeps), value);
value = null;
- return new MarkedDirtyResult(REVERSE_DEPS_UTIL.getReverseDeps(this));
+ return new MarkedDirtyResult(getReverseDepsUtil().getReverseDeps(this));
}
// The caller may be simultaneously trying to mark this node dirty and changed, and the dirty
// thread may have lost the race, but it is the caller's responsibility not to try to mark
@@ -452,7 +463,7 @@
.add("lastChangedVersion", lastChangedVersion)
.add("lastEvaluatedVersion", lastEvaluatedVersion)
.add("directDeps", directDeps == null ? null : GroupedList.create(directDeps))
- .add("reverseDeps", REVERSE_DEPS_UTIL.toString(this))
+ .add("reverseDeps", getReverseDepsUtil().toString(this))
.add("buildingState", buildingState)
.toString();
}
@@ -469,7 +480,7 @@
nodeEntry.value = value;
nodeEntry.lastChangedVersion = this.lastChangedVersion;
nodeEntry.lastEvaluatedVersion = this.lastEvaluatedVersion;
- REVERSE_DEPS_UTIL.addReverseDeps(nodeEntry, REVERSE_DEPS_UTIL.getReverseDeps(this));
+ getReverseDepsUtil().addReverseDeps(nodeEntry, getReverseDepsUtil().getReverseDeps(this));
nodeEntry.directDeps = directDeps;
nodeEntry.buildingState = null;
return nodeEntry;
