Permit marking dirty/changed a node more than once
This functionality will be useful for node restarting. More than one
parent node may request the restart of a shared child node, and this
should not fail.
Instead, the returned MarkedDirtyResult indicates whether the dirtying
was redundant, and the calling code can assert on that.
RELNOTES: None.
PiperOrigin-RevId: 201005663
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 00f52dd..bb6a2d1 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
@@ -475,21 +475,19 @@
DirtyBuildingState.create(isChanged, GroupedList.<SkyKey>create(directDeps), value);
value = null;
directDeps = null;
- return new MarkedDirtyResult(ReverseDepsUtility.getReverseDeps(this));
+ return new FromCleanMarkedDirtyResult(ReverseDepsUtility.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
- // this node changed twice. The end result of racing markers must be a changed node, since one
- // of the markers is trying to mark the node changed.
- Preconditions.checkState(isChanged != isChanged(),
- "Cannot mark node dirty twice or changed twice: %s", this);
+
Preconditions.checkState(value == null, "Value should have been reset already %s", this);
- 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.
- getDirtyBuildingState().markChanged();
+ if (isChanged != isChanged()) {
+ if (isChanged) {
+ getDirtyBuildingState().markChanged();
+ }
+ // If !isChanged, then this call made no changes to the node, but redundancy is a property of
+ // the sequence of markDirty calls, not their effects.
+ return FromDirtyMarkedDirtyResult.NOT_REDUNDANT;
}
- return null;
+ return FromDirtyMarkedDirtyResult.REDUNDANT;
}
@Override
diff --git a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
index ceae771..9d2d811 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
@@ -207,7 +207,7 @@
}
}
- public static class DirtyingInvalidationState extends InvalidationState {
+ static class DirtyingInvalidationState extends InvalidationState {
public DirtyingInvalidationState() {
super(InvalidationType.CHANGED);
}
@@ -477,7 +477,7 @@
// method.
// Any exception thrown should be unrecoverable.
// This entry remains in the graph in this dirty state until it is re-evaluated.
- MarkedDirtyResult markedDirtyResult = null;
+ MarkedDirtyResult markedDirtyResult;
try {
markedDirtyResult = entry.markDirty(isChanged);
} catch (InterruptedException e) {
@@ -487,8 +487,13 @@
// visitation, so we can resume next time.
return;
}
- if (markedDirtyResult == null) {
- // Another thread has already dirtied this node. Don't do anything in this thread.
+ Preconditions.checkState(
+ !markedDirtyResult.wasCallRedundant(),
+ "Node unexpectedly marked dirty or changed twice: %s",
+ entry);
+ if (!markedDirtyResult.wasClean()) {
+ // Another thread has already handled this node's rdeps. Don't do anything in this
+ // thread.
if (supportInterruptions) {
pendingVisitations.remove(Pair.of(key, invalidationType));
}
@@ -496,7 +501,10 @@
}
// Propagate dirtiness upwards and mark this node dirty/changed. Reverse deps should
// only be marked dirty (because only a dependency of theirs has changed).
- visit(markedDirtyResult.getReverseDepsUnsafe(), InvalidationType.DIRTIED, key);
+ visit(
+ markedDirtyResult.getReverseDepsUnsafeIfWasClean(),
+ InvalidationType.DIRTIED,
+ key);
progressReceiver.invalidated(key,
EvaluationProgressReceiver.InvalidationState.DIRTY);
diff --git a/src/main/java/com/google/devtools/build/skyframe/ThinNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/ThinNodeEntry.java
index eef5487..c063ce6 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ThinNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ThinNodeEntry.java
@@ -15,11 +15,10 @@
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
-import javax.annotation.Nullable;
/**
* A node in the graph without the means to access its value. All operations on this class are
- * thread-safe. Note, however, the warning on the return value of {@link #markDirty}.
+ * thread-safe (note, however, the warning on the return value of {@link #markDirty}).
*
* <p>This interface is public only for the benefit of alternative graph implementations outside of
* the package.
@@ -45,45 +44,101 @@
boolean isChanged();
/**
- * Marks this node dirty, or changed if {@code isChanged} is true. The node is put in the
- * just-created state. It will be re-evaluated if necessary during the evaluation phase, but if it
- * has not changed, it will not force a re-evaluation of its parents.
+ * Marks this node dirty, or changed if {@code isChanged} is true.
*
- * <p>{@code markDirty(b)} must not be called on an undone node if {@code isChanged() == b}. It is
- * the caller's responsibility to ensure that this does not happen. Calling {@code
- * markDirty(false)} when {@code isChanged() == true} has no effect. The idea here is that the
- * caller will only ever want to call {@code markDirty()} a second time if a transition from a
- * dirty-unchanged state to a dirty-changed state is required.
+ * <p>A dirty node P is re-evaluated during the evaluation phase if it's requested and directly
+ * depends on some node C whose value changed since the last evaluation of P. If it's requested
+ * and there is no such node C, P is marked clean.
*
- * @return A {@link ThinNodeEntry.MarkedDirtyResult} if the node was previously clean, and {@code
- * null} if it was already dirty. If it was already dirty, the caller should abort its
- * handling of this node, since another thread is already dealing with it. Note the warning on
- * {@link ThinNodeEntry.MarkedDirtyResult} regarding the collection it provides.
+ * <p>A changed node is re-evaluated during the evaluation phase if it's requested (regardless of
+ * the state of its dependencies).
+ *
+ * @return a {@link MarkedDirtyResult} indicating whether the call was redundant and which may
+ * include the node's reverse deps
*/
- @Nullable
@ThreadSafe
MarkedDirtyResult markDirty(boolean isChanged) throws InterruptedException;
- /**
- * Returned by {@link #markDirty} if that call changed the node from clean to dirty. Contains an
- * iterable of the node's reverse deps for efficiency, because the single use case for {@link
- * #markDirty} is during invalidation, and if such an invalidation call wins, the invalidator
- * must immediately afterwards schedule the invalidation of the node's reverse deps.
- *
- * <p>Warning: {@link #getReverseDepsUnsafe()} may return a live view of the reverse deps
- * collection of the marked-dirty node. The consumer of this data must be careful only to
- * iterate over and consume its values while that collection is guaranteed not to change. This
- * is true during invalidation, because reverse deps don't change during invalidation.
- */
- class MarkedDirtyResult {
+ /** Returned by {@link #markDirty}. */
+ interface MarkedDirtyResult {
+
+ /** Returns true iff the node was clean prior to the {@link #markDirty} call. */
+ boolean wasClean();
+
+ /**
+ * Returns true iff the call to {@link #markDirty} was the same as some previous call to {@link
+ * #markDirty} (i.e., sharing the same {@code isChanged} parameter value) since the last time
+ * the node was clean.
+ *
+ * <p>More specifically, this returns true iff the call was {@code n.markDirty(b)} and prior to
+ * the call {@code n.isDirty() && n.isChanged() == b}).
+ */
+ boolean wasCallRedundant();
+
+ /**
+ * If {@code wasClean()}, this returns an iterable of the node's reverse deps for efficiency,
+ * because the {@link #markDirty} caller may be doing graph invalidation, and after dirtying a
+ * node, the invalidation process may want to dirty the node's reverse deps.
+ *
+ * <p>If {@code !wasClean()}, this must not be called. It will throw {@link
+ * IllegalStateException}.
+ *
+ * <p>Warning: the returned iterable may be a live view of the reverse deps collection of the
+ * marked-dirty node. The consumer of this data must be careful only to iterate over and consume
+ * its values while that collection is guaranteed not to change. This is true during
+ * invalidation, because reverse deps don't change during invalidation.
+ */
+ Iterable<SkyKey> getReverseDepsUnsafeIfWasClean();
+ }
+
+ /** A {@link MarkedDirtyResult} returned when {@link #markDirty} is called on a clean node. */
+ class FromCleanMarkedDirtyResult implements MarkedDirtyResult {
private final Iterable<SkyKey> reverseDepsUnsafe;
- public MarkedDirtyResult(Iterable<SkyKey> reverseDepsUnsafe) {
+ public FromCleanMarkedDirtyResult(Iterable<SkyKey> reverseDepsUnsafe) {
this.reverseDepsUnsafe = Preconditions.checkNotNull(reverseDepsUnsafe);
}
- public Iterable<SkyKey> getReverseDepsUnsafe() {
+ @Override
+ public boolean wasClean() {
+ return true;
+ }
+
+ @Override
+ public boolean wasCallRedundant() {
+ return false;
+ }
+
+ @Override
+ public Iterable<SkyKey> getReverseDepsUnsafeIfWasClean() {
return reverseDepsUnsafe;
}
}
+
+ /** A {@link MarkedDirtyResult} returned when {@link #markDirty} is called on a dirty node. */
+ class FromDirtyMarkedDirtyResult implements MarkedDirtyResult {
+ static final FromDirtyMarkedDirtyResult REDUNDANT = new FromDirtyMarkedDirtyResult(true);
+ static final FromDirtyMarkedDirtyResult NOT_REDUNDANT = new FromDirtyMarkedDirtyResult(false);
+
+ private final boolean redundant;
+
+ private FromDirtyMarkedDirtyResult(boolean redundant) {
+ this.redundant = redundant;
+ }
+
+ @Override
+ public boolean wasClean() {
+ return false;
+ }
+
+ @Override
+ public boolean wasCallRedundant() {
+ return redundant;
+ }
+
+ @Override
+ public Iterable<SkyKey> getReverseDepsUnsafeIfWasClean() {
+ throw new IllegalStateException();
+ }
+ }
}
diff --git a/src/test/java/com/google/devtools/build/skyframe/GraphTest.java b/src/test/java/com/google/devtools/build/skyframe/GraphTest.java
index 378f58c..1f843a7 100644
--- a/src/test/java/com/google/devtools/build/skyframe/GraphTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/GraphTest.java
@@ -222,7 +222,7 @@
graph = getGraph(getNextVersion(startingVersion));
NodeEntry sameEntry = Preconditions.checkNotNull(graph.get(null, Reason.OTHER, key));
// Mark the node as dirty again and check that the reverse deps have been preserved.
- sameEntry.markDirty(true);
+ assertThat(sameEntry.markDirty(true).wasCallRedundant()).isFalse();
startEvaluation(sameEntry);
sameEntry.markRebuilding();
sameEntry.setValue(new StringValue("foo2"), getNextVersion(startingVersion));
@@ -368,7 +368,7 @@
throw new IllegalStateException(e);
}
try {
- entry.markDirty(true);
+ assertThat(entry.markDirty(true).wasCallRedundant()).isFalse();
// Make some changes, like adding a dep and rdep.
entry.addReverseDepAndCheckIfDone(key("rdep"));
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 b16206c..4712cfd 100644
--- a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
@@ -30,6 +30,7 @@
import com.google.devtools.build.skyframe.NodeEntry.DependencyState;
import com.google.devtools.build.skyframe.SkyFunctionException.ReifiedSkyFunctionException;
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
+import com.google.devtools.build.skyframe.ThinNodeEntry.MarkedDirtyResult;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
@@ -183,7 +184,9 @@
setValue(entry, new SkyValue() {}, /*errorInfo=*/null, /*graphVersion=*/0L);
assertThat(entry.isDirty()).isFalse();
assertThat(entry.isDone()).isTrue();
- entry.markDirty(/*isChanged=*/false);
+ MarkedDirtyResult markedDirtyResult = entry.markDirty(/*isChanged=*/false);
+ assertThat(markedDirtyResult.wasCallRedundant()).isFalse();
+ assertThat(markedDirtyResult.wasClean()).isTrue();
assertThat(entry.isDirty()).isTrue();
assertThat(entry.isChanged()).isFalse();
assertThat(entry.isDone()).isFalse();
@@ -215,7 +218,9 @@
setValue(entry, new SkyValue() {}, /*errorInfo=*/null, /*graphVersion=*/0L);
assertThat(entry.isDirty()).isFalse();
assertThat(entry.isDone()).isTrue();
- entry.markDirty(/*isChanged=*/true);
+ MarkedDirtyResult markedDirtyResult = entry.markDirty(/*isChanged=*/true);
+ assertThat(markedDirtyResult.wasCallRedundant()).isFalse();
+ assertThat(markedDirtyResult.wasClean()).isTrue();
assertThat(entry.isDirty()).isTrue();
assertThat(entry.isChanged()).isTrue();
assertThat(entry.isDone()).isFalse();
@@ -243,11 +248,15 @@
setValue(entry, new SkyValue() {}, /*errorInfo=*/null, /*graphVersion=*/0L);
assertThat(entry.isDirty()).isFalse();
assertThat(entry.isDone()).isTrue();
- entry.markDirty(/*isChanged=*/false);
+ MarkedDirtyResult firstResult = entry.markDirty(/*isChanged=*/false);
+ assertThat(firstResult.wasCallRedundant()).isFalse();
+ assertThat(firstResult.wasClean()).isTrue();
assertThat(entry.isDirty()).isTrue();
assertThat(entry.isChanged()).isFalse();
assertThat(entry.isDone()).isFalse();
- entry.markDirty(/*isChanged=*/true);
+ MarkedDirtyResult secondResult = entry.markDirty(/*isChanged=*/true);
+ assertThat(secondResult.wasCallRedundant()).isFalse();
+ assertThat(secondResult.wasClean()).isFalse();
assertThat(entry.isDirty()).isTrue();
assertThat(entry.isChanged()).isTrue();
assertThat(entry.isDone()).isFalse();
@@ -267,11 +276,15 @@
setValue(entry, new SkyValue() {}, /*errorInfo=*/null, /*graphVersion=*/0L);
assertThat(entry.isDirty()).isFalse();
assertThat(entry.isDone()).isTrue();
- entry.markDirty(/*isChanged=*/true);
+ MarkedDirtyResult firstResult = entry.markDirty(/*isChanged=*/true);
+ assertThat(firstResult.wasCallRedundant()).isFalse();
+ assertThat(firstResult.wasClean()).isTrue();
assertThat(entry.isDirty()).isTrue();
assertThat(entry.isChanged()).isTrue();
assertThat(entry.isDone()).isFalse();
- entry.markDirty(/*isChanged=*/false);
+ MarkedDirtyResult secondResult = entry.markDirty(/*isChanged=*/false);
+ assertThat(secondResult.wasCallRedundant()).isFalse();
+ assertThat(secondResult.wasClean()).isFalse();
assertThat(entry.isDirty()).isTrue();
assertThat(entry.isChanged()).isTrue();
assertThat(entry.isDone()).isFalse();
@@ -282,35 +295,60 @@
}
@Test
- public void crashOnTwiceMarkedChanged() throws InterruptedException {
+ public void markChangedTwice() throws InterruptedException {
NodeEntry entry = new InMemoryNodeEntry();
entry.addReverseDepAndCheckIfDone(null); // Start evaluation.
- setValue(entry, new SkyValue() {}, /*errorInfo=*/null, /*graphVersion=*/0L);
+
+ setValue(entry, new SkyValue() {}, /*errorInfo=*/ null, /*graphVersion=*/ 0L);
assertThat(entry.isDirty()).isFalse();
+ assertThat(entry.isChanged()).isFalse();
assertThat(entry.isDone()).isTrue();
- entry.markDirty(/*isChanged=*/true);
- try {
- entry.markDirty(/*isChanged=*/true);
- fail("Cannot mark entry changed twice");
- } catch (IllegalStateException e) {
- // Expected.
- }
+
+ MarkedDirtyResult firstResult = entry.markDirty(/*isChanged=*/ true);
+ assertThat(firstResult.getReverseDepsUnsafeIfWasClean()).isNotNull();
+ assertThat(firstResult.wasCallRedundant()).isFalse();
+ assertThat(firstResult.wasClean()).isTrue();
+ assertThat(entry.isDirty()).isTrue();
+ assertThat(entry.isChanged()).isTrue();
+ assertThat(entry.isDone()).isFalse();
+
+ MarkedDirtyResult secondResult = entry.markDirty(/*isChanged=*/ true);
+ assertThat(secondResult.wasClean()).isFalse();
+ assertThat(secondResult.wasCallRedundant()).isTrue();
+ assertThat(entry.isDirty()).isTrue();
+ assertThat(entry.isChanged()).isTrue();
+ assertThat(entry.isDone()).isFalse();
}
@Test
- public void crashOnTwiceMarkedDirty() throws InterruptedException {
+ public void markDirtyTwice() throws InterruptedException {
NodeEntry entry = new InMemoryNodeEntry();
entry.addReverseDepAndCheckIfDone(null); // Start evaluation.
+
+ // To successfully mark a node dirty (but not changed), that node must have at least one
+ // dependency. The node sanity-checks its deps while being marked dirty.
addTemporaryDirectDep(entry, key("dep"));
entry.signalDep();
- setValue(entry, new SkyValue() {}, /*errorInfo=*/null, /*graphVersion=*/0L);
- entry.markDirty(/*isChanged=*/false);
- try {
- entry.markDirty(/*isChanged=*/false);
- fail("Cannot mark entry dirty twice");
- } catch (IllegalStateException e) {
- // Expected.
- }
+
+ setValue(entry, new SkyValue() {}, /*errorInfo=*/ null, /*graphVersion=*/ 0L);
+ assertThat(entry.isDirty()).isFalse();
+ assertThat(entry.isChanged()).isFalse();
+ assertThat(entry.isDone()).isTrue();
+
+ MarkedDirtyResult firstResult = entry.markDirty(/*isChanged=*/ false);
+ assertThat(firstResult.getReverseDepsUnsafeIfWasClean()).isNotNull();
+ assertThat(firstResult.wasCallRedundant()).isFalse();
+ assertThat(firstResult.wasClean()).isTrue();
+ assertThat(entry.isDirty()).isTrue();
+ assertThat(entry.isChanged()).isFalse();
+ assertThat(entry.isDone()).isFalse();
+
+ MarkedDirtyResult secondResult = entry.markDirty(/*isChanged=*/ false);
+ assertThat(secondResult.wasClean()).isFalse();
+ assertThat(secondResult.wasCallRedundant()).isTrue();
+ assertThat(entry.isDirty()).isTrue();
+ assertThat(entry.isChanged()).isFalse();
+ assertThat(entry.isDone()).isFalse();
}
@Test
@@ -373,7 +411,9 @@
setValue(entry, new SkyValue() {}, /*errorInfo=*/null, /*graphVersion=*/0L);
assertThat(entry.isDirty()).isFalse();
assertThat(entry.isDone()).isTrue();
- entry.markDirty(/*isChanged=*/false);
+ MarkedDirtyResult markedDirtyResult = entry.markDirty(/*isChanged=*/false);
+ assertThat(markedDirtyResult.wasCallRedundant()).isFalse();
+ assertThat(markedDirtyResult.wasClean()).isTrue();
assertThat(entry.isDirty()).isTrue();
assertThat(entry.isChanged()).isFalse();
assertThat(entry.isDone()).isFalse();
@@ -419,7 +459,9 @@
addTemporaryDirectDep(entry, dep);
entry.signalDep();
setValue(entry, new IntegerValue(5), /*errorInfo=*/null, /*graphVersion=*/0L);
- entry.markDirty(/*isChanged=*/false);
+ MarkedDirtyResult markedDirtyResult = entry.markDirty(/*isChanged=*/false);
+ assertThat(markedDirtyResult.wasCallRedundant()).isFalse();
+ assertThat(markedDirtyResult.wasClean()).isTrue();
entry.addReverseDepAndCheckIfDone(null); // Start evaluation.
assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES);
entry.addReverseDepAndCheckIfDone(null); // Start evaluation.
@@ -444,7 +486,9 @@
setValue(entry, new IntegerValue(5), /*errorInfo=*/null, /*graphVersion=*/0L);
assertThat(entry.isDirty()).isFalse();
assertThat(entry.isDone()).isTrue();
- entry.markDirty(/*isChanged=*/false);
+ MarkedDirtyResult markedDirtyResult = entry.markDirty(/*isChanged=*/false);
+ assertThat(markedDirtyResult.wasCallRedundant()).isFalse();
+ assertThat(markedDirtyResult.wasClean()).isTrue();
assertThat(entry.isDirty()).isTrue();
assertThat(entry.isChanged()).isFalse();
assertThat(entry.isDone()).isFalse();
@@ -487,7 +531,9 @@
setValue(entry, new IntegerValue(5), /*errorInfo=*/ null, /*graphVersion=*/ 0L);
assertThat(entry.isDirty()).isFalse();
assertThat(entry.isDone()).isTrue();
- entry.markDirty(/*isChanged=*/ false);
+ MarkedDirtyResult markedDirtyResult = entry.markDirty(/*isChanged=*/ false);
+ assertThat(markedDirtyResult.wasCallRedundant()).isFalse();
+ assertThat(markedDirtyResult.wasClean()).isTrue();
assertThat(entry.isDirty()).isTrue();
assertThat(entry.isChanged()).isFalse();
assertThat(entry.isDone()).isFalse();
@@ -525,7 +571,9 @@
key("cause"));
ErrorInfo errorInfo = ErrorInfo.fromException(exception, false);
setValue(entry, /*value=*/null, errorInfo, /*graphVersion=*/0L);
- entry.markDirty(/*isChanged=*/false);
+ MarkedDirtyResult markedDirtyResult = entry.markDirty(/*isChanged=*/false);
+ assertThat(markedDirtyResult.wasCallRedundant()).isFalse();
+ assertThat(markedDirtyResult.wasClean()).isTrue();
entry.addReverseDepAndCheckIfDone(null); // Restart evaluation.
assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES);
assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep);
@@ -553,7 +601,9 @@
entry.signalDep();
entry.signalDep();
setValue(entry, /*value=*/new IntegerValue(5), null, 0L);
- entry.markDirty(/*isChanged=*/false);
+ MarkedDirtyResult markedDirtyResult = entry.markDirty(/*isChanged=*/false);
+ assertThat(markedDirtyResult.wasCallRedundant()).isFalse();
+ assertThat(markedDirtyResult.wasClean()).isTrue();
entry.addReverseDepAndCheckIfDone(null); // Restart evaluation.
assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES);
assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep, dep2);
@@ -584,7 +634,9 @@
new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT),
key("key"));
setValue(entry, null, ErrorInfo.fromException(exception, false), 0L);
- entry.markDirty(/*isChanged=*/false);
+ MarkedDirtyResult markedDirtyResult = entry.markDirty(/*isChanged=*/false);
+ assertThat(markedDirtyResult.wasCallRedundant()).isFalse();
+ assertThat(markedDirtyResult.wasClean()).isTrue();
entry.addReverseDepAndCheckIfDone(null); // Restart evaluation.
assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES);
assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep);
@@ -602,7 +654,9 @@
addTemporaryDirectDep(entry, dep);
entry.signalDep();
setValue(entry, new IntegerValue(5), /*errorInfo=*/null, /*graphVersion=*/0L);
- entry.markDirty(/*isChanged=*/false);
+ MarkedDirtyResult markedDirtyResult = entry.markDirty(/*isChanged=*/false);
+ assertThat(markedDirtyResult.wasCallRedundant()).isFalse();
+ assertThat(markedDirtyResult.wasClean()).isTrue();
entry.addReverseDepAndCheckIfDone(null); // Start evaluation.
assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES);
assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep);
@@ -630,7 +684,9 @@
entry.signalDep();
}
setValue(entry, new IntegerValue(5), /*errorInfo=*/null, /*graphVersion=*/0L);
- entry.markDirty(/*isChanged=*/false);
+ MarkedDirtyResult markedDirtyResult = entry.markDirty(/*isChanged=*/false);
+ assertThat(markedDirtyResult.wasCallRedundant()).isFalse();
+ assertThat(markedDirtyResult.wasClean()).isTrue();
entry.addReverseDepAndCheckIfDone(null); // Start new evaluation.
assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES);
for (int ii = 0; ii < 10; ii++) {
@@ -650,7 +706,9 @@
NodeEntry entry = new InMemoryNodeEntry();
entry.addReverseDepAndCheckIfDone(key("parent"));
setValue(entry, new SkyValue() {}, /*errorInfo=*/null, /*graphVersion=*/0L);
- entry.markDirty(/*isChanged=*/true);
+ MarkedDirtyResult markedDirtyResult = entry.markDirty(/*isChanged=*/true);
+ assertThat(markedDirtyResult.wasCallRedundant()).isFalse();
+ assertThat(markedDirtyResult.wasClean()).isTrue();
SkyKey newParent = key("new parent");
entry.addReverseDepAndCheckIfDone(newParent);
assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING);
@@ -678,7 +736,9 @@
entry.removeReverseDep(key("parent1"));
entry.removeReverseDep(key("parent2"));
IntegerValue updatedValue = new IntegerValue(52);
- clone2.markDirty(true);
+ MarkedDirtyResult markedDirtyResult = clone2.markDirty(true);
+ assertThat(markedDirtyResult.wasCallRedundant()).isFalse();
+ assertThat(markedDirtyResult.wasClean()).isTrue();
clone2.addReverseDepAndCheckIfDone(null);
SkyKey newChild = key("newchild");
addTemporaryDirectDep(clone2, newChild);
