Refactor DirtyBuildingState to allow custom implementation on storing the
dependencies used in last build
--
PiperOrigin-RevId: 146169454
MOS_MIGRATED_REVID=146169454
diff --git a/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
index 2152fb3..f0003f7 100644
--- a/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
@@ -106,17 +106,17 @@
}
@Override
- public Collection<SkyKey> getNextDirtyDirectDeps() {
+ public Collection<SkyKey> getNextDirtyDirectDeps() throws InterruptedException {
return getDelegate().getNextDirtyDirectDeps();
}
@Override
- public Iterable<SkyKey> getAllDirectDepsForIncompleteNode() {
+ public Iterable<SkyKey> getAllDirectDepsForIncompleteNode() throws InterruptedException {
return getDelegate().getAllDirectDepsForIncompleteNode();
}
@Override
- public Set<SkyKey> getAllRemainingDirtyDirectDeps() {
+ public Set<SkyKey> getAllRemainingDirtyDirectDeps() throws InterruptedException {
return getDelegate().getAllRemainingDirtyDirectDeps();
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java b/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java
index a476275..2c34feb 100644
--- a/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java
+++ b/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java
@@ -44,7 +44,14 @@
* it means that this node is being built for the first time. See {@link
* InMemoryNodeEntry#directDeps} for more on dependency group storage.
*/
- protected final GroupedList<SkyKey> lastBuildDirectDeps;
+ protected abstract GroupedList<SkyKey> getLastBuildDirectDeps() throws InterruptedException;
+
+ /**
+ * The number of groups of the dependencies requested last time when the node was built.
+ *
+ * <p>Getting the number of last-built dependencies should not throw {@link InterruptedException}.
+ */
+ protected abstract int getNumOfGroupsInLastBuildDirectDeps();
/** The value of the node the last time it was built. */
protected abstract SkyValue getLastBuildValue() throws InterruptedException;
@@ -55,12 +62,7 @@
*/
private int dirtyDirectDepIndex = -1;
- protected DirtyBuildingState(boolean isChanged, GroupedList<SkyKey> lastBuildDirectDeps) {
- this.lastBuildDirectDeps = lastBuildDirectDeps;
- Preconditions.checkState(
- isChanged || !this.lastBuildDirectDeps.isEmpty(),
- "%s is being marked dirty, not changed, but has no children that could have dirtied it",
- this);
+ protected DirtyBuildingState(boolean isChanged) {
dirtyState = isChanged ? DirtyState.NEEDS_REBUILDING : DirtyState.CHECK_DEPENDENCIES;
// We need to iterate through the deps to see if they have changed, or to remove them if one
// has. Initialize the iterating index.
@@ -69,7 +71,7 @@
static BuildingState create(
boolean isChanged, GroupedList<SkyKey> lastBuildDirectDeps, SkyValue lastBuildValue) {
- return new DirtyBuildingStateWithValue(isChanged, lastBuildDirectDeps, lastBuildValue);
+ return new FullDirtyBuildingState(isChanged, lastBuildDirectDeps, lastBuildValue);
}
final void markChanged() {
@@ -83,7 +85,7 @@
Preconditions.checkState(isDirty(), this);
Preconditions.checkState(!isChanged(), this);
Preconditions.checkState(isEvaluating(), this);
- Preconditions.checkState(lastBuildDirectDeps.listSize() == dirtyDirectDepIndex, this);
+ Preconditions.checkState(getNumOfGroupsInLastBuildDirectDeps() == dirtyDirectDepIndex, this);
dirtyState = DirtyState.REBUILDING;
}
@@ -117,7 +119,7 @@
* DirtyState#NEEDS_REBUILDING} if the child has changed, and {@link DirtyState#VERIFIED_CLEAN} if
* the child has not changed and this was the last child to be checked (as determined by {@link
* #isReady} and comparing {@link #dirtyDirectDepIndex} and {@link
- * DirtyBuildingState#lastBuildDirectDeps#listSize}.
+ * DirtyBuildingState#getNumOfGroupsInLastBuildDirectDeps()}.
*/
@Override
final void signalDepInternal(boolean childChanged, int numDirectDeps) {
@@ -128,7 +130,7 @@
dirtyState = DirtyState.NEEDS_REBUILDING;
} else if (dirtyState == DirtyState.CHECK_DEPENDENCIES
&& isReady(numDirectDeps)
- && lastBuildDirectDeps.listSize() == dirtyDirectDepIndex) {
+ && getNumOfGroupsInLastBuildDirectDeps() == dirtyDirectDepIndex) {
// No other dep already marked this as NEEDS_REBUILDING, no deps outstanding, and this was
// the last block of deps to be checked.
dirtyState = DirtyState.VERIFIED_CLEAN;
@@ -152,13 +154,14 @@
* Returns true if the deps requested during this evaluation ({@code directDeps}) are exactly
* those requested the last time this node was built, in the same order.
*/
- final boolean depsUnchangedFromLastBuild(GroupedList<SkyKey> directDeps) {
+ final boolean depsUnchangedFromLastBuild(GroupedList<SkyKey> directDeps)
+ throws InterruptedException {
checkFinishedBuildingWhenAboutToSetValue();
- return lastBuildDirectDeps.equals(directDeps);
+ return getLastBuildDirectDeps().equals(directDeps);
}
final boolean noDepsLastBuild() {
- return lastBuildDirectDeps.isEmpty();
+ return getNumOfGroupsInLastBuildDirectDeps() == 0;
}
/**
@@ -179,12 +182,12 @@
*
* <p>See {@link NodeEntry#getNextDirtyDirectDeps}.
*/
- final Collection<SkyKey> getNextDirtyDirectDeps() {
+ final Collection<SkyKey> getNextDirtyDirectDeps() throws InterruptedException {
Preconditions.checkState(isDirty(), this);
Preconditions.checkState(dirtyState == DirtyState.CHECK_DEPENDENCIES, this);
Preconditions.checkState(isEvaluating(), this);
- Preconditions.checkState(dirtyDirectDepIndex < lastBuildDirectDeps.listSize(), this);
- return lastBuildDirectDeps.get(dirtyDirectDepIndex++);
+ Preconditions.checkState(dirtyDirectDepIndex < getNumOfGroupsInLastBuildDirectDeps(), this);
+ return getLastBuildDirectDeps().get(dirtyDirectDepIndex++);
}
/**
@@ -192,15 +195,15 @@
* true, this method is non-mutating. If {@code preservePosition} is false, the caller must
* process the returned set, and so subsequent calls to this method will return the empty set.
*/
- Set<SkyKey> getAllRemainingDirtyDirectDeps(boolean preservePosition) {
+ Set<SkyKey> getAllRemainingDirtyDirectDeps(boolean preservePosition) throws InterruptedException {
Preconditions.checkState(isDirty(), this);
ImmutableSet.Builder<SkyKey> result = ImmutableSet.builder();
- for (int ind = dirtyDirectDepIndex; ind < lastBuildDirectDeps.listSize(); ind++) {
- result.addAll(lastBuildDirectDeps.get(ind));
+ for (int ind = dirtyDirectDepIndex; ind < getNumOfGroupsInLastBuildDirectDeps(); ind++) {
+ result.addAll(getLastBuildDirectDeps().get(ind));
}
if (!preservePosition) {
- dirtyDirectDepIndex = lastBuildDirectDeps.listSize();
+ dirtyDirectDepIndex = getNumOfGroupsInLastBuildDirectDeps();
}
return result.build();
}
@@ -214,16 +217,21 @@
protected MoreObjects.ToStringHelper getStringHelper() {
return super.getStringHelper()
.add("dirtyState", dirtyState)
- .add("lastBuildDirectDeps", lastBuildDirectDeps)
.add("dirtyDirectDepIndex", dirtyDirectDepIndex);
}
- private static class DirtyBuildingStateWithValue extends DirtyBuildingState {
+ private static class FullDirtyBuildingState extends DirtyBuildingState {
+ private final GroupedList<SkyKey> lastBuildDirectDeps;
private final SkyValue lastBuildValue;
- private DirtyBuildingStateWithValue(
+ private FullDirtyBuildingState(
boolean isChanged, GroupedList<SkyKey> lastBuildDirectDeps, SkyValue lastBuildValue) {
- super(isChanged, lastBuildDirectDeps);
+ super(isChanged);
+ this.lastBuildDirectDeps = lastBuildDirectDeps;
+ Preconditions.checkState(
+ isChanged || getNumOfGroupsInLastBuildDirectDeps() > 0,
+ "%s is being marked dirty, not changed, but has no children that could have dirtied it",
+ this);
this.lastBuildValue = lastBuildValue;
}
@@ -233,8 +241,20 @@
}
@Override
+ protected GroupedList<SkyKey> getLastBuildDirectDeps() throws InterruptedException {
+ return lastBuildDirectDeps;
+ }
+
+ @Override
+ protected int getNumOfGroupsInLastBuildDirectDeps() {
+ return lastBuildDirectDeps.listSize();
+ }
+
+ @Override
protected MoreObjects.ToStringHelper getStringHelper() {
- return super.getStringHelper().add("lastBuildValue", lastBuildValue);
+ return super.getStringHelper()
+ .add("lastBuildDirectDeps", lastBuildDirectDeps)
+ .add("lastBuildValue", lastBuildValue);
}
}
}
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 5d8dbb3..fefee27 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
@@ -405,13 +405,14 @@
/** @see DirtyBuildingState#getNextDirtyDirectDeps() */
@Override
- public synchronized Collection<SkyKey> getNextDirtyDirectDeps() {
+ public synchronized Collection<SkyKey> getNextDirtyDirectDeps() throws InterruptedException {
Preconditions.checkState(isReady(), this);
return getDirtyBuildingState().getNextDirtyDirectDeps();
}
@Override
- public synchronized Iterable<SkyKey> getAllDirectDepsForIncompleteNode() {
+ public synchronized Iterable<SkyKey> getAllDirectDepsForIncompleteNode()
+ throws InterruptedException {
Preconditions.checkState(!isDone(), this);
if (!isDirty()) {
return Iterables.concat(getTemporaryDirectDeps());
@@ -428,7 +429,7 @@
}
@Override
- public synchronized Set<SkyKey> getAllRemainingDirtyDirectDeps() {
+ public synchronized Set<SkyKey> getAllRemainingDirtyDirectDeps() throws InterruptedException {
if (isDirty()) {
Preconditions.checkState(
getDirtyBuildingState().getDirtyState() == DirtyState.REBUILDING, this);
diff --git a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
index b86d471..d7f7fcf 100644
--- a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
@@ -284,16 +284,16 @@
* @see DirtyBuildingState#getNextDirtyDirectDeps()
*/
@ThreadSafe
- Collection<SkyKey> getNextDirtyDirectDeps();
+ Collection<SkyKey> getNextDirtyDirectDeps() throws InterruptedException;
/**
* Returns all deps of a node that has not yet finished evaluating. In other words, if a node has
* a reverse dep on this node, its key will be in the returned set here. If this node was freshly
* created, this is just any elements that were added using {@link #addTemporaryDirectDeps} (so it
* is the same as {@link #getTemporaryDirectDeps}). If this node is marked dirty, this includes
- * all the elements that would have been returned by successive calls to
- * {@link #getNextDirtyDirectDeps} (or, equivalently, one call to
- * {@link #getAllRemainingDirtyDirectDeps}).
+ * all the elements that would have been returned by successive calls to {@link
+ * #getNextDirtyDirectDeps} (or, equivalently, one call to {@link
+ * #getAllRemainingDirtyDirectDeps}).
*
* <p>This method should only be called when this node is about to be deleted after an aborted
* evaluation. After such an evaluation, any nodes that did not finish evaluating are deleted, as
@@ -305,7 +305,7 @@
* <p>This method must not be called twice: the next thing done to this node after this method is
* called should be the removal of the node from the graph.
*/
- Iterable<SkyKey> getAllDirectDepsForIncompleteNode();
+ Iterable<SkyKey> getAllDirectDepsForIncompleteNode() throws InterruptedException;
/**
* If an entry {@link #isDirty}, returns all direct deps that were present last build, but have
@@ -319,10 +319,10 @@
* started evaluation.
*
* <p>This method does not mutate the entry. In particular, multiple calls to this method will
- * always produce the same result until the entry finishes evaluation. Contrast with
- * {@link #getAllDirectDepsForIncompleteNode}.
+ * always produce the same result until the entry finishes evaluation. Contrast with {@link
+ * #getAllDirectDepsForIncompleteNode}.
*/
- Set<SkyKey> getAllRemainingDirtyDirectDeps();
+ Set<SkyKey> getAllRemainingDirtyDirectDeps() throws InterruptedException;
/**
* Notifies a node that it is about to be rebuilt. This method can only be called if the node
diff --git a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
index 559b3cf..18f4739 100644
--- a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
+++ b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
@@ -306,7 +306,7 @@
}
@Override
- public Iterable<SkyKey> getAllDirectDepsForIncompleteNode() {
+ public Iterable<SkyKey> getAllDirectDepsForIncompleteNode() throws InterruptedException {
graphListener.accept(
myKey, EventType.GET_ALL_DIRECT_DEPS_FOR_INCOMPLETE_NODE, Order.BEFORE, this);
return super.getAllDirectDepsForIncompleteNode();
