diff --git a/src/main/java/com/google/devtools/build/skyframe/IncrementalInMemoryNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/IncrementalInMemoryNodeEntry.java index 5e345763490d18..1cf013ab621b0d 100644 --- a/src/main/java/com/google/devtools/build/skyframe/IncrementalInMemoryNodeEntry.java +++ b/src/main/java/com/google/devtools/build/skyframe/IncrementalInMemoryNodeEntry.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.skyframe; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; @@ -161,6 +162,10 @@ public synchronized Set getInProgressReverseDeps() { public synchronized Set setValue( SkyValue value, Version graphVersion, @Nullable Version maxTransitiveSourceVersion) throws InterruptedException { + checkArgument( + keepsEdges() || graphVersion.equals(ConstantVersion.INSTANCE), + "Non-incremental evaluations must use a constant graph version, got %s", + graphVersion); checkState(!hasUnsignaledDeps(), "Has unsignaled deps (this=%s, value=%s)", this, value); checkState( version.lastChanged().atMost(graphVersion) && version.lastEvaluated().atMost(graphVersion), diff --git a/src/test/java/com/google/devtools/build/skyframe/InMemoryGraphTest.java b/src/test/java/com/google/devtools/build/skyframe/InMemoryGraphTest.java index 339b4d28bc6163..19de5a25304cdc 100644 --- a/src/test/java/com/google/devtools/build/skyframe/InMemoryGraphTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryGraphTest.java @@ -57,6 +57,11 @@ protected ProcessableGraph getGraph(Version version) { return new EdgelessInMemoryGraphImpl(/* usePooledInterning= */ true); } + @Override + protected Version getStartingVersion() { + return ConstantVersion.INSTANCE; + } + @Override protected Version getNextVersion(Version version) { throw new UnsupportedOperationException(); 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 4bc17e1849ac2f..304ba18df5bf57 100644 --- a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.skyframe; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.truth.Truth.assertThat; import static com.google.devtools.build.skyframe.NodeEntrySubjectFactory.assertThatNodeEntry; import static org.junit.Assert.assertThrows; @@ -28,6 +29,7 @@ import com.google.devtools.build.skyframe.NodeEntry.DirtyType; import com.google.devtools.build.skyframe.SkyFunctionException.ReifiedSkyFunctionException; import com.google.devtools.build.skyframe.SkyFunctionException.Transience; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.errorprone.annotations.ForOverride; import com.google.testing.junit.testparameterinjector.TestParameter; import java.util.Set; @@ -57,14 +59,13 @@ public boolean supportsPartialReevaluation() { } }; - static final IntVersion ZERO_VERSION = IntVersion.of(0L); - static final IntVersion ONE_VERSION = IntVersion.of(1L); - private static final NestedSet NO_EVENTS = NestedSetBuilder.emptySet(Order.STABLE_ORDER); @TestParameter boolean isPartialReevaluation; + final Version initialVersion = keepEdges() ? IntVersion.of(0) : ConstantVersion.INSTANCE; + static SkyKey key(String name) { return GraphTester.skyKey(name); } @@ -102,7 +103,7 @@ public void signalEntry() throws InterruptedException { entry.addSingletonTemporaryDirectDep(dep1); assertThat(entry.isReadyToEvaluate()).isEqualTo(isPartialReevaluation); assertThat(entry.hasUnsignaledDeps()).isTrue(); - assertThat(entry.signalDep(ZERO_VERSION, dep1)).isTrue(); + assertThat(entry.signalDep(initialVersion, dep1)).isTrue(); assertThat(entry.isReadyToEvaluate()).isTrue(); assertThat(entry.hasUnsignaledDeps()).isFalse(); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep1); @@ -112,16 +113,15 @@ public void signalEntry() throws InterruptedException { entry.addSingletonTemporaryDirectDep(dep3); assertThat(entry.isReadyToEvaluate()).isEqualTo(isPartialReevaluation); assertThat(entry.hasUnsignaledDeps()).isTrue(); - assertThat(entry.signalDep(ZERO_VERSION, dep2)).isFalse(); + assertThat(entry.signalDep(initialVersion, dep2)).isFalse(); assertThat(entry.isReadyToEvaluate()).isEqualTo(isPartialReevaluation); assertThat(entry.hasUnsignaledDeps()).isTrue(); - assertThat(entry.signalDep(ZERO_VERSION, dep3)).isTrue(); + assertThat(entry.signalDep(initialVersion, dep3)).isTrue(); assertThat(entry.isReadyToEvaluate()).isTrue(); assertThat(entry.hasUnsignaledDeps()).isFalse(); - assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L)) - .isEmpty(); + assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion)).isEmpty(); assertThat(entry.isDone()).isTrue(); - assertThat(entry.getVersion()).isEqualTo(ZERO_VERSION); + assertThat(entry.getVersion()).isEqualTo(initialVersion); if (!keepEdges()) { return; @@ -138,13 +138,13 @@ public void signalExternalDep() throws InterruptedException { entry.addExternalDep(); assertThat(entry.isReadyToEvaluate()).isEqualTo(isPartialReevaluation); assertThat(entry.hasUnsignaledDeps()).isTrue(); - assertThat(entry.signalDep(ZERO_VERSION, null)).isTrue(); + assertThat(entry.signalDep(initialVersion, null)).isTrue(); assertThat(entry.isReadyToEvaluate()).isTrue(); assertThat(entry.hasUnsignaledDeps()).isFalse(); entry.addExternalDep(); assertThat(entry.isReadyToEvaluate()).isEqualTo(isPartialReevaluation); assertThat(entry.hasUnsignaledDeps()).isTrue(); - assertThat(entry.signalDep(ZERO_VERSION, null)).isTrue(); + assertThat(entry.signalDep(initialVersion, null)).isTrue(); assertThat(entry.isReadyToEvaluate()).isTrue(); assertThat(entry.hasUnsignaledDeps()).isFalse(); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(); @@ -162,7 +162,7 @@ public void reverseDeps() throws InterruptedException { assertThat(entry.addReverseDepAndCheckIfDone(father)) .isEqualTo(DependencyState.ALREADY_EVALUATING); entry.markRebuilding(); - assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L)) + assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion)) .containsExactly(mother, father); if (!keepEdges()) { @@ -184,7 +184,7 @@ public void errorValue() throws InterruptedException { new ReifiedSkyFunctionException( new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT)); ErrorInfo errorInfo = ErrorInfo.fromException(exception, false); - assertThat(setValue(entry, /* value= */ null, errorInfo, /* graphVersion= */ 0L)).isEmpty(); + assertThat(setValue(entry, /* value= */ null, errorInfo, initialVersion)).isEmpty(); assertThat(entry.isDone()).isTrue(); assertThat(entry.getValue()).isNull(); assertThat(entry.getErrorInfo()).isEqualTo(errorInfo); @@ -199,7 +199,7 @@ public void errorAndValue() throws InterruptedException { new ReifiedSkyFunctionException( new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT)); ErrorInfo errorInfo = ErrorInfo.fromException(exception, false); - setValue(entry, new SkyValue() {}, errorInfo, /* graphVersion= */ 0L); + setValue(entry, new SkyValue() {}, errorInfo, initialVersion); assertThat(entry.isDone()).isTrue(); assertThat(entry.getErrorInfo()).isEqualTo(errorInfo); } @@ -211,7 +211,7 @@ public void crashOnNullErrorAndValue() throws InterruptedException { entry.markRebuilding(); assertThrows( IllegalStateException.class, - () -> setValue(entry, /* value= */ null, /* errorInfo= */ null, /* graphVersion= */ 0L)); + () -> setValue(entry, /* value= */ null, /* errorInfo= */ null, initialVersion)); } @Test @@ -219,7 +219,7 @@ public void crashOnTooManySignals() { InMemoryNodeEntry entry = createEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. entry.markRebuilding(); - assertThrows(IllegalStateException.class, () -> entry.signalDep(ZERO_VERSION, null)); + assertThrows(IllegalStateException.class, () -> entry.signalDep(initialVersion, null)); } @Test @@ -227,11 +227,11 @@ public void crashOnSetValueWhenDone() throws InterruptedException { NodeEntry entry = createEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. entry.markRebuilding(); - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L); + setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion); assertThat(entry.isDone()).isTrue(); assertThrows( IllegalStateException.class, - () -> setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 1L)); + () -> setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion)); } @Test @@ -241,8 +241,8 @@ public void forceRebuildLifecycle() throws InterruptedException { entry.markRebuilding(); SkyKey dep = key("dep"); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L); + entry.signalDep(initialVersion, dep); + setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion); assertThat(entry.isDirty()).isFalse(); assertThat(entry.isDone()).isTrue(); @@ -268,10 +268,10 @@ public void forceRebuildLifecycle() throws InterruptedException { // A force-rebuilt node tolerates evaluating to different values within the same version. entry.forceRebuild(); - assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L)) + assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion)) .containsExactly(parent); - assertThat(entry.getVersion()).isEqualTo(ZERO_VERSION); + assertThat(entry.getVersion()).isEqualTo(initialVersion); } @Test @@ -279,7 +279,7 @@ public void allowTwiceMarkedForceRebuild() throws InterruptedException { NodeEntry entry = createEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. entry.markRebuilding(); - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L); + setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion); assertThat(entry.isDirty()).isFalse(); assertThat(entry.isDone()).isTrue(); entry.markDirty(DirtyType.FORCE_REBUILD); @@ -301,8 +301,7 @@ public void crashOnAddReverseDepTwice() throws InterruptedException { assertThrows( "Cannot add same dep twice", IllegalStateException.class, - () -> - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L)); + () -> setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion)); assertThat(e).hasMessageThat().containsMatch("[Dd]uplicate( new)? reverse deps"); } @@ -398,10 +397,13 @@ public void addTemporaryDirectDepsInGroups_depsExhausted_throws() { ImmutableSet.of(key("1"), key("2"), key("3")), ImmutableList.of(1, 1, 2))); } + @CanIgnoreReturnValue static Set setValue( - NodeEntry entry, SkyValue value, @Nullable ErrorInfo errorInfo, long graphVersion) + NodeEntry entry, SkyValue value, @Nullable ErrorInfo errorInfo, Version graphVersion) throws InterruptedException { return entry.setValue( - ValueWithMetadata.normal(value, errorInfo, NO_EVENTS), IntVersion.of(graphVersion), null); + ValueWithMetadata.normal(value, errorInfo, NO_EVENTS), + checkNotNull(graphVersion), + /* maxTransitiveSourceVersion= */ null); } } 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 b3e019a054a55d..354a147f890d39 100644 --- a/src/test/java/com/google/devtools/build/skyframe/IncrementalInMemoryNodeEntryTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/IncrementalInMemoryNodeEntryTest.java @@ -34,6 +34,8 @@ @RunWith(TestParameterInjector.class) public final class IncrementalInMemoryNodeEntryTest extends InMemoryNodeEntryTest { + private final Version incrementalVersion = ((IntVersion) initialVersion).next(); + @Override boolean keepEdges() { return true; @@ -46,8 +48,8 @@ public void dirtyLifecycle() throws InterruptedException { entry.markRebuilding(); SkyKey dep = key("dep"); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L); + entry.signalDep(initialVersion, dep); + setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion); assertThat(entry.isDirty()).isFalse(); assertThat(entry.isDone()).isTrue(); entry.markDirty(DirtyType.DIRTY); @@ -67,12 +69,12 @@ public void dirtyLifecycle() throws InterruptedException { assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ONE_VERSION, dep); + entry.signalDep(incrementalVersion, dep); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep); assertThat(entry.isReadyToEvaluate()).isTrue(); assertThat(entry.hasUnsignaledDeps()).isFalse(); entry.markRebuilding(); - assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 1L)) + assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, incrementalVersion)) .containsExactly(parent); } @@ -83,8 +85,8 @@ public void changedLifecycle() throws InterruptedException { entry.markRebuilding(); SkyKey dep = key("dep"); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L); + entry.signalDep(initialVersion, dep); + setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion); assertThat(entry.isDirty()).isFalse(); assertThat(entry.isDone()).isTrue(); entry.markDirty(DirtyType.CHANGE); @@ -103,9 +105,9 @@ public void changedLifecycle() throws InterruptedException { assertThat(entry.hasUnsignaledDeps()).isFalse(); assertThat(entry.getTemporaryDirectDeps()).isEmpty(); entry.markRebuilding(); - assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 1L)) + assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, incrementalVersion)) .containsExactly(parent); - assertThat(entry.getVersion()).isEqualTo(ONE_VERSION); + assertThat(entry.getVersion()).isEqualTo(incrementalVersion); } @Test @@ -115,8 +117,8 @@ public void markDirtyThenChanged() throws InterruptedException { entry.markRebuilding(); SkyKey dep = key("dep"); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L); + entry.signalDep(initialVersion, dep); + setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion); assertThat(entry.isDirty()).isFalse(); assertThat(entry.isDone()).isTrue(); entry.markDirty(DirtyType.DIRTY); @@ -141,8 +143,8 @@ public void markChangedThenDirty() throws InterruptedException { entry.markRebuilding(); SkyKey dep = key("dep"); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L); + entry.signalDep(initialVersion, dep); + setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion); assertThat(entry.isDirty()).isFalse(); assertThat(entry.isDone()).isTrue(); entry.markDirty(DirtyType.CHANGE); @@ -165,7 +167,7 @@ public void crashOnTwiceMarkedChanged() throws InterruptedException { NodeEntry entry = createEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. entry.markRebuilding(); - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L); + setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion); assertThat(entry.isDirty()).isFalse(); assertThat(entry.isDone()).isTrue(); entry.markDirty(DirtyType.CHANGE); @@ -182,8 +184,8 @@ public void crashOnTwiceMarkedDirty() throws InterruptedException { entry.markRebuilding(); SkyKey dep = key("dep"); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L); + entry.signalDep(initialVersion, dep); + setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion); entry.markDirty(DirtyType.DIRTY); assertThrows( "Cannot mark entry dirty twice", @@ -196,7 +198,7 @@ public void crashOnAddReverseDepTwiceAfterDone() throws InterruptedException { NodeEntry entry = createEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. entry.markRebuilding(); - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L); + setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion); SkyKey parent = key("parent"); assertThat(entry.addReverseDepAndCheckIfDone(parent)).isEqualTo(DependencyState.DONE); entry.addReverseDepAndCheckIfDone(parent); @@ -214,7 +216,7 @@ public void crashOnAddReverseDepBeforeAfterDone() throws InterruptedException { assertThat(entry.addReverseDepAndCheckIfDone(parent)) .isEqualTo(DependencyState.NEEDS_SCHEDULING); entry.markRebuilding(); - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L); + setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion); entry.addReverseDepAndCheckIfDone(parent); assertThrows( "Cannot add same dep twice", @@ -230,8 +232,8 @@ public void pruneBeforeBuild() throws InterruptedException { entry.addReverseDepAndCheckIfDone(null); // Start evaluation. entry.markRebuilding(); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L); + entry.signalDep(initialVersion, dep); + setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion); assertThat(entry.isDirty()).isFalse(); assertThat(entry.isDone()).isTrue(); entry.markDirty(DirtyType.DIRTY); @@ -248,11 +250,11 @@ public void pruneBeforeBuild() throws InterruptedException { assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, /* childForDebugging= */ null); + entry.signalDep(initialVersion, /* childForDebugging= */ null); assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.VERIFIED_CLEAN); assertThat(entry.markClean().getRdepsToSignal()).containsExactly(parent); assertThat(entry.isDone()).isTrue(); - assertThat(entry.getVersion()).isEqualTo(ZERO_VERSION); + assertThat(entry.getVersion()).isEqualTo(initialVersion); } @Test @@ -262,21 +264,21 @@ public void pruneAfterBuild() throws InterruptedException { entry.markRebuilding(); SkyKey dep = key("dep"); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); - setValue(entry, new IntegerValue(5), /* errorInfo= */ null, /* graphVersion= */ 0L); + entry.signalDep(initialVersion, dep); + 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); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ONE_VERSION, /* childForDebugging= */ null); + entry.signalDep(incrementalVersion, /* childForDebugging= */ null); assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep); entry.markRebuilding(); - setValue(entry, new IntegerValue(5), /* errorInfo= */ null, /* graphVersion= */ 1L); + setValue(entry, new IntegerValue(5), /* errorInfo= */ null, incrementalVersion); assertThat(entry.isDone()).isTrue(); - assertThat(entry.getVersion()).isEqualTo(ZERO_VERSION); + assertThat(entry.getVersion()).isEqualTo(initialVersion); } @Test @@ -286,8 +288,8 @@ public void noPruneWhenDetailsChange() throws InterruptedException { entry.markRebuilding(); SkyKey dep = key("dep"); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); - setValue(entry, new IntegerValue(5), /* errorInfo= */ null, /* graphVersion= */ 0L); + entry.signalDep(initialVersion, dep); + setValue(entry, new IntegerValue(5), /* errorInfo= */ null, initialVersion); assertThat(entry.isDirty()).isFalse(); assertThat(entry.isDone()).isTrue(); entry.markDirty(DirtyType.DIRTY); @@ -304,7 +306,7 @@ public void noPruneWhenDetailsChange() throws InterruptedException { assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ONE_VERSION, /* childForDebugging= */ null); + entry.signalDep(incrementalVersion, /* childForDebugging= */ null); assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep); ReifiedSkyFunctionException exception = @@ -312,10 +314,7 @@ public void noPruneWhenDetailsChange() throws InterruptedException { new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT)); entry.markRebuilding(); setValue( - entry, - new IntegerValue(5), - ErrorInfo.fromException(exception, false), - /* graphVersion= */ 1L); + entry, new IntegerValue(5), ErrorInfo.fromException(exception, false), incrementalVersion); assertThat(entry.isDone()).isTrue(); assertWithMessage("Version increments when setValue changes") .that(entry.getVersion()) @@ -332,10 +331,10 @@ public void pruneWhenDepGroupReordered() throws InterruptedException { SkyKey dep2InGroup = key("dep2InGroup"); entry.addSingletonTemporaryDirectDep(dep); entry.addTemporaryDirectDepGroup(ImmutableList.of(dep1InGroup, dep2InGroup)); - entry.signalDep(ZERO_VERSION, dep); - entry.signalDep(ZERO_VERSION, dep1InGroup); - entry.signalDep(ZERO_VERSION, dep2InGroup); - setValue(entry, new IntegerValue(5), /* errorInfo= */ null, /* graphVersion= */ 0L); + entry.signalDep(initialVersion, dep); + entry.signalDep(initialVersion, dep1InGroup); + entry.signalDep(initialVersion, dep2InGroup); + setValue(entry, new IntegerValue(5), /* errorInfo= */ null, initialVersion); assertThat(entry.isDirty()).isFalse(); assertThat(entry.isDone()).isTrue(); entry.markDirty(DirtyType.DIRTY); @@ -351,14 +350,14 @@ public void pruneWhenDepGroupReordered() throws InterruptedException { assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ONE_VERSION, /* childForDebugging= */ null); + entry.signalDep(incrementalVersion, /* childForDebugging= */ null); assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep); entry.markRebuilding(); entry.addTemporaryDirectDepGroup(ImmutableList.of(dep2InGroup, dep1InGroup)); - assertThat(entry.signalDep(ONE_VERSION, dep2InGroup)).isFalse(); - assertThat(entry.signalDep(ONE_VERSION, dep1InGroup)).isTrue(); - setValue(entry, new IntegerValue(5), /* errorInfo= */ null, /* graphVersion= */ 1L); + assertThat(entry.signalDep(incrementalVersion, dep2InGroup)).isFalse(); + assertThat(entry.signalDep(incrementalVersion, dep1InGroup)).isTrue(); + setValue(entry, new IntegerValue(5), /* errorInfo= */ null, incrementalVersion); assertThat(entry.isDone()).isTrue(); assertWithMessage("Version does not change when dep group reordered") .that(entry.getVersion()) @@ -372,25 +371,25 @@ public void errorInfoCannotBePruned() throws InterruptedException { entry.markRebuilding(); SkyKey dep = key("dep"); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); + entry.signalDep(initialVersion, dep); ReifiedSkyFunctionException exception = new ReifiedSkyFunctionException( new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT)); ErrorInfo errorInfo = ErrorInfo.fromException(exception, false); - setValue(entry, /* value= */ null, errorInfo, /* graphVersion= */ 0L); + 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.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ONE_VERSION, /* childForDebugging= */ null); + entry.signalDep(incrementalVersion, /* childForDebugging= */ null); assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep); entry.markRebuilding(); - setValue(entry, /* value= */ null, errorInfo, /* graphVersion= */ 1L); + setValue(entry, /* value= */ null, errorInfo, incrementalVersion); assertThat(entry.isDone()).isTrue(); // ErrorInfo is treated as a NotComparableSkyValue, so it is not pruned. - assertThat(entry.getVersion()).isEqualTo(ONE_VERSION); + assertThat(entry.getVersion()).isEqualTo(incrementalVersion); } @Test @@ -403,17 +402,17 @@ public void getDependencyGroup() throws InterruptedException { SkyKey dep3 = key("dep3"); entry.addTemporaryDirectDepGroup(ImmutableList.of(dep, dep2)); entry.addSingletonTemporaryDirectDep(dep3); - entry.signalDep(ZERO_VERSION, dep); - entry.signalDep(ZERO_VERSION, dep2); - entry.signalDep(ZERO_VERSION, dep3); - setValue(entry, /* value= */ new IntegerValue(5), null, 0L); + entry.signalDep(initialVersion, dep); + entry.signalDep(initialVersion, dep2); + entry.signalDep(initialVersion, dep3); + 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.getNextDirtyDirectDeps()).containsExactly(dep, dep2); entry.addTemporaryDirectDepGroup(ImmutableList.of(dep, dep2)); - entry.signalDep(ZERO_VERSION, /* childForDebugging= */ null); - entry.signalDep(ZERO_VERSION, /* childForDebugging= */ null); + entry.signalDep(initialVersion, /* childForDebugging= */ null); + entry.signalDep(initialVersion, /* childForDebugging= */ null); assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep3); } @@ -431,20 +430,20 @@ public void maintainDependencyGroupAfterRemoval() throws InterruptedException { entry.addTemporaryDirectDepGroup(ImmutableList.of(dep, dep2, dep3)); entry.addSingletonTemporaryDirectDep(dep4); entry.addSingletonTemporaryDirectDep(dep5); - entry.signalDep(ZERO_VERSION, dep4); - entry.signalDep(ZERO_VERSION, dep); + entry.signalDep(initialVersion, dep4); + entry.signalDep(initialVersion, dep); // Oops! Evaluation terminated with an error, but we're going to set this entry's value anyway. entry.removeUnfinishedDeps(ImmutableSet.of(dep2, dep3, dep5)); ReifiedSkyFunctionException exception = new ReifiedSkyFunctionException( new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT)); - setValue(entry, null, ErrorInfo.fromException(exception, false), 0L); + 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.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); + entry.signalDep(initialVersion, dep); assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep4); } @@ -456,22 +455,22 @@ public void pruneWhenDepsChange() throws InterruptedException { entry.markRebuilding(); SkyKey dep = key("dep"); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); - setValue(entry, new IntegerValue(5), /* errorInfo= */ null, /* graphVersion= */ 0L); + entry.signalDep(initialVersion, dep); + 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.getNextDirtyDirectDeps()).containsExactly(dep); entry.addSingletonTemporaryDirectDep(dep); - assertThat(entry.signalDep(ONE_VERSION, /* childForDebugging= */ null)).isTrue(); + assertThat(entry.signalDep(incrementalVersion, /* childForDebugging= */ null)).isTrue(); assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep); entry.markRebuilding(); entry.addSingletonTemporaryDirectDep(key("dep2")); - assertThat(entry.signalDep(ONE_VERSION, /* childForDebugging= */ null)).isTrue(); - setValue(entry, new IntegerValue(5), /* errorInfo= */ null, /* graphVersion= */ 1L); + assertThat(entry.signalDep(incrementalVersion, /* childForDebugging= */ null)).isTrue(); + setValue(entry, new IntegerValue(5), /* errorInfo= */ null, incrementalVersion); assertThat(entry.isDone()).isTrue(); - assertThatNodeEntry(entry).hasVersionThat().isEqualTo(ZERO_VERSION); + assertThatNodeEntry(entry).hasVersionThat().isEqualTo(initialVersion); } @Test @@ -484,16 +483,16 @@ public void checkDepsOneByOne() throws InterruptedException { SkyKey dep = key(Integer.toString(ii)); deps.add(dep); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); + entry.signalDep(initialVersion, dep); } - setValue(entry, new IntegerValue(5), /* errorInfo= */ null, /* graphVersion= */ 0L); + 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); for (int ii = 0; ii < 10; ii++) { assertThat(entry.getNextDirtyDirectDeps()).containsExactly(deps.get(ii)); entry.addSingletonTemporaryDirectDep(deps.get(ii)); - assertThat(entry.signalDep(ZERO_VERSION, /* childForDebugging= */ null)).isTrue(); + assertThat(entry.signalDep(initialVersion, /* childForDebugging= */ null)).isTrue(); if (ii < 9) { assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.CHECK_DEPENDENCIES); } else { @@ -507,14 +506,14 @@ public void signalOnlyNewParents() throws InterruptedException { NodeEntry entry = createEntry(); entry.addReverseDepAndCheckIfDone(key("parent")); entry.markRebuilding(); - setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 0L); + setValue(entry, new SkyValue() {}, /* errorInfo= */ null, initialVersion); entry.markDirty(DirtyType.CHANGE); SkyKey newParent = key("new parent"); entry.addReverseDepAndCheckIfDone(newParent); assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); entry.markRebuilding(); assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.REBUILDING); - assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, /* graphVersion= */ 1L)) + assertThat(setValue(entry, new SkyValue() {}, /* errorInfo= */ null, incrementalVersion)) .containsExactly(newParent); } @@ -534,7 +533,7 @@ public void getCompressedDirectDepsForDoneEntry() throws InterruptedException { for (ImmutableList depGroup : groupedDirectDeps) { entry.addTemporaryDirectDepGroup(depGroup); for (SkyKey dep : depGroup) { - entry.signalDep(ZERO_VERSION, dep); + entry.signalDep(initialVersion, dep); } } entry.setValue(new IntegerValue(42), IntVersion.of(42L), null); @@ -552,8 +551,8 @@ public void hasAtLeastOneDep_true() throws Exception { .isEqualTo(DependencyState.NEEDS_SCHEDULING); entry.markRebuilding(); entry.addSingletonTemporaryDirectDep(dep); - entry.signalDep(ZERO_VERSION, dep); - entry.setValue(new IntegerValue(1), ZERO_VERSION, null); + entry.signalDep(initialVersion, dep); + entry.setValue(new IntegerValue(1), initialVersion, null); assertThat(entry.hasAtLeastOneDep()).isTrue(); } @@ -565,7 +564,7 @@ public void hasAtLeastOneDep_false() throws Exception { .isEqualTo(DependencyState.NEEDS_SCHEDULING); entry.markRebuilding(); entry.addTemporaryDirectDepGroup(ImmutableList.of()); - entry.setValue(new IntegerValue(1), ZERO_VERSION, null); + entry.setValue(new IntegerValue(1), initialVersion, null); assertThat(entry.hasAtLeastOneDep()).isFalse(); } } diff --git a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java index 8f84b44bbc8cd4..2df65d086ee419 100644 --- a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java @@ -125,12 +125,11 @@ private ParallelEvaluator makeEvaluator( ProcessableGraph graph, ImmutableMap builders, boolean keepGoing, - EventFilter storedEventFilter) { - Version oldGraphVersion = graphVersion; - graphVersion = graphVersion.next(); + EventFilter storedEventFilter, + Version evaluationVersion) { return new ParallelEvaluator( graph, - oldGraphVersion, + evaluationVersion, Version.minimal(), builders, reportedEvents, @@ -146,6 +145,16 @@ private ParallelEvaluator makeEvaluator( UnnecessaryTemporaryStateDropperReceiver.NULL); } + private ParallelEvaluator makeEvaluator( + ProcessableGraph graph, + ImmutableMap builders, + boolean keepGoing, + EventFilter storedEventFilter) { + Version oldGraphVersion = graphVersion; + graphVersion = graphVersion.next(); + return makeEvaluator(graph, builders, keepGoing, storedEventFilter, oldGraphVersion); + } + private ParallelEvaluator makeEvaluator( ProcessableGraph graph, ImmutableMap builders, @@ -995,7 +1004,16 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept SkyKey topKey = GraphTester.toSkyKey("top"); tester.getOrCreate(topKey).addDependency(catastropheKey).setComputedValue(CONCATENATE); - EvaluationResult result = eval(keepGoing, topKey, otherKey); + + ParallelEvaluator evaluator = + makeEvaluator( + graph, + tester.getSkyFunctionMap(), + keepGoing, + EventFilter.FULL_STORAGE, + keepEdges ? graphVersion : ConstantVersion.INSTANCE); + + EvaluationResult result = evaluator.eval(ImmutableList.of(topKey, otherKey)); assertThatEvaluationResult(result).hasErrorEntryForKeyThat(topKey); if (keepGoing) { assertThat(result.getCatastrophe()).isSameInstanceAs(catastrophe);