Skip to content

Commit

Permalink
Rebrand NodeEntry#DirtyState as LifecycleState to support getting…
Browse files Browse the repository at this point in the history
… the state at any point in the node's lifecycle.

Currently, calling `getDirtyState()` is only supported when `dirtyBuildingState` is not null. To support other states, two new enum values are added: `NOT_YET_EVALUATING` and `DONE`. The enum is accordingly renamed to `LifecycleState`.

PiperOrigin-RevId: 582336688
Change-Id: Icb92f7531ced2c3255a598c8612af574d720c4f6
  • Loading branch information
justinhorvitz authored and copybara-github committed Nov 14, 2023
1 parent 82a469f commit 8f8a4b2
Show file tree
Hide file tree
Showing 11 changed files with 137 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -164,8 +169,7 @@ public final synchronized ImmutableSet<SkyKey> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -229,7 +229,7 @@ private boolean invalidatedByErrorTransience(Collection<SkyKey> 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.

Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ public Version getMaxTransitiveSourceVersion() {
}

@Override
public DirtyState getDirtyState() {
return getDelegate().getDirtyState();
public LifecycleState getLifecycleState() {
return getDelegate().getLifecycleState();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.
*
* <p>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}.
Expand Down Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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);
}
Expand All @@ -136,40 +138,40 @@ 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() {
return signaledDeps > NOT_EVALUATING_SENTINEL;
}

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);
}

/**
* Signals that a child is done.
*
* <p>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
* <p>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(
Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -239,7 +241,7 @@ final DirtyState getDirtyState() {
* <p>See {@link NodeEntry#getNextDirtyDirectDeps}.
*/
final List<SkyKey> getNextDirtyDirectDeps() throws InterruptedException {
checkState(dirtyState == DirtyState.CHECK_DEPENDENCIES, this);
checkState(state == LifecycleState.CHECK_DEPENDENCIES, this);
checkState(dirtyDirectDepIndex < getNumOfGroupsInLastBuildDirectDeps(), this);
return getLastBuildDirectDeps().getDepGroup(dirtyDirectDepIndex++);
}
Expand Down Expand Up @@ -269,8 +271,8 @@ ImmutableSet<SkyKey> 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() {
Expand All @@ -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);
Expand Down
55 changes: 33 additions & 22 deletions src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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. */
Expand Down Expand Up @@ -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)
*/
Expand Down Expand Up @@ -426,8 +435,9 @@ Set<SkyKey> 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.
*
* <p>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.
Expand All @@ -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}.
*
* <p>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.
*
* <p>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
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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}.
*
* <p>Called on a {@link DirtyState#REBUILDING} node when one of the following scenarios is
* <p>Called on a {@link LifecycleState#REBUILDING} node when one of the following scenarios is
* observed:
*
* <ol>
Expand Down Expand Up @@ -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.
*
* <p>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
* <p>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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ private Map<SkyKey, ValueWithMetadata> 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.
Expand Down
Loading

0 comments on commit 8f8a4b2

Please sign in to comment.