diff options
Diffstat (limited to 'src/main/java')
4 files changed, 48 insertions, 53 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java index c96c178d7f..b58e57c1dd 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java @@ -258,13 +258,14 @@ public class SkyframeBuilder implements Builder { @Override public void evaluated(SkyKey skyKey, SkyValue node, EvaluationState state) { SkyFunctionName type = skyKey.functionName(); - if (type == SkyFunctions.TARGET_COMPLETION) { + if (type == SkyFunctions.TARGET_COMPLETION && node != null) { TargetCompletionValue val = (TargetCompletionValue) node; ConfiguredTarget target = val.getConfiguredTarget(); builtTargets.add(target); eventBus.post(TargetCompleteEvent.createSuccessful(target)); } else if (type == SkyFunctions.ACTION_EXECUTION) { - // Remember all completed actions, regardless of having been cached or really executed. + // Remember all completed actions, even those in error, regardless of having been cached or + // really executed. actionCompleted((Action) skyKey.argument()); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java index 857c23187f..1221f0bd92 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java @@ -495,7 +495,7 @@ public final class SkyframeBuildView { @Override public void evaluated(SkyKey skyKey, SkyValue value, EvaluationState state) { - if (skyKey.functionName() == SkyFunctions.CONFIGURED_TARGET) { + if (skyKey.functionName() == SkyFunctions.CONFIGURED_TARGET && value != null) { if (state == EvaluationState.BUILT) { evaluatedConfiguredTargets.add(skyKey); // During multithreaded operation, this is only set to true, so no concurrency issues. diff --git a/src/main/java/com/google/devtools/build/skyframe/EvaluationProgressReceiver.java b/src/main/java/com/google/devtools/build/skyframe/EvaluationProgressReceiver.java index 7928878e98..aeef2e4404 100644 --- a/src/main/java/com/google/devtools/build/skyframe/EvaluationProgressReceiver.java +++ b/src/main/java/com/google/devtools/build/skyframe/EvaluationProgressReceiver.java @@ -15,6 +15,8 @@ package com.google.devtools.build.skyframe; import com.google.devtools.build.lib.concurrent.ThreadSafety; +import javax.annotation.Nullable; + /** * Receiver to inform callers which values have been invalidated. Values may be invalidated and then * re-validated if they have been found not to be changed. @@ -66,12 +68,12 @@ public interface EvaluationProgressReceiver { void enqueueing(SkyKey skyKey); /** - * Notifies that {@code value} has been evaluated. + * Notifies that the node for {@code skyKey} has been evaluated. * - * <p>{@code state} indicates the new state of the value. + * <p>{@code state} indicates the new state of the node. * - * <p>This method is not called if the value builder threw an error when building this value. + * <p>If the value builder threw an error when building this node, then {@code value} is null. */ @ThreadSafety.ThreadSafe - void evaluated(SkyKey skyKey, SkyValue value, EvaluationState state); + void evaluated(SkyKey skyKey, @Nullable SkyValue value, EvaluationState state); } 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 a99105847b..55c1ab59c0 100644 --- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java @@ -555,43 +555,38 @@ public final class ParallelEvaluator implements Evaluator { // during a --keep_going build. NestedSet<TaggedEvents> events = buildEvents(/*missingChildren=*/false); + Version valueVersion; + SkyValue valueWithMetadata; if (value == null) { Preconditions.checkNotNull(errorInfo, "%s %s", skyKey, primaryEntry); - // We could consider using max(childVersions) here instead of graphVersion. When full - // versioning is implemented, this would allow evaluation at a version between - // max(childVersions) and graphVersion to re-use this result. - Set<SkyKey> reverseDeps = primaryEntry.setValue( - ValueWithMetadata.error(errorInfo, events), graphVersion); - signalValuesAndEnqueueIfReady(enqueueParents ? visitor : null, reverseDeps, graphVersion); + valueWithMetadata = ValueWithMetadata.error(errorInfo, events); } else { // We must be enqueueing parents if we have a value. Preconditions.checkState(enqueueParents, "%s %s", skyKey, primaryEntry); - Set<SkyKey> reverseDeps; - Version valueVersion; - // 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 could consider using max(childVersions) here instead of graphVersion. When full - // versioning is implemented, this would allow evaluation at a version between - // max(childVersions) and graphVersion to re-use this result. - reverseDeps = primaryEntry.setValue( - ValueWithMetadata.normal(value, errorInfo, events), graphVersion); - // Note that if this update didn't actually change the value entry, this version may not - // be the graph version. - valueVersion = primaryEntry.getVersion(); - Preconditions.checkState(valueVersion.atMost(graphVersion), - "%s should be at most %s in the version partial ordering", - valueVersion, graphVersion); - if (progressReceiver != null) { - // Tell the receiver that this value was built. If valueVersion.equals(graphVersion), it - // was evaluated this run, and so was changed. Otherwise, it is less than graphVersion, - // 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, - valueVersion.equals(graphVersion) ? EvaluationState.BUILT : EvaluationState.CLEAN); - } - signalValuesAndEnqueueIfReady(visitor, reverseDeps, valueVersion); - } + valueWithMetadata = ValueWithMetadata.normal(value, errorInfo, events); + } + // 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 could consider using max(childVersions) here instead of graphVersion. When full + // versioning is implemented, this would allow evaluation at a version between + // max(childVersions) and graphVersion to re-use this result. + Set<SkyKey> reverseDeps = primaryEntry.setValue(valueWithMetadata, graphVersion); + // Note that if this update didn't actually change the value entry, this version may not + // be the graph version. + valueVersion = primaryEntry.getVersion(); + Preconditions.checkState(valueVersion.atMost(graphVersion), + "%s should be at most %s in the version partial ordering", + valueVersion, graphVersion); + if (progressReceiver != null) { + // Tell the receiver that this value was built. If valueVersion.equals(graphVersion), it + // was evaluated this run, and so was changed. Otherwise, it is less than graphVersion, + // 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, + valueVersion.equals(graphVersion) ? EvaluationState.BUILT : EvaluationState.CLEAN); + } + signalValuesAndEnqueueIfReady(enqueueParents ? visitor : null, reverseDeps, valueVersion); visitor.notifyDone(skyKey); replayingNestedSetEventVisitor.visit(events); @@ -815,7 +810,7 @@ public final class ParallelEvaluator implements Evaluator { visitor.notifyDone(skyKey); Set<SkyKey> reverseDeps = state.markClean(); SkyValue value = state.getValue(); - if (progressReceiver != null && value != null) { + if (progressReceiver != null) { // Tell the receiver that the value was not actually changed this run. progressReceiver.evaluated(skyKey, value, EvaluationState.CLEAN); } @@ -1054,19 +1049,16 @@ public final class ParallelEvaluator implements Evaluator { NodeEntry entry = graph.get(key); Preconditions.checkState(entry.isDone(), entry); SkyValue value = entry.getValue(); - if (value != null) { - Version valueVersion = entry.getVersion(); - Preconditions.checkState(valueVersion.atMost(graphVersion), - "%s should be at most %s in the version partial ordering", valueVersion, graphVersion); - // Nodes with errors will have no value. Don't inform the receiver in that case. - // For most nodes we do not inform the progress receiver if they were already done - // when we 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) - ? EvaluationState.BUILT - : EvaluationState.CLEAN); - } + Version valueVersion = entry.getVersion(); + Preconditions.checkState(valueVersion.atMost(graphVersion), + "%s should be at most %s in the version partial ordering", valueVersion, graphVersion); + // For most nodes we do not inform the progress receiver if they were already done when we + // 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) + ? EvaluationState.BUILT + : EvaluationState.CLEAN); } } |