Refactor BuildingState to save memory.
Collapse the "evaluating" boolean into the "signaledDeps" int field, since signaledDeps is always 0 if evaluating is false, so we can use the sentinel value -1 to indicate that evaluation has not yet started. This leads to a slightly less tolerant node entry: it must "start evaluating" before you can do things like set its value. Places that wasn't being done have been fixed, at least as far as we have test coverage for.
Also, factor the "dirty" parts of BuildingState out into a subclass. It would probably be cleaner to use composition here, but I don't want to pay the price of another object.
--
MOS_MIGRATED_REVID=126729331
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 63eb7fd..9ba0dbe 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
@@ -163,9 +163,9 @@
if (isDone()) {
return getErrorInfo() == null ? getValue() : null;
} else if (isChanged() || isDirty()) {
- return (buildingState.getLastBuildValue() == null)
- ? null
- : ValueWithMetadata.justValue(buildingState.getLastBuildValue());
+ return (getDirtyBuildingState().getLastBuildValue() == null)
+ ? null
+ : ValueWithMetadata.justValue(getDirtyBuildingState().getLastBuildValue());
} else {
// Value has not finished evaluating. It's probably about to be cleaned from the graph.
return null;
@@ -194,6 +194,10 @@
return ValueWithMetadata.getMaybeErrorInfo(value);
}
+ private DirtyBuildingState getDirtyBuildingState() {
+ return (DirtyBuildingState) Preconditions.checkNotNull(buildingState, this);
+ }
+
/**
* Puts entry in "done" state, as checked by {@link #isDone}. Subclasses that override one may
* need to override the other.
@@ -235,10 +239,10 @@
this.lastEvaluatedVersion.atMost(version), "%s %s %s", this, version, value);
this.lastEvaluatedVersion = version;
- if (isDirty() && buildingState.unchangedFromLastBuild(value)) {
+ if (isDirty() && getDirtyBuildingState().unchangedFromLastBuild(value)) {
// If the value is the same as before, just use the old value. Note that we don't use the new
// value, because preserving == equality is even better than .equals() equality.
- this.value = buildingState.getLastBuildValue();
+ this.value = getDirtyBuildingState().getLastBuildValue();
} else {
// If this is a new value, or it has changed since the last build, set the version to the
// current graph version.
@@ -345,7 +349,7 @@
assertKeepEdges();
if (isDone()) {
buildingState =
- BuildingState.newDirtyState(isChanged, GroupedList.<SkyKey>create(directDeps), value);
+ DirtyBuildingState.create(isChanged, GroupedList.<SkyKey>create(directDeps), value);
value = null;
directDeps = null;
return new MarkedDirtyResult(getReverseDepsUtil().getReverseDeps(this));
@@ -360,17 +364,17 @@
if (isChanged) {
// If the changed marker lost the race, we just need to mark changed in this method -- all
// other work was done by the dirty marker.
- buildingState.markChanged();
+ getDirtyBuildingState().markChanged();
}
return null;
}
@Override
public synchronized Set<SkyKey> markClean() {
- this.value = buildingState.getLastBuildValue();
+ this.value = getDirtyBuildingState().getLastBuildValue();
Preconditions.checkState(isReady(), "Should be ready when clean: %s", this);
Preconditions.checkState(
- buildingState.depsUnchangedFromLastBuild(getTemporaryDirectDeps()),
+ getDirtyBuildingState().depsUnchangedFromLastBuild(getTemporaryDirectDeps()),
"Direct deps must be the same as those found last build for node to be marked clean: %s",
this);
Preconditions.checkState(isDirty(), this);
@@ -381,8 +385,8 @@
@Override
public synchronized void forceRebuild() {
Preconditions.checkState(
- getTemporaryDirectDeps().numElements() == buildingState.getSignaledDeps(), this);
- buildingState.forceChanged();
+ getTemporaryDirectDeps().numElements() == getDirtyBuildingState().getSignaledDeps(), this);
+ getDirtyBuildingState().forceChanged();
}
@Override
@@ -390,17 +394,17 @@
return lastChangedVersion;
}
- /** @see BuildingState#getDirtyState() */
+ /** @see DirtyBuildingState#getDirtyState() */
@Override
public synchronized NodeEntry.DirtyState getDirtyState() {
- return buildingState.getDirtyState();
+ return getDirtyBuildingState().getDirtyState();
}
- /** @see BuildingState#getNextDirtyDirectDeps() */
+ /** @see DirtyBuildingState#getNextDirtyDirectDeps() */
@Override
public synchronized Collection<SkyKey> getNextDirtyDirectDeps() {
Preconditions.checkState(isReady(), this);
- return buildingState.getNextDirtyDirectDeps();
+ return getDirtyBuildingState().getNextDirtyDirectDeps();
}
@Override
@@ -414,7 +418,8 @@
for (Iterable<SkyKey> group : getTemporaryDirectDeps()) {
result.addAll(group);
}
- result.addAll(buildingState.getAllRemainingDirtyDirectDeps(/*preservePosition=*/ false));
+ result.addAll(
+ getDirtyBuildingState().getAllRemainingDirtyDirectDeps(/*preservePosition=*/ false));
return result.build();
}
}
@@ -422,17 +427,18 @@
@Override
public synchronized Set<SkyKey> getAllRemainingDirtyDirectDeps() {
if (isDirty()) {
- Preconditions.checkState(buildingState.getDirtyState() == DirtyState.REBUILDING, this);
- return buildingState.getAllRemainingDirtyDirectDeps(/*preservePosition=*/ true);
+ Preconditions.checkState(
+ getDirtyBuildingState().getDirtyState() == DirtyState.REBUILDING, this);
+ return getDirtyBuildingState().getAllRemainingDirtyDirectDeps(/*preservePosition=*/ true);
} else {
- Preconditions.checkState(buildingState.evaluating, this);
+ Preconditions.checkState(buildingState.isEvaluating(), this);
return ImmutableSet.of();
}
}
@Override
public synchronized void markRebuilding() {
- buildingState.markRebuilding();
+ getDirtyBuildingState().markRebuilding();
}
@SuppressWarnings("unchecked")
@@ -448,7 +454,7 @@
@Override
public synchronized boolean noDepsLastBuild() {
- return buildingState.noDepsLastBuild();
+ return getDirtyBuildingState().noDepsLastBuild();
}
/**
