Convert evaluated tracking to take a lazy SkyValue
The EvaluationProgressReceiver#evaluated method took a SkyValue,
but that parameter was only used by some of its implementations,
and only under some conditions.
Outside of tests, the main users are SkyframeBuildView's
ConfiguredTargetValueInvalidationReceiver and SkyframeBuilder's
ExecutionProgressReceiver.
The former builds up a set of built ConfiguredTarget keys when the
SkyValue is non-null and the EvaluationState is BUILT, and so its
nullity check can live behind those two conditions.
The latter cares about builting up a set of ConfiguredTargets, and
raising events on the eventBus when a TARGET_COMPLETION or
ASPECT_COMPLETION value is evaluated and is non-null. The extraction
of these values can live behind the conditions that check the type of
the SkyKey.
By making the SkyValue parameter lazy, this change enforces that it's
only accessed under these conditions.
This CL introduces a semantic change that should be small in effect.
The SkyframeBuildView keeps track of a set, dirtiedConfiguredTargetKeys,
and ConfiguredTarget keys evaluated as CLEAN were removed from it if
they had a non-null value. With this change, ConfiguredTarget keys
evaluated as CLEAN get removed regardless of whether their values are
null or non-null. The set is used to determine whether artifact conflict
search has to be rerun, and the extra removals that result from this CL
could cause fewer artifact conflict searches to run, but the only
affected searches would be those that were caused by dirtied configured
target values in error all of which were subsequently marked as clean,
which is probably rare.
--
MOS_MIGRATED_REVID=101144655
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 09296b5..f56dc56 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
@@ -16,6 +16,8 @@
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -106,6 +108,20 @@
}
};
+ private static class SkyValueSupplier implements Supplier<SkyValue> {
+
+ private final NodeEntry state;
+
+ public SkyValueSupplier(NodeEntry state) {
+ this.state = state;
+ }
+
+ @Override
+ public SkyValue get() {
+ return state.getValue();
+ }
+ }
+
private final ImmutableMap<? extends SkyFunctionName, ? extends SkyFunction> skyFunctions;
private final EventHandler reporter;
@@ -459,7 +475,7 @@
// by the Preconditions check above, and was not actually changed this run -- when it was
// written above, its version stayed below this update's version, so its value remains the
// same as before.
- progressReceiver.evaluated(skyKey, value,
+ progressReceiver.evaluated(skyKey, Suppliers.ofInstance(value),
valueVersion.equals(graphVersion) ? EvaluationState.BUILT : EvaluationState.CLEAN);
}
signalValuesAndEnqueueIfReady(enqueueParents ? visitor : null, reverseDeps, valueVersion);
@@ -670,10 +686,10 @@
// without any re-evaluation.
visitor.notifyDone(skyKey);
Set<SkyKey> reverseDeps = state.markClean();
- SkyValue value = state.getValue();
if (progressReceiver != null) {
// Tell the receiver that the value was not actually changed this run.
- progressReceiver.evaluated(skyKey, value, EvaluationState.CLEAN);
+ progressReceiver.evaluated(skyKey, new SkyValueSupplier(state),
+ EvaluationState.CLEAN);
}
if (!keepGoing && state.getErrorInfo() != null) {
if (!visitor.preventNewEvaluations()) {
@@ -920,7 +936,7 @@
// retrieve them, but top-level nodes are presumably of more interest.
// If valueVersion is not equal to graphVersion, it must be less than it (by the
// Preconditions check above), and so the node is clean.
- progressReceiver.evaluated(key, value, valueVersion.equals(graphVersion)
+ progressReceiver.evaluated(key, Suppliers.ofInstance(value), valueVersion.equals(graphVersion)
? EvaluationState.BUILT
: EvaluationState.CLEAN);
}
