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 c586075308530b..385bd2b9234adf 100644 --- a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java @@ -125,6 +125,7 @@ abstract class AbstractParallelEvaluator { AbstractParallelEvaluator( ProcessableGraph graph, Version graphVersion, + Version minimalVersion, ImmutableMap skyFunctions, ExtendedEventHandler reporter, EmittedEventState emittedEventState, @@ -146,6 +147,7 @@ abstract class AbstractParallelEvaluator { new ParallelEvaluatorContext( graph, graphVersion, + minimalVersion, skyFunctions, reporter, emittedEventState, @@ -161,7 +163,7 @@ abstract class AbstractParallelEvaluator { stateCache); } - private Supplier getQuiescingExecutorSupplier( + private static Supplier getQuiescingExecutorSupplier( Supplier executorService, int cpuHeavySkyKeysThreadPoolSize, int executionJobsThreadPoolSize) { diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java index 5b3031e07cc5d5..92e719260db31f 100644 --- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java @@ -186,6 +186,7 @@ public EvaluationResult evaluate( new ParallelEvaluator( graph, graphVersion, + MinimalVersion.INSTANCE, skyFunctions, evaluationContext.getEventHandler(), emittedEventState, 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 ad0be1897bb60e..17c5d0d789d4e5 100644 --- a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java +++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java @@ -13,8 +13,6 @@ // limitations under the License. package com.google.devtools.build.skyframe; -import static com.google.common.base.MoreObjects.firstNonNull; - import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -276,14 +274,14 @@ public synchronized Set setValue( SkyValue value, Version graphVersion, @Nullable Version maxTransitiveSourceVersion) throws InterruptedException { Preconditions.checkState(isReady(), "Not ready (this=%s, value=%s)", this, value); - Version version = firstNonNull(maxTransitiveSourceVersion, graphVersion); Preconditions.checkState( - this.lastChangedVersion.atMost(version) && this.lastEvaluatedVersion.atMost(version), + this.lastChangedVersion.atMost(graphVersion) + && this.lastEvaluatedVersion.atMost(graphVersion), "Bad version (this=%s, version=%s, value=%s)", this, - version, + graphVersion, value); - this.lastEvaluatedVersion = version; + this.lastEvaluatedVersion = graphVersion; if (dirtyBuildingState.unchangedFromLastBuild(value)) { // If the value is the same as before, just use the old value. Note that we don't use the new @@ -292,7 +290,7 @@ public synchronized Set setValue( } else { // If this is a new value, or it has changed since the last build, set the version to the // current graph version. - this.lastChangedVersion = version; + this.lastChangedVersion = graphVersion; this.value = value; } return setStateFinishedAndReturnReverseDepsToSignal(); 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 e554dde59c6613..250b9c27f155eb 100644 --- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java @@ -61,8 +61,9 @@ public class ParallelEvaluator extends AbstractParallelEvaluator { public ParallelEvaluator( ProcessableGraph graph, Version graphVersion, + Version minimalVersion, ImmutableMap skyFunctions, - final ExtendedEventHandler reporter, + ExtendedEventHandler reporter, EmittedEventState emittedEventState, EventFilter storedEventFilter, ErrorInfoManager errorInfoManager, @@ -77,6 +78,7 @@ public ParallelEvaluator( super( graph, graphVersion, + minimalVersion, skyFunctions, reporter, emittedEventState, @@ -507,7 +509,7 @@ private Map bubbleErrorUp( *

{@code visitor} may be null, but only in the case where all graph entries corresponding to * {@code skyKeys} are known to be in the DONE state ({@code entry.isDone()} returns true). */ - EvaluationResult constructResult( + private EvaluationResult constructResult( Iterable skyKeys, @Nullable Map bubbleErrorInfo, boolean catastrophe) diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java index fb7311d0c66ce1..2fa6ea2ec606a6 100644 --- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java +++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java @@ -39,6 +39,7 @@ class ParallelEvaluatorContext { private final QueryableGraph graph; private final Version graphVersion; + private final Version minimalVersion; private final ImmutableMap skyFunctions; private final ExtendedEventHandler reporter; private final NestedSetVisitor replayingNestedSetEventVisitor; @@ -74,6 +75,7 @@ interface ComparableRunnable extends Runnable, Comparable {} public ParallelEvaluatorContext( QueryableGraph graph, Version graphVersion, + Version minimalVersion, ImmutableMap skyFunctions, ExtendedEventHandler reporter, EmittedEventState emittedEventState, @@ -87,6 +89,7 @@ public ParallelEvaluatorContext( Cache stateCache) { this.graph = graph; this.graphVersion = graphVersion; + this.minimalVersion = minimalVersion; this.skyFunctions = skyFunctions; this.reporter = reporter; this.graphInconsistencyReceiver = graphInconsistencyReceiver; @@ -151,6 +154,10 @@ Version getGraphVersion() { return graphVersion; } + Version getMinimalVersion() { + return minimalVersion; + } + boolean keepGoing() { return keepGoing; } diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java index f941c6e81272ca..2a3fe35428bca3 100644 --- a/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java +++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java @@ -414,7 +414,21 @@ interface SkyKeyComputeState {} * Returns the max transitive source version of a {@link NodeEntry}. * *

This value might not consider all deps' source versions if called before all deps have - * been requested or if {@link #valuesMissing()} returns true. + * been requested or if {@link #valuesMissing} returns {@code true}. + * + *

Rules for calculation of the max transitive source version: + * + *

    + *
  • Returns {@code null} during cycle detection and error bubbling, or for transient + * errors. + *
  • If the node is {@link FunctionHermeticity#NONHERMETIC}, returns the version passed to + * {@link #injectVersionForNonHermeticFunction} if it was called, or else {@code null}. + *
  • For all other nodes, queries {@link NodeEntry#getMaxTransitiveSourceVersion} of direct + * dependency nodes and chooses the maximal version seen (according to {@link + * Version#atMost}). If there are no direct dependencies, returns {@link + * ParallelEvaluatorContext#getMinimalVersion}. If any direct dependency node has a {@code + * null} MTSV, returns {@code null}. + *
*/ @Nullable Version getMaxTransitiveSourceVersionSoFar(); diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java index 5c463a0ef3cb0a..967166afda565e 100644 --- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java +++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java @@ -207,7 +207,7 @@ private SkyFunctionEnvironment( this.maxTransitiveSourceVersion = bubbleErrorInfo == null && skyKey.functionName().getHermeticity() != FunctionHermeticity.NONHERMETIC - ? MinimalVersion.INSTANCE + ? evaluatorContext.getMinimalVersion() : null; this.previouslyRequestedDepsValues = batchPrefetch(throwIfPreviouslyRequestedDepsUndone); Preconditions.checkState( @@ -847,10 +847,6 @@ Set commitAndGetParents(NodeEntry primaryEntry) throws InterruptedExcept } } - if (temporaryDirectDeps.isEmpty() - && skyKey.functionName().getHermeticity() != FunctionHermeticity.NONHERMETIC) { - maxTransitiveSourceVersion = null; // No dependencies on source. - } Preconditions.checkState( maxTransitiveSourceVersion == null || newlyRegisteredDeps.isEmpty(), "Dependency registration not supported when tracking max transitive source versions"); diff --git a/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java b/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java index 8491cdb890c537..7618b077c1e96f 100644 --- a/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java @@ -137,6 +137,7 @@ protected EvaluationResult eval(boolean keepGoing, SkyKe new ParallelEvaluator( graph, graphVersion, + MinimalVersion.INSTANCE, tester.getSkyFunctionMap(), reporter, new MemoizingEvaluator.EmittedEventState(), 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 c729c635f7a693..b855156b3cce41 100644 --- a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java @@ -120,6 +120,7 @@ private ParallelEvaluator makeEvaluator( return new ParallelEvaluator( graph, oldGraphVersion, + MinimalVersion.INSTANCE, builders, storedEventHandler, new MemoizingEvaluator.EmittedEventState(), @@ -3118,6 +3119,7 @@ public void onEvaluationFinished() { new ParallelEvaluator( graph, graphVersion, + MinimalVersion.INSTANCE, tester.getSkyFunctionMap(), storedEventHandler, new MemoizingEvaluator.EmittedEventState(),