Replace `EvaluationVersionBehavior` with a method `NodeEntry#getMaxTransitiveSourceVersion`.
Both the graph version and the max transitive source version (if present) are provided to node entries in `setValue`. Nodes can choose which to use.
PiperOrigin-RevId: 414266059
diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractExceptionalParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/AbstractExceptionalParallelEvaluator.java
index 912fef9..4623cf5 100644
--- a/src/main/java/com/google/devtools/build/skyframe/AbstractExceptionalParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/AbstractExceptionalParallelEvaluator.java
@@ -95,7 +95,6 @@
GraphInconsistencyReceiver graphInconsistencyReceiver,
Supplier<ExecutorService> executorService,
CycleDetector cycleDetector,
- EvaluationVersionBehavior evaluationVersionBehavior,
int cpuHeavySkyKeysThreadPoolSize,
int executionJobsThreadPoolSize) {
super(
@@ -111,7 +110,6 @@
graphInconsistencyReceiver,
executorService,
cycleDetector,
- evaluationVersionBehavior,
cpuHeavySkyKeysThreadPoolSize,
executionJobsThreadPoolSize);
}
@@ -690,7 +688,7 @@
prevEntry.noDepsLastBuild(), "existing entry for %s has deps: %s", key, prevEntry);
}
prevEntry.markRebuilding();
- prevEntry.setValue(value, version);
+ prevEntry.setValue(value, version, /*maxTransitiveSourceVersion=*/ null);
// Now that this key's injected value is set, it is no longer dirty.
progressReceiver.injected(key);
}
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 fb2b1ad..4faad8b 100644
--- a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
@@ -102,8 +102,7 @@
DirtyTrackingProgressReceiver progressReceiver,
GraphInconsistencyReceiver graphInconsistencyReceiver,
Supplier<ExecutorService> executorService,
- CycleDetector cycleDetector,
- EvaluationVersionBehavior evaluationVersionBehavior) {
+ CycleDetector cycleDetector) {
this(
graph,
graphVersion,
@@ -117,7 +116,6 @@
graphInconsistencyReceiver,
executorService,
cycleDetector,
- evaluationVersionBehavior,
/*cpuHeavySkyKeysThreadPoolSize=*/ 0,
/*executionJobsThreadPoolSize=*/ 0);
}
@@ -135,7 +133,6 @@
GraphInconsistencyReceiver graphInconsistencyReceiver,
Supplier<ExecutorService> executorService,
CycleDetector cycleDetector,
- EvaluationVersionBehavior evaluationVersionBehavior,
int cpuHeavySkyKeysThreadPoolSize,
int executionJobsThreadPoolSize) {
this.graph = graph;
@@ -158,7 +155,6 @@
() ->
new NodeEntryVisitor(
quiescingExecutorSupplier.get(), progressReceiver, Evaluate::new),
- evaluationVersionBehavior,
/*mergingSkyframeAnalysisExecutionPhases=*/ executionJobsThreadPoolSize > 0);
}
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 d040549..3adda55 100644
--- a/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
@@ -55,8 +55,10 @@
}
@Override
- public Set<SkyKey> setValue(SkyValue value, Version version) throws InterruptedException {
- return getDelegate().setValue(value, version);
+ public Set<SkyKey> setValue(
+ SkyValue value, Version graphVersion, @Nullable Version maxTransitiveSourceVersion)
+ throws InterruptedException {
+ return getDelegate().setValue(value, graphVersion, maxTransitiveSourceVersion);
}
@Override
@@ -92,6 +94,11 @@
}
@Override
+ public Version getMaxTransitiveSourceVersion() {
+ return getDelegate().getMaxTransitiveSourceVersion();
+ }
+
+ @Override
public DirtyState getDirtyState() {
return getDelegate().getDirtyState();
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/EvaluationVersionBehavior.java b/src/main/java/com/google/devtools/build/skyframe/EvaluationVersionBehavior.java
deleted file mode 100644
index a4cc371..0000000
--- a/src/main/java/com/google/devtools/build/skyframe/EvaluationVersionBehavior.java
+++ /dev/null
@@ -1,29 +0,0 @@
-// Copyright 2018 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.skyframe;
-
-/**
- * What version to give an evaluated node: the max of its child versions or the graph version. Even
- * for {@link #MAX_CHILD_VERSIONS} the version may still be the graph version depending on
- * properties of the {@link SkyFunction} (if it is {@link FunctionHermeticity#NONHERMETIC}) or the
- * error state of the node.
- *
- * <p>Should be set to {@link #MAX_CHILD_VERSIONS} unless the evaluation framework is being very
- * sneaky.
- */
-public enum EvaluationVersionBehavior {
- MAX_CHILD_VERSIONS,
- GRAPH_VERSION
-}
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 9f507fb..b471603 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
@@ -208,7 +208,6 @@
AbstractQueueVisitor.createExecutorService(
evaluationContext.getParallelism(), "skyframe-evaluator")),
new SimpleCycleDetector(),
- EvaluationVersionBehavior.GRAPH_VERSION,
evaluationContext.getCPUHeavySkyKeysThreadPoolSize(),
evaluationContext.getExecutionPhaseThreadPoolSize());
result = evaluator.eval(roots);
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 38dc272..7084da7 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
@@ -13,6 +13,8 @@
// 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,9 +278,11 @@
* version and the value.
*/
@Override
- public synchronized Set<SkyKey> setValue(SkyValue value, Version version)
+ public synchronized Set<SkyKey> 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),
"Bad version (this=%s, version=%s, value=%s)",
@@ -300,29 +304,6 @@
return setStateFinishedAndReturnReverseDepsToSignal();
}
- /** An exception indicating that the node's value changed but its version did not. */
- public static final class ChangedValueAtSameVersionException extends IllegalStateException {
- private final SkyValue newValue;
-
- private ChangedValueAtSameVersionException(
- Version lastChangedVersion,
- Version newVersion,
- SkyValue newValue,
- InMemoryNodeEntry nodeEntry) {
- super(
- String.format(
- "Changed value but with the same version? "
- + "lastChangedVersion: %s, newVersion: %s newValue: %s, nodeEntry: %s",
- lastChangedVersion, newVersion, newValue, nodeEntry));
- this.newValue = newValue;
- }
-
- /** Returns the value that this node changed to. */
- public SkyValue getNewValue() {
- return newValue;
- }
- }
-
@Override
public DependencyState addReverseDepAndCheckIfDone(SkyKey reverseDep) {
if ((reverseDep == null || !keepReverseDeps()) && isDone()) {
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 afabf8d..9a87cf0 100644
--- a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
@@ -48,10 +48,10 @@
NEEDS_SCHEDULING,
/**
- * The node was already created, but isn't done yet. The evaluator is responsible for
- * signaling the reverse dependency node.
+ * The node was already created, but isn't done yet. The evaluator is responsible for signaling
+ * the reverse dependency node.
*/
- ALREADY_EVALUATING;
+ ALREADY_EVALUATING
}
/**
@@ -193,13 +193,24 @@
* reverse dependencies that are registered at the time of the {@code setValue} call. If b comes
* in before a, it is signaled (and re-scheduled) by a, otherwise it needs to do that itself.
*
- * <p>{@code version} indicates the graph version at which this node is being written. If the
- * entry determines that the new value is equal to the previous value, the entry will keep its
- * current version. Callers can query that version to see if the node considers its value to have
- * changed.
+ * <p>Nodes may elect to use either {@code graphVersion} or {@code maxTransitiveSourceVersion} (if
+ * not {@code null}) for their {@linkplain #getVersion version}. The choice can be distinguished
+ * by calling {@link #getMaxTransitiveSourceVersion} - a return of {@code null} indicates that the
+ * node uses the graph version.
+ *
+ * <p>If the entry determines that the new value is equal to the previous value, the entry may
+ * keep its current version. Callers can query that version to see if the node considers its value
+ * to have changed.
+ *
+ * @param value the new value of this node
+ * @param graphVersion the version of the graph at which this node is being written
+ * @param maxTransitiveSourceVersion the maximal version of this node's dependencies from source,
+ * or {@code null} if source versions are not being tracked
*/
@ThreadSafe
- Set<SkyKey> setValue(SkyValue value, Version version) throws InterruptedException;
+ Set<SkyKey> setValue(
+ SkyValue value, Version graphVersion, @Nullable Version maxTransitiveSourceVersion)
+ throws InterruptedException;
/**
* Queries if the node is done and adds the given key as a reverse dependency. The return code
@@ -303,13 +314,24 @@
@ThreadSafe
void forceRebuild();
- /**
- * Gets the current version of this entry.
- */
+ /** Returns the current version of this node. */
@ThreadSafe
Version getVersion();
/**
+ * Returns the maximal version of this node's dependencies from source.
+ *
+ * <p>This version should only be tracked when non-hermetic functions {@linkplain
+ * SkyFunction.Environment#injectVersionForNonHermeticFunction inject} source versions. Otherwise,
+ * returns {@code null} to signal that source versions are not being tracked.
+ */
+ @ThreadSafe
+ @Nullable
+ default Version getMaxTransitiveSourceVersion() {
+ return null;
+ }
+
+ /**
* 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}.
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 b849010..7cd5e20 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
@@ -49,8 +49,7 @@
DirtyTrackingProgressReceiver progressReceiver,
GraphInconsistencyReceiver graphInconsistencyReceiver,
Supplier<ExecutorService> executorService,
- CycleDetector cycleDetector,
- EvaluationVersionBehavior evaluationVersionBehavior) {
+ CycleDetector cycleDetector) {
this(
graph,
graphVersion,
@@ -64,7 +63,6 @@
graphInconsistencyReceiver,
executorService,
cycleDetector,
- evaluationVersionBehavior,
/*cpuHeavySkyKeysThreadPoolSize=*/ 0,
/*executionJobsThreadPoolSize=*/ 0);
}
@@ -82,7 +80,6 @@
GraphInconsistencyReceiver graphInconsistencyReceiver,
Supplier<ExecutorService> executorService,
CycleDetector cycleDetector,
- EvaluationVersionBehavior evaluationVersionBehavior,
int cpuHeavySkyKeysThreadPoolSize,
int executionJobsThreadPoolSize) {
super(
@@ -98,7 +95,6 @@
graphInconsistencyReceiver,
executorService,
cycleDetector,
- evaluationVersionBehavior,
cpuHeavySkyKeysThreadPoolSize,
executionJobsThreadPoolSize);
}
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 16e743d..62da390 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java
@@ -46,7 +46,6 @@
private final EventFilter storedEventFilter;
private final ErrorInfoManager errorInfoManager;
private final GraphInconsistencyReceiver graphInconsistencyReceiver;
- private final EvaluationVersionBehavior evaluationVersionBehavior;
private final boolean mergingSkyframeAnalysisExecutionPhases;
/**
@@ -81,14 +80,12 @@
ErrorInfoManager errorInfoManager,
GraphInconsistencyReceiver graphInconsistencyReceiver,
Supplier<NodeEntryVisitor> visitorSupplier,
- EvaluationVersionBehavior evaluationVersionBehavior,
boolean mergingSkyframeAnalysisExecutionPhases) {
this.graph = graph;
this.graphVersion = graphVersion;
this.skyFunctions = skyFunctions;
this.reporter = reporter;
this.graphInconsistencyReceiver = graphInconsistencyReceiver;
- this.evaluationVersionBehavior = evaluationVersionBehavior;
this.replayingNestedSetEventVisitor =
new NestedSetVisitor<>(new NestedSetEventReceiver(reporter), emittedEventState.eventState);
this.replayingNestedSetPostableVisitor =
@@ -189,10 +186,6 @@
return errorInfoManager;
}
- EvaluationVersionBehavior getEvaluationVersionBehavior() {
- return evaluationVersionBehavior;
- }
-
boolean restartPermitted() {
return graphInconsistencyReceiver.restartPermitted();
}
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 0601925..fc74426 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java
@@ -394,10 +394,11 @@
}
/**
- * Injects non-hermetic {@link Version} information for this environment.
+ * Injects non-hermetic {@link Version} information for the currently evaluating {@link SkyKey}.
*
- * <p>This may be called during the course of {@link SkyFunction#compute(SkyKey, Environment)}
- * if the function discovers version information for the {@link SkyKey}.
+ * <p>This may be called during the course of {@link SkyFunction#compute} if the function
+ * determines that the currently evaluating key's source dependencies have not changed since the
+ * given {@code version}.
*
* <p>Environments that either do not need or wish to ignore non-hermetic version information
* may keep the default no-op implementation.
@@ -410,8 +411,8 @@
* <p>WARNING: Dependencies here MUST be done! Only use this function if you know what you're
* doing.
*
- * <p>If the {@link EvaluationVersionBehavior} is {@link
- * EvaluationVersionBehavior#MAX_CHILD_VERSIONS} then this method must not be called.
+ * <p>If {@linkplain NodeEntry#getMaxTransitiveSourceVersion max transitive source versions} are
+ * being tracked, then this method must not be called.
*/
void registerDependencies(Iterable<SkyKey> keys);
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 06f735c..0f5d88a 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.skyframe;
-
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
@@ -74,10 +73,7 @@
private SkyValue value = null;
private ErrorInfo errorInfo = null;
- @Nullable private Version maxChildVersion = null;
-
- /** If present, takes precedence over {@link #maxChildVersion}. */
- @Nullable private Version injectedVersion = null;
+ @Nullable private Version maxTransitiveSourceVersion;
/**
* This is not {@code null} only during cycle detection and error bubbling. The nullness of this
@@ -108,8 +104,7 @@
private final Map<SkyKey, SkyValue> newlyRequestedDepsValues = new HashMap<>();
/**
- * Keys of dependencies registered via {@link #registerDependencies} if not using {@link
- * EvaluationVersionBehavior#MAX_CHILD_VERSIONS}.
+ * Keys of dependencies registered via {@link #registerDependencies}.
*
* <p>The {@link #registerDependencies} method is hacky. Deps registered through it may not have
* entries in {@link #newlyRequestedDepsValues}, but they are expected to be done. This set tracks
@@ -203,6 +198,13 @@
this.bubbleErrorInfo = bubbleErrorInfo;
this.oldDeps = Preconditions.checkNotNull(oldDeps);
this.evaluatorContext = Preconditions.checkNotNull(evaluatorContext);
+ // Cycles can lead to a state where the versions of done children don't accurately reflect the
+ // state that led to this node's value. Be conservative then.
+ this.maxTransitiveSourceVersion =
+ bubbleErrorInfo == null
+ && skyKey.functionName().getHermeticity() != FunctionHermeticity.NONHERMETIC
+ ? MinimalVersion.INSTANCE
+ : null;
this.previouslyRequestedDepsValues = batchPrefetch(throwIfPreviouslyRequestedDepsUndone);
Preconditions.checkState(
!this.previouslyRequestedDepsValues.containsKey(ErrorTransienceValue.KEY),
@@ -250,10 +252,10 @@
}
depValuesBuilder.put(entry.getKey(), !depDone ? NULL_MARKER : valueMaybeWithMetadata);
if (depDone) {
- maybeUpdateMaxChildVersion(entry.getValue());
+ maybeUpdateMaxTransitiveSourceVersion(entry.getValue());
}
}
- return depValuesBuilder.build();
+ return depValuesBuilder.buildOrThrow();
}
private void checkActive() {
@@ -338,7 +340,7 @@
triState == DependencyState.DONE, "%s %s %s", skyKey, triState, errorInfo);
state.addTemporaryDirectDeps(GroupedListHelper.create(ErrorTransienceValue.KEY));
state.signalDep(evaluatorContext.getGraphVersion(), ErrorTransienceValue.KEY);
- maxChildVersion = evaluatorContext.getGraphVersion();
+ maxTransitiveSourceVersion = null;
}
this.errorInfo = Preconditions.checkNotNull(errorInfo, skyKey);
@@ -400,7 +402,7 @@
result.put(key, valueOrNullMarker);
newlyRequestedDepsValues.put(key, valueOrNullMarker);
if (valueOrNullMarker != NULL_MARKER) {
- maybeUpdateMaxChildVersion(depEntry);
+ maybeUpdateMaxTransitiveSourceVersion(depEntry);
}
}
return result;
@@ -452,7 +454,7 @@
result.set(i, valueOrNullMarker);
newlyRequestedDepsValues.put(key, valueOrNullMarker);
if (valueOrNullMarker != NULL_MARKER) {
- maybeUpdateMaxChildVersion(depEntry);
+ maybeUpdateMaxTransitiveSourceVersion(depEntry);
}
}
return result;
@@ -535,7 +537,7 @@
Preconditions.checkState(!assertDone, "%s had not done: %s", skyKey, key);
continue;
}
- maybeUpdateMaxChildVersion(depEntry);
+ maybeUpdateMaxTransitiveSourceVersion(depEntry);
result.add(valueOrNullMarker);
}
return result;
@@ -834,33 +836,23 @@
}
}
- Version evaluationVersion = maxChildVersion;
- if (bubbleErrorInfo != null) {
- // Cycles can lead to a state where the versions of done children don't accurately reflect the
- // state that led to this node's value. Be conservative then.
- evaluationVersion = evaluatorContext.getGraphVersion();
- } else if (injectedVersion != null) {
- evaluationVersion = injectedVersion;
- } else if (evaluatorContext.getEvaluationVersionBehavior()
- == EvaluationVersionBehavior.GRAPH_VERSION
- || skyKey.functionName().getHermeticity() == FunctionHermeticity.NONHERMETIC) {
- evaluationVersion = evaluatorContext.getGraphVersion();
- } else if (evaluationVersion == null) {
- Preconditions.checkState(
- temporaryDirectDeps.isEmpty(),
- "No max child version found, but have direct deps: %s %s",
- skyKey,
- primaryEntry);
- evaluationVersion = evaluatorContext.getGraphVersion();
+ if (temporaryDirectDeps.isEmpty()
+ && skyKey.functionName().getHermeticity() != FunctionHermeticity.NONHERMETIC) {
+ maxTransitiveSourceVersion = null; // No dependencies on source.
}
- Version previousVersion = primaryEntry.getVersion();
- // If this entry is dirty, setValue may not actually change it, if it determines that
- // the data being written now is the same as the data already present in the entry.
- Set<SkyKey> reverseDeps = primaryEntry.setValue(valueWithMetadata, evaluationVersion);
+ Preconditions.checkState(
+ maxTransitiveSourceVersion == null || newlyRegisteredDeps.isEmpty(),
+ "Dependency registration not supported when tracking max transitive source versions");
- // Note that if this update didn't actually change the entry, this version may not be
- // evaluationVersion.
+ // If this entry is dirty, setValue may not actually change it, if it determines that the data
+ // being written now is the same as the data already present in the entry. We detect this case
+ // by comparing versions before and after setting the value.
+ Version previousVersion = primaryEntry.getVersion();
+ Set<SkyKey> reverseDeps =
+ primaryEntry.setValue(
+ valueWithMetadata, evaluatorContext.getGraphVersion(), maxTransitiveSourceVersion);
Version currentVersion = primaryEntry.getVersion();
+
// Tell the receiver that this value was built. If currentVersion.equals(evaluationVersion), it
// was evaluated this run, and so was changed. Otherwise, it is less than evaluationVersion, by
// the Preconditions check above, and was not actually changed this run -- when it was written
@@ -904,9 +896,6 @@
@Override
public void registerDependencies(Iterable<SkyKey> keys) {
- Preconditions.checkState(
- evaluatorContext.getEvaluationVersionBehavior() == EvaluationVersionBehavior.GRAPH_VERSION,
- "Dependency registration not supported when tracking max child versions");
newlyRequestedDeps.startGroup();
for (SkyKey key : keys) {
if (!previouslyRequestedDepsValues.containsKey(key)) {
@@ -921,17 +910,30 @@
public void injectVersionForNonHermeticFunction(Version version) {
Preconditions.checkState(
skyKey.functionName().getHermeticity() == FunctionHermeticity.NONHERMETIC, skyKey);
- injectedVersion = version;
+ Preconditions.checkState(
+ maxTransitiveSourceVersion == null,
+ "Multiple injected versions (%s, %s) for %s",
+ maxTransitiveSourceVersion,
+ version,
+ skyKey);
+ Preconditions.checkNotNull(version, skyKey);
+ Preconditions.checkState(
+ version.atMost(evaluatorContext.getGraphVersion()),
+ "Invalid injected version (%s > %s) for %s",
+ version,
+ evaluatorContext.getGraphVersion(),
+ skyKey);
+ maxTransitiveSourceVersion = version;
}
- private void maybeUpdateMaxChildVersion(NodeEntry depEntry) {
- if (skyKey.functionName().getHermeticity() != FunctionHermeticity.NONHERMETIC
- && evaluatorContext.getEvaluationVersionBehavior()
- == EvaluationVersionBehavior.MAX_CHILD_VERSIONS) {
- Version depVersion = depEntry.getVersion();
- if (maxChildVersion == null || maxChildVersion.atMost(depVersion)) {
- maxChildVersion = depVersion;
- }
+ private void maybeUpdateMaxTransitiveSourceVersion(NodeEntry depEntry) {
+ if (maxTransitiveSourceVersion == null
+ || skyKey.functionName().getHermeticity() == FunctionHermeticity.NONHERMETIC) {
+ return;
+ }
+ Version depMtsv = depEntry.getMaxTransitiveSourceVersion();
+ if (depMtsv == null || maxTransitiveSourceVersion.atMost(depMtsv)) {
+ maxTransitiveSourceVersion = depMtsv;
}
}
@@ -948,8 +950,7 @@
.add("newlyRequestedDeps", newlyRequestedDeps)
.add("childErrorInfos", childErrorInfos)
.add("depErrorKey", depErrorKey)
- .add("maxChildVersion", maxChildVersion)
- .add("injectedVersion", injectedVersion)
+ .add("maxTransitiveSourceVersion", maxTransitiveSourceVersion)
.add("bubbleErrorInfo", bubbleErrorInfo)
.add("evaluatorContext", evaluatorContext)
.toString();
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
index 7766cb3..8a23bd5 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
@@ -385,7 +385,7 @@
nodeEntry.addReverseDepAndCheckIfDone(null);
nodeEntry.markRebuilding();
try {
- nodeEntry.setValue(value, ignored -> false);
+ nodeEntry.setValue(value, ignored -> false, null);
} catch (InterruptedException e) {
throw new RuntimeException();
}
diff --git a/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java b/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java
index 0519adc..f9922b9 100644
--- a/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java
+++ b/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java
@@ -153,9 +153,11 @@
}
@Override
- public Set<SkyKey> setValue(SkyValue value, Version version) throws InterruptedException {
+ public Set<SkyKey> setValue(
+ SkyValue value, Version graphVersion, @Nullable Version maxTransitiveSourceVersion)
+ throws InterruptedException {
TreeSet<SkyKey> result = new TreeSet<>(ALPHABETICAL_SKYKEY_COMPARATOR);
- result.addAll(super.setValue(value, version));
+ result.addAll(super.setValue(value, graphVersion, maxTransitiveSourceVersion));
return result;
}
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 9d26f88..c50a1dd 100644
--- a/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java
@@ -145,8 +145,7 @@
new DirtyTrackingProgressReceiver(null),
GraphInconsistencyReceiver.THROWING,
() -> AbstractQueueVisitor.createExecutorService(200, "test-pool"),
- new SimpleCycleDetector(),
- EvaluationVersionBehavior.MAX_CHILD_VERSIONS);
+ new SimpleCycleDetector());
graphVersion = graphVersion.next();
return evaluator.eval(ImmutableList.copyOf(keys));
}
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 1445f18..8535a6b 100644
--- a/src/test/java/com/google/devtools/build/skyframe/GraphTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/GraphTest.java
@@ -211,7 +211,7 @@
waitForStart.countDown();
waitForAddedRdep.await(TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
entry.markRebuilding();
- entry.setValue(new StringValue("foo1"), startingVersion);
+ entry.setValue(new StringValue("foo1"), startingVersion, null);
waitForSetValue.countDown();
wrapper.waitForTasksAndMaybeThrow();
assertThat(ExecutorUtil.interruptibleShutdown(pool)).isFalse();
@@ -227,7 +227,7 @@
sameEntry.markDirty(DirtyType.CHANGE);
startEvaluation(sameEntry);
sameEntry.markRebuilding();
- sameEntry.setValue(new StringValue("foo2"), getNextVersion(startingVersion));
+ sameEntry.setValue(new StringValue("foo2"), getNextVersion(startingVersion), null);
assertThat(graph.get(null, Reason.OTHER, key).getValue()).isEqualTo(new StringValue("foo2"));
if (checkRdeps()) {
assertThat(graph.get(null, Reason.OTHER, key).getReverseDepsForDoneEntry())
@@ -284,7 +284,7 @@
entry.markRebuilding();
assertThat(valuesSet.add(key)).isTrue();
// Set to done.
- entry.setValue(new StringValue("bar" + keyNum), startingVersion);
+ entry.setValue(new StringValue("bar" + keyNum), startingVersion, null);
assertThat(entry.isDone()).isTrue();
}
} catch (InterruptedException e) {
@@ -335,7 +335,7 @@
NodeEntry entry = entries.get(key("foo" + i));
startEvaluation(entry);
entry.markRebuilding();
- entry.setValue(new StringValue("bar"), startingVersion);
+ entry.setValue(new StringValue("bar"), startingVersion, null);
}
assertThat(graph.get(null, Reason.OTHER, key("foo" + 0))).isNotNull();
@@ -380,7 +380,7 @@
Version nextVersion = getNextVersion(startingVersion);
entry.signalDep(nextVersion, dep);
- entry.setValue(new StringValue("bar" + keyNum), nextVersion);
+ entry.setValue(new StringValue("bar" + keyNum), nextVersion, null);
} catch (InterruptedException e) {
throw new IllegalStateException(keyNum + ", " + entry, e);
}
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 7aa4998..550bcd3 100644
--- a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
@@ -754,7 +754,7 @@
entry.markRebuilding();
addTemporaryDirectDep(entry, originalChild);
entry.signalDep(ZERO_VERSION, originalChild);
- entry.setValue(originalValue, version);
+ entry.setValue(originalValue, version, null);
entry.addReverseDepAndCheckIfDone(key("parent1"));
InMemoryNodeEntry clone1 = entry.cloneNodeEntry();
entry.addReverseDepAndCheckIfDone(key("parent2"));
@@ -768,7 +768,7 @@
addTemporaryDirectDep(clone2, newChild);
clone2.signalDep(ONE_VERSION, newChild);
clone2.markRebuilding();
- clone2.setValue(updatedValue, version.next());
+ clone2.setValue(updatedValue, version.next(), null);
assertThat(entry.getVersion()).isEqualTo(version);
assertThat(clone1.getVersion()).isEqualTo(version);
@@ -812,7 +812,7 @@
entry.signalDep(ZERO_VERSION, dep);
}
}
- entry.setValue(new IntegerValue(42), IntVersion.of(42L));
+ entry.setValue(new IntegerValue(42), IntVersion.of(42L), null);
int i = 0;
GroupedList<SkyKey> entryGroupedDirectDeps =
GroupedList.create(entry.getCompressedDirectDepsForDoneEntry());
@@ -832,7 +832,7 @@
entry.markRebuilding();
entry.addTemporaryDirectDeps(GroupedListHelper.create(dep));
entry.signalDep(ZERO_VERSION, dep);
- entry.setValue(new IntegerValue(1), ZERO_VERSION);
+ entry.setValue(new IntegerValue(1), ZERO_VERSION, null);
assertThat(entry.hasAtLeastOneDep()).isTrue();
}
@@ -844,7 +844,7 @@
.isEqualTo(DependencyState.NEEDS_SCHEDULING);
entry.markRebuilding();
entry.addTemporaryDirectDeps(new GroupedListHelper<>());
- entry.setValue(new IntegerValue(1), ZERO_VERSION);
+ entry.setValue(new IntegerValue(1), ZERO_VERSION, null);
assertThat(entry.hasAtLeastOneDep()).isFalse();
}
@@ -853,7 +853,8 @@
throws InterruptedException {
return entry.setValue(
ValueWithMetadata.normal(value, errorInfo, NO_EVENTS, NO_POSTS),
- IntVersion.of(graphVersion));
+ IntVersion.of(graphVersion),
+ null);
}
private static void addTemporaryDirectDep(NodeEntry entry, SkyKey key) {
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 409b3d9..53a3824 100644
--- a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
+++ b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
@@ -279,9 +279,11 @@
}
@Override
- public Set<SkyKey> setValue(SkyValue value, Version version) throws InterruptedException {
+ public Set<SkyKey> setValue(
+ SkyValue value, Version graphVersion, @Nullable Version maxTransitiveSourceVersion)
+ throws InterruptedException {
graphListener.accept(myKey, EventType.SET_VALUE, Order.BEFORE, value);
- Set<SkyKey> result = super.setValue(value, version);
+ Set<SkyKey> result = super.setValue(value, graphVersion, maxTransitiveSourceVersion);
graphListener.accept(myKey, EventType.SET_VALUE, Order.AFTER, value);
return result;
}
@@ -370,7 +372,7 @@
@Override
public String toString() {
- return MoreObjects.toStringHelper(this).add("delegate", getThinDelegate()).toString();
+ return MoreObjects.toStringHelper(this).add("delegate", delegate).toString();
}
}
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 cf929a9..a07e217 100644
--- a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
@@ -119,8 +119,7 @@
revalidationReceiver,
GraphInconsistencyReceiver.THROWING,
() -> AbstractQueueVisitor.createExecutorService(200, "test-pool"),
- new SimpleCycleDetector(),
- EvaluationVersionBehavior.MAX_CHILD_VERSIONS);
+ new SimpleCycleDetector());
}
private ParallelEvaluator makeEvaluator(
