diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryNodeEntry.java index d62421129330de..093ae661588233 100644 --- a/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryNodeEntry.java +++ b/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryNodeEntry.java @@ -145,9 +145,14 @@ public final synchronized void addExternalDep() { } @Override - public final synchronized DirtyState getDirtyState() { - checkNotNull(dirtyBuildingState, this); - return dirtyBuildingState.getDirtyState(); + public final synchronized LifecycleState getLifecycleState() { + if (isDone()) { + return LifecycleState.DONE; + } else if (dirtyBuildingState == null) { + return LifecycleState.NOT_YET_EVALUATING; + } else { + return dirtyBuildingState.getLifecycleState(); + } } @Override @@ -164,8 +169,7 @@ public final synchronized ImmutableSet getAllRemainingDirtyDirectDeps() checkNotNull(dirtyBuildingState, this); checkState(dirtyBuildingState.isEvaluating(), "Not evaluating for remaining dirty? %s", this); if (isDirty()) { - DirtyState dirtyState = dirtyBuildingState.getDirtyState(); - checkState(dirtyState == DirtyState.REBUILDING, this); + checkState(dirtyBuildingState.getLifecycleState() == LifecycleState.REBUILDING, this); return dirtyBuildingState.getAllRemainingDirtyDirectDeps(/* preservePosition= */ true); } else { return ImmutableSet.of(); diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java index 867f0d8d20c83d..3008577d7aa873 100644 --- a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java @@ -40,8 +40,8 @@ import com.google.devtools.build.skyframe.EvaluationProgressReceiver.EvaluationState; import com.google.devtools.build.skyframe.EvaluationProgressReceiver.NodeState; import com.google.devtools.build.skyframe.NodeEntry.DependencyState; -import com.google.devtools.build.skyframe.NodeEntry.DirtyState; import com.google.devtools.build.skyframe.NodeEntry.DirtyType; +import com.google.devtools.build.skyframe.NodeEntry.LifecycleState; import com.google.devtools.build.skyframe.QueryableGraph.Reason; import com.google.devtools.build.skyframe.SkyFunction.Environment.SkyKeyComputeState; import com.google.devtools.build.skyframe.SkyFunction.Reset; @@ -143,7 +143,7 @@ abstract class AbstractParallelEvaluator { * If the entry is dirty and not already rebuilding, puts it in a state so that it can rebuild. */ static void maybeMarkRebuilding(NodeEntry entry) { - if (entry.isDirty() && entry.getDirtyState() != DirtyState.REBUILDING) { + if (entry.isDirty() && entry.getLifecycleState() != LifecycleState.REBUILDING) { entry.markRebuilding(); } } @@ -229,7 +229,7 @@ private boolean invalidatedByErrorTransience(Collection depGroup, NodeEn } private DirtyOutcome maybeHandleDirtyNode(NodeEntry nodeEntry) throws InterruptedException { - while (nodeEntry.getDirtyState().equals(DirtyState.CHECK_DEPENDENCIES)) { + while (nodeEntry.getLifecycleState() == LifecycleState.CHECK_DEPENDENCIES) { // Evaluating a dirty node for the first time, and checking its children to see if any // of them have changed. Note that there must be dirty children for this to happen. @@ -341,7 +341,7 @@ private DirtyOutcome maybeHandleDirtyNode(NodeEntry nodeEntry) throws Interrupte // enqueueing and dequeueing), and we also save wall time since the node gets processed // now rather than at some point in the future. } - switch (nodeEntry.getDirtyState()) { + switch (nodeEntry.getLifecycleState()) { case VERIFIED_CLEAN: // No child has a changed value. This node can be marked done and its parents signaled // without any re-evaluation. 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 bd5bda4b9e9bf6..09b5a202ca7232 100644 --- a/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java +++ b/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java @@ -94,8 +94,8 @@ public Version getMaxTransitiveSourceVersion() { } @Override - public DirtyState getDirtyState() { - return getDelegate().getDirtyState(); + public LifecycleState getLifecycleState() { + return getDelegate().getLifecycleState(); } @Override 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 fd9acdc96cf209..66c6af91731c14 100644 --- a/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java +++ b/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java @@ -17,8 +17,8 @@ import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.skyframe.NodeEntry.DirtyState; import com.google.devtools.build.skyframe.NodeEntry.DirtyType; +import com.google.devtools.build.skyframe.NodeEntry.LifecycleState; import java.util.List; import javax.annotation.Nullable; @@ -38,12 +38,14 @@ public abstract class DirtyBuildingState { private static final int NOT_EVALUATING_SENTINEL = -1; /** - * The state of a dirty node. A node is marked dirty in the DirtyBuildingState constructor, and - * goes into either the state {@link DirtyState#CHECK_DEPENDENCIES} or {@link - * DirtyState#NEEDS_REBUILDING}, depending on whether the caller specified that the node was - * itself changed or not. Never null. + * The state of a dirty node. + * + *

Initialized to either {@link LifecycleState#CHECK_DEPENDENCIES} or {@link + * LifecycleState#NEEDS_REBUILDING} depending on the {@link DirtyType} (see {@link + * #initialState}). May take on any {@link LifecycleState} value except {@link + * LifecycleState#NOT_YET_EVALUATING} and {@link LifecycleState#DONE}. */ - private DirtyState dirtyState; + private LifecycleState state; /** * The number of dependencies that are known to be done in a {@link NodeEntry}. @@ -78,8 +80,8 @@ public abstract class DirtyBuildingState { // all of them complete. Therefore, we only need a single bit to track this fact. If the mere // existence of this field turns out to be a significant memory burden, we could change the // implementation by moving to a single-bit approach, and then store that bit as part of the - // dirtyState field, e.g., by adding a REBUILDING_WAITING_FOR_EXTERNAL_DEPS enum value, as this - // can only happen during evaluation. + // state field, e.g., by adding a REBUILDING_WAITING_FOR_EXTERNAL_DEPS enum value, as this can + // only happen during evaluation. private int externalDeps; /** @@ -118,16 +120,16 @@ public abstract class DirtyBuildingState { protected int dirtyDirectDepIndex = 0; protected DirtyBuildingState(DirtyType dirtyType) { - dirtyState = initialDirtyState(dirtyType); + state = initialState(dirtyType); } - private static DirtyState initialDirtyState(DirtyType dirtyType) { + private static LifecycleState initialState(DirtyType dirtyType) { switch (dirtyType) { case DIRTY: - return DirtyState.CHECK_DEPENDENCIES; + return LifecycleState.CHECK_DEPENDENCIES; case CHANGE: case REWIND: - return DirtyState.NEEDS_REBUILDING; + return LifecycleState.NEEDS_REBUILDING; } throw new AssertionError(dirtyType); } @@ -136,16 +138,16 @@ private static DirtyState initialDirtyState(DirtyType dirtyType) { protected abstract boolean isIncremental(); final void markChanged() { - checkState(dirtyState == DirtyState.CHECK_DEPENDENCIES, this); + checkState(state == LifecycleState.CHECK_DEPENDENCIES, this); checkState(dirtyDirectDepIndex == 0, "Unexpected evaluation: %s", this); - dirtyState = DirtyState.NEEDS_REBUILDING; + state = LifecycleState.NEEDS_REBUILDING; } final void forceRebuild(int numTemporaryDirectDeps) { - checkState(dirtyState == DirtyState.CHECK_DEPENDENCIES, this); + checkState(state == LifecycleState.CHECK_DEPENDENCIES, this); checkState(numTemporaryDirectDeps + externalDeps == signaledDeps, this); checkState(getNumOfGroupsInLastBuildDirectDeps() == dirtyDirectDepIndex, this); - dirtyState = DirtyState.REBUILDING; + state = LifecycleState.REBUILDING; } final boolean isEvaluating() { @@ -153,12 +155,12 @@ final boolean isEvaluating() { } final boolean isChanged() { - return dirtyState == DirtyState.NEEDS_REBUILDING || dirtyState == DirtyState.REBUILDING; + return state == LifecycleState.NEEDS_REBUILDING || state == LifecycleState.REBUILDING; } private void checkFinishedBuildingWhenAboutToSetValue() { checkState( - dirtyState == DirtyState.VERIFIED_CLEAN || dirtyState == DirtyState.REBUILDING, + state == LifecycleState.VERIFIED_CLEAN || state == LifecycleState.REBUILDING, "not done building %s", this); } @@ -166,10 +168,10 @@ private void checkFinishedBuildingWhenAboutToSetValue() { /** * Signals that a child is done. * - *

If this node is not yet known to need rebuilding, sets {@link #dirtyState} to {@link - * 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 {@code - * isReady} and comparing {@link #dirtyDirectDepIndex} and {@link + *

If this node is not yet known to need rebuilding, sets {@link #state} to {@link + * LifecycleState#NEEDS_REBUILDING} if the child has changed, and {@link + * LifecycleState#VERIFIED_CLEAN} if the child has not changed and this was the last child to be + * checked (as determined by {@code isReady} and comparing {@link #dirtyDirectDepIndex} and {@link * DirtyBuildingState#getNumOfGroupsInLastBuildDirectDeps()}. */ final void signalDep( @@ -186,13 +188,13 @@ final void signalDep( // childVersion > version.lastEvaluated() means the child has changed since the last evaluation. boolean childChanged = !childVersion.atMost(version.lastEvaluated()); if (childChanged) { - dirtyState = DirtyState.NEEDS_REBUILDING; - } else if (dirtyState == DirtyState.CHECK_DEPENDENCIES + state = LifecycleState.NEEDS_REBUILDING; + } else if (state == LifecycleState.CHECK_DEPENDENCIES && isReady(entry.getNumTemporaryDirectDeps()) && 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; + state = LifecycleState.VERIFIED_CLEAN; } } @@ -228,9 +230,9 @@ final boolean noDepsLastBuild() { return getNumOfGroupsInLastBuildDirectDeps() == 0; } - /** Returns the {@link DirtyState} as documented by {@link NodeEntry#getDirtyState}. */ - final DirtyState getDirtyState() { - return dirtyState; + /** Returns the {@link LifecycleState} as documented by {@link NodeEntry#getLifecycleState}. */ + final LifecycleState getLifecycleState() { + return state; } /** @@ -239,7 +241,7 @@ final DirtyState getDirtyState() { *

See {@link NodeEntry#getNextDirtyDirectDeps}. */ final List getNextDirtyDirectDeps() throws InterruptedException { - checkState(dirtyState == DirtyState.CHECK_DEPENDENCIES, this); + checkState(state == LifecycleState.CHECK_DEPENDENCIES, this); checkState(dirtyDirectDepIndex < getNumOfGroupsInLastBuildDirectDeps(), this); return getLastBuildDirectDeps().getDepGroup(dirtyDirectDepIndex++); } @@ -269,8 +271,8 @@ ImmutableSet getResetDirectDeps() { } protected void markRebuilding() { - checkState(dirtyState == DirtyState.NEEDS_REBUILDING, this); - dirtyState = DirtyState.REBUILDING; + checkState(state == LifecycleState.NEEDS_REBUILDING, this); + state = LifecycleState.REBUILDING; } final void startEvaluating() { @@ -290,7 +292,7 @@ final boolean isReady(int numDirectDeps) { protected MoreObjects.ToStringHelper getStringHelper() { return MoreObjects.toStringHelper(this) - .add("dirtyState", dirtyState) + .add("state", state) .add("signaledDeps", signaledDeps) .add("externalDeps", externalDeps) .add("dirtyDirectDepIndex", dirtyDirectDepIndex); 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 480ba2dd5643e3..532c8a29a98882 100644 --- a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java +++ b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java @@ -56,8 +56,14 @@ enum DependencyState { ALREADY_EVALUATING } - /** Return code for {@link #getDirtyState}. */ - enum DirtyState { + /** Represents the various states in a node's lifecycle. */ + enum LifecycleState { + /** + * The entry has never started evaluating. The next call to {@link #addReverseDepAndCheckIfDone} + * will put the entry into the {@link #NEEDS_REBUILDING} state and return {@link + * DependencyState#NEEDS_SCHEDULING}. + */ + NOT_YET_EVALUATING, /** * The node's dependencies need to be checked to see if it needs to be rebuilt. The dependencies * must be obtained through calls to {@link #getNextDirtyDirectDeps} and checked. @@ -81,6 +87,8 @@ enum DirtyState { NEEDS_REBUILDING, /** A rebuilding is in progress. */ REBUILDING, + /** The node {@link #isDone}. */ + DONE, } /** Ways that a node may be dirtied. */ @@ -383,11 +391,12 @@ DependencyState addReverseDepAndCheckIfDone(@Nullable SkyKey reverseDep) * dirtied and checks its dep on child. child signals parent with version v2. That should not in * and of itself trigger a rebuild, since parent has already rebuilt with child at v2. * - * @param childVersion If this entry {@link #isDirty()} and the last version at which this entry - * was evaluated did not include the changes at version {@code childVersion} (for instance, if + * @param childVersion If this entry {@link #isDirty} and the last version at which this entry was + * evaluated did not include the changes at version {@code childVersion} (for instance, if * {@code childVersion} is after the last version at which this entry was evaluated), then * this entry records that one of its children has changed since it was last evaluated. Thus, - * the next call to {@link #getDirtyState()} will return {@link DirtyState#NEEDS_REBUILDING}. + * the next call to {@link #getLifecycleState} will return {@link + * LifecycleState#NEEDS_REBUILDING}. * @param childForDebugging for use in debugging (can be used to identify specific children that * invalidate this node) */ @@ -426,8 +435,9 @@ Set getRdepsToSignal() { } /** - * Called on a dirty node during {@linkplain DirtyState#CHECK_DEPENDENCIES dependency checking} to - * force the node to be re-evaluated, even if none of its dependencies are known to have changed. + * Called on a dirty node during {@linkplain LifecycleState#CHECK_DEPENDENCIES dependency + * checking} to force the node to be re-evaluated, even if none of its dependencies are known to + * have changed. * *

Used when a caller has reason to believe that re-evaluating may yield a new result, such as * when the prior evaluation encountered a transient error. @@ -453,20 +463,21 @@ default Version getMaxTransitiveSourceVersion() { } /** - * Gets the current state of checking this dirty entry to see if it must be re-evaluated. Must be - * called each time evaluation of a dirty entry starts to find the proper action to perform next, - * as enumerated by {@link NodeEntry.DirtyState}. + * Returns the state of this entry as enumerated by {@link LifecycleState}. + * + *

This method may be called at any time. Returns {@link LifecycleState#DONE} iff the node + * {@link #isDone}. */ @ThreadSafe - NodeEntry.DirtyState getDirtyState(); + LifecycleState getLifecycleState(); /** - * Should only be called if the entry is dirty. During the examination to see if the entry must be - * re-evaluated, this method returns the next group of children to be checked. Callers should have - * already called {@link #getDirtyState} and received a return value of {@link - * DirtyState#CHECK_DEPENDENCIES} before calling this method -- any other return value from {@link - * #getDirtyState} means that this method must not be called, since whether or not the node needs - * to be rebuilt is already known. + * Should only be called if the entry is in the {@link LifecycleState#CHECK_DEPENDENCIES} state. + * During the examination to see if the entry must be re-evaluated, this method returns the next + * group of children to be checked. Callers should have already called {@link #getLifecycleState} + * and received a return value of {@link LifecycleState#CHECK_DEPENDENCIES} before calling this + * method -- any other return value from {@link #getLifecycleState} means that this method must + * not be called, since whether or not the node needs to be rebuilt is already known. * *

Deps are returned in groups. The deps in each group were requested in parallel by the {@code * SkyFunction} last build, meaning independently of the values of any other deps in this group @@ -525,8 +536,8 @@ default Version getMaxTransitiveSourceVersion() { /** * Notifies a node that it is about to be rebuilt. This method can only be called if the node - * {@link DirtyState#NEEDS_REBUILDING}. After this call, this node is ready to be rebuilt (it will - * be in {@link DirtyState#REBUILDING}). + * {@link LifecycleState#NEEDS_REBUILDING}. After this call, this node is ready to be rebuilt (it + * will be in {@link LifecycleState#REBUILDING}). */ void markRebuilding(); @@ -558,7 +569,7 @@ default Version getMaxTransitiveSourceVersion() { * requested during the restarted evaluation of this node. If the graph keeps dependency edges, * however, the temporary direct deps must be accounted for in {@link #getResetDirectDeps}. * - *

Called on a {@link DirtyState#REBUILDING} node when one of the following scenarios is + *

Called on a {@link LifecycleState#REBUILDING} node when one of the following scenarios is * observed: * *

    @@ -587,8 +598,8 @@ default Version getMaxTransitiveSourceVersion() { * this node since it was last done, returns the set of temporary direct deps that were registered * prior to the restart. Otherwise, returns an empty set. * - *

    Called on a {@link DirtyState#REBUILDING} node when it is about to finish evaluating. Used - * to determine which of its {@linkplain #getTemporaryDirectDeps temporary direct deps} have + *

    Called on a {@link LifecycleState#REBUILDING} node when it is about to finish evaluating. + * Used to determine which of its {@linkplain #getTemporaryDirectDeps temporary direct deps} have * already registered a corresponding reverse dep, in order to avoid creating duplicate rdep * edges. * diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java index 3d62f8d9c899a1..d171e07a52a5dd 100644 --- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java @@ -390,7 +390,7 @@ private Map bubbleErrorUp( Preconditions.checkNotNull(parentEntry, "%s %s", errorKey, parent); SkyFunction skyFunction = evaluatorContext.getSkyFunctions().get(parent.functionName()); if (parentEntry.isDirty()) { - switch (parentEntry.getDirtyState()) { + switch (parentEntry.getLifecycleState()) { case CHECK_DEPENDENCIES: // If this value's child was bubbled up to, it did not signal this value, and so we must // manually make it ready to build. diff --git a/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java b/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java index 42280fa7057043..be0bb89c2c45b1 100644 --- a/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java +++ b/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.bugreport.BugReport; import com.google.devtools.build.lib.profiler.AutoProfiler; import com.google.devtools.build.lib.profiler.GoogleAutoProfilerUtils; +import com.google.devtools.build.skyframe.NodeEntry.LifecycleState; import com.google.devtools.build.skyframe.QueryableGraph.Reason; import com.google.devtools.build.skyframe.SkyFunctionEnvironment.UndonePreviouslyRequestedDeps; import java.time.Duration; @@ -202,7 +203,7 @@ private static ErrorInfo checkForCycles(SkyKey root, ParallelEvaluatorContext ev // though nothing changed. int loopCount = 0; Version graphVersion = evaluatorContext.getGraphVersion(); - while (entry.getDirtyState() == NodeEntry.DirtyState.CHECK_DEPENDENCIES) { + while (entry.getLifecycleState() == LifecycleState.CHECK_DEPENDENCIES) { entry.signalDep(graphVersion, null); loopCount++; } @@ -218,7 +219,7 @@ private static ErrorInfo checkForCycles(SkyKey root, ParallelEvaluatorContext ev + ", " + graphPath)); } - if (entry.getDirtyState() == NodeEntry.DirtyState.NEEDS_REBUILDING) { + if (entry.getLifecycleState() == LifecycleState.NEEDS_REBUILDING) { entry.markRebuilding(); } else if (maybeHandleVerifiedCleanNode(key, entry, evaluatorContext, graphPath)) { continue; @@ -318,7 +319,7 @@ private static boolean maybeHandleVerifiedCleanNode( ParallelEvaluatorContext evaluatorContext, List graphPathForDebugging) throws InterruptedException { - if (entry.getDirtyState() != NodeEntry.DirtyState.VERIFIED_CLEAN) { + if (entry.getLifecycleState() != LifecycleState.VERIFIED_CLEAN) { return false; } Set rdeps = entry.markClean().getRdepsToSignal(); diff --git a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java index e582240e645e0b..d2303828726d49 100644 --- a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java @@ -25,8 +25,8 @@ import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.events.Reportable; import com.google.devtools.build.skyframe.NodeEntry.DependencyState; -import com.google.devtools.build.skyframe.NodeEntry.DirtyState; import com.google.devtools.build.skyframe.NodeEntry.DirtyType; +import com.google.devtools.build.skyframe.NodeEntry.LifecycleState; import com.google.devtools.build.skyframe.SkyFunctionException.ReifiedSkyFunctionException; import com.google.devtools.build.skyframe.SkyFunctionException.Transience; import com.google.errorprone.annotations.CanIgnoreReturnValue; @@ -85,8 +85,10 @@ final InMemoryNodeEntry createEntry() { @Test public void entryAtStartOfEvaluation() { InMemoryNodeEntry entry = createEntry(); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.NOT_YET_EVALUATING); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. assertThat(entry.isDone()).isFalse(); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.NEEDS_REBUILDING); assertThat(entry.isReadyToEvaluate()).isTrue(); assertThat(entry.hasUnsignaledDeps()).isFalse(); assertThat(entry.isDirty()).isTrue(); @@ -123,6 +125,7 @@ public void signalEntry() throws InterruptedException { assertThat(entry.hasUnsignaledDeps()).isFalse(); assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion)).isEmpty(); assertThat(entry.isDone()).isTrue(); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.DONE); assertThat(entry.getVersion()).isEqualTo(initialVersion); if (!entry.keepsEdges()) { @@ -188,6 +191,7 @@ public void errorValue() throws InterruptedException { ErrorInfo errorInfo = ErrorInfo.fromException(exception, false); assertThat(setValue(entry, /* value= */ null, errorInfo, initialVersion)).isEmpty(); assertThat(entry.isDone()).isTrue(); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.DONE); assertThat(entry.getValue()).isNull(); assertThat(entry.getErrorInfo()).isEqualTo(errorInfo); } @@ -203,6 +207,7 @@ public void errorAndValue() throws InterruptedException { ErrorInfo errorInfo = ErrorInfo.fromException(exception, false); setValue(entry, new SkyValue() {}, errorInfo, initialVersion); assertThat(entry.isDone()).isTrue(); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.DONE); assertThat(entry.getErrorInfo()).isEqualTo(errorInfo); } @@ -364,7 +369,7 @@ public void resetLifecycle() throws Exception { // Reset clears temporary direct deps. entry.resetEvaluationFromScratch(); - assertThat(entry.getDirtyState()).isEqualTo(DirtyState.REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.REBUILDING); assertThat(entry.getTemporaryDirectDeps()).isEmpty(); assertThat(entry.getTemporaryDirectDeps() instanceof GroupedDeps.WithHashSet) .isEqualTo(isPartialReevaluation); @@ -480,12 +485,12 @@ public void rewindingLifecycle() throws InterruptedException { ? entry.checkIfDoneForDirtyReverseDep(resetParent) : entry.addReverseDepAndCheckIfDone(resetParent); assertThat(dependencyState).isEqualTo(DependencyState.NEEDS_SCHEDULING); - assertThat(entry.getDirtyState()).isEqualTo(DirtyState.NEEDS_REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.NEEDS_REBUILDING); assertThat(entry.isReadyToEvaluate()).isTrue(); assertThat(entry.hasUnsignaledDeps()).isFalse(); assertThat(entry.getTemporaryDirectDeps()).isEmpty(); entry.markRebuilding(); - assertThat(entry.getDirtyState()).isEqualTo(DirtyState.REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.REBUILDING); // Rewound evaluation completes. The parent that initiated rewinding is signalled. assertThat(setValue(entry, new IntegerValue(2), /* errorInfo= */ null, initialVersion)) @@ -509,7 +514,7 @@ public void concurrentRewindingAllowed() throws InterruptedException { assertThat(entry.isDirty()).isTrue(); assertThat(entry.isChanged()).isTrue(); assertThat(entry.isDone()).isFalse(); - assertThat(entry.getDirtyState()).isEqualTo(DirtyState.NEEDS_REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.NEEDS_REBUILDING); } @CanIgnoreReturnValue diff --git a/src/test/java/com/google/devtools/build/skyframe/IncrementalInMemoryNodeEntryTest.java b/src/test/java/com/google/devtools/build/skyframe/IncrementalInMemoryNodeEntryTest.java index af66ef9aa5c0ed..fa60384734f751 100644 --- a/src/test/java/com/google/devtools/build/skyframe/IncrementalInMemoryNodeEntryTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/IncrementalInMemoryNodeEntryTest.java @@ -21,8 +21,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.skyframe.NodeEntry.DependencyState; -import com.google.devtools.build.skyframe.NodeEntry.DirtyState; import com.google.devtools.build.skyframe.NodeEntry.DirtyType; +import com.google.devtools.build.skyframe.NodeEntry.LifecycleState; import com.google.devtools.build.skyframe.SkyFunctionException.ReifiedSkyFunctionException; import com.google.devtools.build.skyframe.SkyFunctionException.Transience; import com.google.testing.junit.testparameterinjector.TestParameter; @@ -73,7 +73,7 @@ public void dirtyLifecycle() throws InterruptedException { .isEqualTo(isPartialReevaluation); SkyKey parent = key("parent"); entry.addReverseDepAndCheckIfDone(parent); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); entry.signalDep(incrementalVersion, dep); @@ -107,7 +107,7 @@ public void changedLifecycle() throws InterruptedException { assertThat(entry.hasUnsignaledDeps()).isFalse(); SkyKey parent = key("parent"); entry.addReverseDepAndCheckIfDone(parent); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.NEEDS_REBUILDING); assertThat(entry.isReadyToEvaluate()).isTrue(); assertThat(entry.hasUnsignaledDeps()).isFalse(); assertThat(entry.getTemporaryDirectDeps()).isEmpty(); @@ -236,7 +236,7 @@ public void forceRebuildAfterTransientError() throws Exception { .isEqualTo(DependencyState.NEEDS_SCHEDULING); assertThat(entry.isReadyToEvaluate()).isTrue(); assertThat(entry.hasUnsignaledDeps()).isFalse(); - assertThat(entry.getDirtyState()).isEqualTo(DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); assertThat(entry.isReadyToEvaluate()).isTrue(); assertThat(entry.hasUnsignaledDeps()).isFalse(); assertThat(entry.getTemporaryDirectDeps()).isEmpty(); @@ -244,11 +244,11 @@ public void forceRebuildAfterTransientError() throws Exception { assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); assertThat(entry.signalDep(initialVersion, dep)).isTrue(); - assertThat(entry.getDirtyState()).isEqualTo(DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(ErrorTransienceValue.KEY); entry.forceRebuild(); - assertThat(entry.getDirtyState()).isEqualTo(DirtyState.REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.REBUILDING); assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, incrementalVersion)) .containsExactly(parent); @@ -310,11 +310,11 @@ public void pruneBeforeBuild() throws InterruptedException { assertThat(entry.hasUnsignaledDeps()).isFalse(); SkyKey parent = key("parent"); entry.addReverseDepAndCheckIfDone(parent); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); entry.signalDep(initialVersion, /* childForDebugging= */ null); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.VERIFIED_CLEAN); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.VERIFIED_CLEAN); assertThat(entry.markClean().getRdepsToSignal()).containsExactly(parent); assertThat(entry.isDone()).isTrue(); assertThat(entry.getVersion()).isEqualTo(initialVersion); @@ -331,12 +331,12 @@ public void pruneAfterBuild() throws InterruptedException { setValue(entry, new IntegerValue(5), /* errorInfo= */ null, initialVersion); entry.markDirty(DirtyType.DIRTY); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); entry.signalDep(incrementalVersion, /* childForDebugging= */ null); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.NEEDS_REBUILDING); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep); entry.markRebuilding(); setValue(entry, new IntegerValue(5), /* errorInfo= */ null, incrementalVersion); @@ -366,11 +366,11 @@ public void noPruneWhenDetailsChange() throws InterruptedException { assertThat(entry.hasUnsignaledDeps()).isFalse(); SkyKey parent = key("parent"); entry.addReverseDepAndCheckIfDone(parent); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); entry.signalDep(incrementalVersion, /* childForDebugging= */ null); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.NEEDS_REBUILDING); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep); ReifiedSkyFunctionException exception = new ReifiedSkyFunctionException( @@ -410,11 +410,11 @@ public void pruneWhenDepGroupReordered() throws InterruptedException { assertThat(entry.isReadyToEvaluate()).isTrue(); assertThat(entry.hasUnsignaledDeps()).isFalse(); entry.addReverseDepAndCheckIfDone(null); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); entry.signalDep(incrementalVersion, /* childForDebugging= */ null); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.NEEDS_REBUILDING); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep); entry.markRebuilding(); entry.addTemporaryDirectDepGroup(ImmutableList.of(dep2InGroup, dep1InGroup)); @@ -442,11 +442,11 @@ public void errorInfoCannotBePruned() throws InterruptedException { setValue(entry, /* value= */ null, errorInfo, initialVersion); entry.markDirty(DirtyType.DIRTY); entry.addReverseDepAndCheckIfDone(null); // Restart evaluation. - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); entry.signalDep(incrementalVersion, /* childForDebugging= */ null); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.NEEDS_REBUILDING); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep); entry.markRebuilding(); setValue(entry, /* value= */ null, errorInfo, incrementalVersion); @@ -471,12 +471,12 @@ public void getDependencyGroup() throws InterruptedException { setValue(entry, /* value= */ new IntegerValue(5), null, initialVersion); entry.markDirty(DirtyType.DIRTY); entry.addReverseDepAndCheckIfDone(null); // Restart evaluation. - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep, dep2); entry.addTemporaryDirectDepGroup(ImmutableList.of(dep, dep2)); entry.signalDep(initialVersion, /* childForDebugging= */ null); entry.signalDep(initialVersion, /* childForDebugging= */ null); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep3); } @@ -503,11 +503,11 @@ public void maintainDependencyGroupAfterRemoval() throws InterruptedException { setValue(entry, null, ErrorInfo.fromException(exception, false), initialVersion); entry.markDirty(DirtyType.DIRTY); entry.addReverseDepAndCheckIfDone(null); // Restart evaluation. - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); entry.signalDep(initialVersion, dep); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep4); } @@ -522,11 +522,11 @@ public void pruneWhenDepsChange() throws InterruptedException { setValue(entry, new IntegerValue(5), /* errorInfo= */ null, initialVersion); entry.markDirty(DirtyType.DIRTY); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); assertThat(entry.signalDep(incrementalVersion, /* childForDebugging= */ null)).isTrue(); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.NEEDS_REBUILDING); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep); entry.markRebuilding(); entry.addSingletonTemporaryDirectDep(key("dep2")); @@ -551,15 +551,15 @@ public void checkDepsOneByOne() throws InterruptedException { setValue(entry, new IntegerValue(5), /* errorInfo= */ null, initialVersion); entry.markDirty(DirtyType.DIRTY); entry.addReverseDepAndCheckIfDone(null); // Start new evaluation. - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); for (int ii = 0; ii < 10; ii++) { assertThat(entry.getNextDirtyDirectDeps()).containsExactly(deps.get(ii)); entry.addSingletonTemporaryDirectDep(deps.get(ii)); assertThat(entry.signalDep(initialVersion, /* childForDebugging= */ null)).isTrue(); if (ii < 9) { - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); } else { - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.VERIFIED_CLEAN); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.VERIFIED_CLEAN); } } } @@ -573,9 +573,9 @@ public void signalOnlyNewParents() throws InterruptedException { entry.markDirty(DirtyType.CHANGE); SkyKey newParent = key("new parent"); entry.addReverseDepAndCheckIfDone(newParent); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.NEEDS_REBUILDING); entry.markRebuilding(); - assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.REBUILDING); assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, incrementalVersion)) .containsExactly(newParent); } @@ -747,7 +747,7 @@ public void resetOnDirtyNode(@TestParameter boolean valueChanges) throws Excepti // Reset clears temporary direct deps. entry.resetEvaluationFromScratch(); - assertThat(entry.getDirtyState()).isEqualTo(DirtyState.REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.REBUILDING); assertThat(entry.getTemporaryDirectDeps()).isEmpty(); assertThat(entry.getTemporaryDirectDeps() instanceof GroupedDeps.WithHashSet) .isEqualTo(isPartialReevaluation); @@ -826,9 +826,9 @@ public void rewindOnIncrementalBuild(@TestParameter boolean valueChanges) // Parent declares dep again after resetting. assertThat(entry.checkIfDoneForDirtyReverseDep(resetDirtyParent)) .isEqualTo(DependencyState.NEEDS_SCHEDULING); - assertThat(entry.getDirtyState()).isEqualTo(DirtyState.NEEDS_REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.NEEDS_REBUILDING); entry.markRebuilding(); - assertThat(entry.getDirtyState()).isEqualTo(DirtyState.REBUILDING); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.REBUILDING); // Rdep removed after rewinding was initiated. entry.removeReverseDep(lateOldParent); @@ -884,9 +884,11 @@ public void rewindOnDirtyNodeIgnored(@TestParameter boolean valueChanges) // Start of incremental build. entry.markDirty(DirtyType.DIRTY); entry.addReverseDepAndCheckIfDone(null); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); - // Attempt to rewind node while it is dirty. + // Attempt to rewind node while it is dirty. Its lifecycle state does not change. entry.markDirty(DirtyType.REWIND); + assertThat(entry.getLifecycleState()).isEqualTo(LifecycleState.CHECK_DEPENDENCIES); // Add back same dep. entry.addSingletonTemporaryDirectDep(dep); diff --git a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java index e4dfcc438fc55c..9493ab736d1b94 100644 --- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java @@ -4997,7 +4997,7 @@ public void shutDownBuildOnCachedError_Done() throws Exception { if (!synchronizeThreads.get()) { return; } - if (type == EventType.GET_DIRTY_STATE && key.equals(failingKey)) { + if (type == EventType.GET_LIFECYCLE_STATE && key.equals(failingKey)) { // Wait for the build to abort or for the other node to incorrectly build. try { assertThat(slowBuilt.await(TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS)) 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 92bb5e5b9e5cc0..2c7da6d7f030c5 100644 --- a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java +++ b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java @@ -156,7 +156,7 @@ public enum EventType { MARK_DIRTY, MARK_CLEAN, IS_CHANGED, - GET_DIRTY_STATE, + GET_LIFECYCLE_STATE, GET_VALUE_WITH_METADATA, IS_DIRTY, IS_READY, @@ -305,11 +305,11 @@ public boolean isReadyToEvaluate() { } @Override - public DirtyState getDirtyState() { - graphListener.accept(myKey, EventType.GET_DIRTY_STATE, Order.BEFORE, this); - DirtyState dirtyState = super.getDirtyState(); - graphListener.accept(myKey, EventType.GET_DIRTY_STATE, Order.AFTER, dirtyState); - return dirtyState; + public LifecycleState getLifecycleState() { + graphListener.accept(myKey, EventType.GET_LIFECYCLE_STATE, Order.BEFORE, this); + LifecycleState lifecycleState = super.getLifecycleState(); + graphListener.accept(myKey, EventType.GET_LIFECYCLE_STATE, Order.AFTER, lifecycleState); + return lifecycleState; } @Override