diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryMemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryMemoizingEvaluator.java index 146382146e3220..f893e769e7ab9a 100644 --- a/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryMemoizingEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryMemoizingEvaluator.java @@ -22,22 +22,29 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Multiset; import com.google.common.collect.Sets; +import com.google.devtools.build.lib.concurrent.AbstractQueueVisitor; import com.google.devtools.build.lib.profiler.AutoProfiler; import com.google.devtools.build.lib.profiler.GoogleAutoProfilerUtils; +import com.google.devtools.build.lib.profiler.Profiler; +import com.google.devtools.build.lib.profiler.SilentCloseable; +import com.google.devtools.build.skyframe.Differencer.Diff; import com.google.devtools.build.skyframe.Differencer.DiffWithDelta.Delta; import com.google.devtools.build.skyframe.InvalidatingNodeVisitor.DeletingInvalidationState; import com.google.devtools.build.skyframe.InvalidatingNodeVisitor.DirtyingInvalidationState; import com.google.devtools.build.skyframe.InvalidatingNodeVisitor.InvalidationState; +import com.google.errorprone.annotations.ForOverride; import java.io.PrintStream; import java.time.Duration; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashSet; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiConsumer; import java.util.function.Predicate; import javax.annotation.Nullable; @@ -53,13 +60,13 @@ public abstract class AbstractInMemoryMemoizingEvaluator implements MemoizingEva protected final DirtyAndInflightTrackingProgressReceiver progressReceiver; // State related to invalidation and deletion. - protected Set valuesToDelete = new LinkedHashSet<>(); + private Set valuesToDelete = new LinkedHashSet<>(); private Set valuesToDirty = new LinkedHashSet<>(); - protected Map valuesToInject = new HashMap<>(); + private Map valuesToInject = new HashMap<>(); private final DeletingInvalidationState deleterState = new DeletingInvalidationState(); - protected final Differencer differencer; + private final Differencer differencer; protected final GraphInconsistencyReceiver graphInconsistencyReceiver; - protected final EventFilter eventFilter; + private final EventFilter eventFilter; /** * Whether to store edges in the graph. Can be false to save memory, in which case incremental @@ -67,14 +74,22 @@ public abstract class AbstractInMemoryMemoizingEvaluator implements MemoizingEva */ protected final boolean keepEdges; + private final Version minimalVersion; + // Values that the caller explicitly specified are assumed to be changed -- they will be // re-evaluated even if none of their children are changed. private final InvalidationState invalidatorState = new DirtyingInvalidationState(); - protected final EmittedEventState emittedEventState; + private final EmittedEventState emittedEventState; // Null until the first incremental evaluation completes. Always null when not keeping edges. - @Nullable protected IntVersion lastGraphVersion = null; + @Nullable private IntVersion lastGraphVersion = null; + + private final AtomicBoolean evaluating = new AtomicBoolean(false); + + private Set latestTopLevelEvaluations = new HashSet<>(); + + private boolean skyfocusEnabled; protected AbstractInMemoryMemoizingEvaluator( ImmutableMap skyFunctions, @@ -83,7 +98,8 @@ protected AbstractInMemoryMemoizingEvaluator( EventFilter eventFilter, EmittedEventState emittedEventState, GraphInconsistencyReceiver graphInconsistencyReceiver, - boolean keepEdges) { + boolean keepEdges, + Version minimalVersion) { this.skyFunctions = checkNotNull(skyFunctions); this.differencer = checkNotNull(differencer); this.progressReceiver = checkNotNull(progressReceiver); @@ -91,6 +107,90 @@ protected AbstractInMemoryMemoizingEvaluator( this.eventFilter = checkNotNull(eventFilter); this.graphInconsistencyReceiver = checkNotNull(graphInconsistencyReceiver); this.keepEdges = keepEdges; + this.minimalVersion = checkNotNull(minimalVersion); + } + + @Override + public EvaluationResult evaluate( + Iterable roots, EvaluationContext evaluationContext) + throws InterruptedException { + // NOTE: Performance critical code. See bug "Null build performance parity". + Version graphVersion = getNextGraphVersion(); + setAndCheckEvaluateState(true, roots); + + // Only remember roots for Skyfocus if we're tracking incremental states by keeping edges. + if (keepEdges && skyfocusEnabled) { + // Remember the top level evaluation of the build invocation for post-build consumption. + Iterables.addAll(latestTopLevelEvaluations, roots); + } + + // Mark for removal any nodes from the previous evaluation that were still inflight or were + // rewound but did not complete successfully. When the invalidator runs, it will delete the + // reverse transitive closure. + valuesToDelete.addAll(progressReceiver.getAndClearInflightKeys()); + valuesToDelete.addAll(progressReceiver.getAndClearUnsuccessfullyRewoundKeys()); + try { + // The RecordingDifferencer implementation is not quite working as it should be at this point. + // It clears the internal data structures after getDiff is called and will not return + // diffs for historical versions. This makes the following code sensitive to interrupts. + // Ideally we would simply not update lastGraphVersion if an interrupt occurs. + Diff diff = + differencer.getDiff( + new DelegatingWalkableGraph(getInMemoryGraph()), lastGraphVersion, graphVersion); + if (!diff.isEmpty() || !valuesToInject.isEmpty() || !valuesToDelete.isEmpty()) { + valuesToInject.putAll(diff.changedKeysWithNewValues()); + invalidate(diff.changedKeysWithoutNewValues()); + pruneInjectedValues(valuesToInject); + invalidate(valuesToInject.keySet()); + + performInvalidation(); + injectValues(graphVersion); + } + ProcessableGraph graph = getGraphForEvaluation(evaluationContext); + + EvaluationResult result; + try (SilentCloseable c = Profiler.instance().profile("ParallelEvaluator.eval")) { + ParallelEvaluator evaluator = + new ParallelEvaluator( + graph, + graphVersion, + minimalVersion, + skyFunctions, + evaluationContext.getEventHandler(), + emittedEventState, + eventFilter, + ErrorInfoManager.UseChildErrorInfoIfNecessary.INSTANCE, + evaluationContext.getKeepGoing(), + progressReceiver, + graphInconsistencyReceiver, + evaluationContext + .getExecutor() + .orElseGet( + () -> + AbstractQueueVisitor.create( + "skyframe-evaluator", + evaluationContext.getParallelism(), + ParallelEvaluatorErrorClassifier.instance())), + new SimpleCycleDetector(), + evaluationContext.getUnnecessaryTemporaryStateDropperReceiver()); + result = evaluator.eval(roots); + } + return EvaluationResult.builder() + .mergeFrom(result) + .setWalkableGraph(new DelegatingWalkableGraph(getInMemoryGraph())) + .build(); + } finally { + if (keepEdges) { + lastGraphVersion = (IntVersion) graphVersion; + } + setAndCheckEvaluateState(false, roots); + } + } + + @ForOverride + protected ProcessableGraph getGraphForEvaluation(EvaluationContext evaluationContext) + throws InterruptedException { + return getInMemoryGraph(); } @Override @@ -109,6 +209,21 @@ public final void delete(Predicate deletePredicate) { } } + private void setAndCheckEvaluateState(boolean newValue, Iterable roots) { + checkState( + evaluating.getAndSet(newValue) != newValue, "Re-entrant evaluation for request: %s", roots); + } + + @Override + public void setSkyfocusEnabled(boolean enabled) { + this.skyfocusEnabled = enabled; + } + + @Override + public boolean skyfocusSupported() { + return true; + } + @Override public final void deleteDirty(long versionAgeLimit) { checkArgument(versionAgeLimit >= 0, versionAgeLimit); @@ -264,7 +379,7 @@ public final void cleanupInterningPools() { getInMemoryGraph().cleanupInterningPools(); } - protected final void invalidate(Iterable diff) { + private void invalidate(Iterable diff) { Iterables.addAll(valuesToDirty, diff); } @@ -272,7 +387,7 @@ protected final void invalidate(Iterable diff) { * Removes entries in {@code valuesToInject} whose values are equal to the present values in the * graph. */ - protected final void pruneInjectedValues(Map valuesToInject) { + private void pruneInjectedValues(Map valuesToInject) { for (Iterator> it = valuesToInject.entrySet().iterator(); it.hasNext(); ) { Map.Entry entry = it.next(); SkyKey key = entry.getKey(); @@ -308,7 +423,7 @@ protected final void pruneInjectedValues(Map valuesToInject) { } /** Injects values in {@code valuesToInject} into the graph. */ - protected final void injectValues(Version version) { + private void injectValues(Version version) { if (valuesToInject.isEmpty()) { return; } @@ -321,7 +436,7 @@ protected final void injectValues(Version version) { valuesToInject = new HashMap<>(); } - protected final void performInvalidation() throws InterruptedException { + private void performInvalidation() throws InterruptedException { EagerInvalidator.delete( getInMemoryGraph(), valuesToDelete, progressReceiver, deleterState, keepEdges); // Note that clearing the valuesToDelete would not do an internal resizing. Therefore, if any @@ -335,7 +450,7 @@ protected final void performInvalidation() throws InterruptedException { valuesToDirty = new LinkedHashSet<>(); } - protected final Version getNextGraphVersion() { + private Version getNextGraphVersion() { if (!keepEdges) { return Version.constant(); } else if (lastGraphVersion == null) { @@ -344,4 +459,13 @@ protected final Version getNextGraphVersion() { return lastGraphVersion.next(); } } + + public Set getLatestTopLevelEvaluations() { + return latestTopLevelEvaluations; + } + + @Override + public void cleanupLatestTopLevelEvaluations() { + latestTopLevelEvaluations = new HashSet<>(); + } } 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 d6d4bd35bbdc57..2efa4c25a7de00 100644 --- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java @@ -15,18 +15,11 @@ import static com.google.common.base.Preconditions.checkState; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.concurrent.AbstractQueueVisitor; import com.google.devtools.build.lib.events.ExtendedEventHandler; -import com.google.devtools.build.lib.profiler.Profiler; -import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.util.TestType; -import com.google.devtools.build.skyframe.Differencer.Diff; -import java.util.HashSet; import java.util.Map; -import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; /** * An in-memory {@link MemoizingEvaluator} that uses the eager invalidation strategy. This class is, @@ -41,11 +34,6 @@ public final class InMemoryMemoizingEvaluator extends AbstractInMemoryMemoizingE // Not final only for testing. private InMemoryGraph graph; - private final AtomicBoolean evaluating = new AtomicBoolean(false); - - private Set latestTopLevelEvaluations = new HashSet<>(); - private boolean skyfocusEnabled; - public InMemoryMemoizingEvaluator( Map skyFunctions, Differencer differencer) { this(skyFunctions, differencer, EvaluationProgressReceiver.NULL); @@ -82,95 +70,14 @@ public InMemoryMemoizingEvaluator( eventFilter, emittedEventState, graphInconsistencyReceiver, - keepEdges); + keepEdges, + Version.minimal()); this.graph = keepEdges ? InMemoryGraph.create(usePooledInterning) : InMemoryGraph.createEdgeless(usePooledInterning); } - @Override - public EvaluationResult evaluate( - Iterable roots, EvaluationContext evaluationContext) - throws InterruptedException { - // NOTE: Performance critical code. See bug "Null build performance parity". - Version graphVersion = getNextGraphVersion(); - setAndCheckEvaluateState(true, roots); - - // Only remember roots for Skyfocus if we're tracking incremental states by keeping edges. - if (keepEdges && skyfocusEnabled) { - // Remember the top level evaluation of the build invocation for post-build consumption. - Iterables.addAll(latestTopLevelEvaluations, roots); - } - - // Mark for removal any nodes from the previous evaluation that were still inflight or were - // rewound but did not complete successfully. When the invalidator runs, it will delete the - // reverse transitive closure. - valuesToDelete.addAll(progressReceiver.getAndClearInflightKeys()); - valuesToDelete.addAll(progressReceiver.getAndClearUnsuccessfullyRewoundKeys()); - try { - // The RecordingDifferencer implementation is not quite working as it should be at this point. - // It clears the internal data structures after getDiff is called and will not return - // diffs for historical versions. This makes the following code sensitive to interrupts. - // Ideally we would simply not update lastGraphVersion if an interrupt occurs. - Diff diff = - differencer.getDiff(new DelegatingWalkableGraph(graph), lastGraphVersion, graphVersion); - if (!diff.isEmpty() || !valuesToInject.isEmpty() || !valuesToDelete.isEmpty()) { - valuesToInject.putAll(diff.changedKeysWithNewValues()); - invalidate(diff.changedKeysWithoutNewValues()); - pruneInjectedValues(valuesToInject); - invalidate(valuesToInject.keySet()); - - performInvalidation(); - injectValues(graphVersion); - } - - EvaluationResult result; - try (SilentCloseable c = Profiler.instance().profile("ParallelEvaluator.eval")) { - ParallelEvaluator evaluator = - new ParallelEvaluator( - graph, - graphVersion, - Version.minimal(), - skyFunctions, - evaluationContext.getEventHandler(), - emittedEventState, - eventFilter, - ErrorInfoManager.UseChildErrorInfoIfNecessary.INSTANCE, - evaluationContext.getKeepGoing(), - progressReceiver, - graphInconsistencyReceiver, - evaluationContext - .getExecutor() - .orElseGet( - () -> - AbstractQueueVisitor.create( - "skyframe-evaluator", - evaluationContext.getParallelism(), - ParallelEvaluatorErrorClassifier.instance())), - new SimpleCycleDetector(), - evaluationContext.getUnnecessaryTemporaryStateDropperReceiver()); - result = evaluator.eval(roots); - } - return EvaluationResult.builder() - .mergeFrom(result) - .setWalkableGraph(new DelegatingWalkableGraph(graph)) - .build(); - } finally { - if (keepEdges) { - lastGraphVersion = (IntVersion) graphVersion; - } - setAndCheckEvaluateState(false, roots); - } - } - - private void setAndCheckEvaluateState(boolean newValue, Object requestInfo) { - checkState( - evaluating.getAndSet(newValue) != newValue, - "Re-entrant evaluation for request: %s", - requestInfo); - } - @Override public void postLoggingStats(ExtendedEventHandler eventHandler) { eventHandler.post(new SkyframeGraphStatsEvent(graph.valuesSize())); @@ -182,30 +89,12 @@ public void injectGraphTransformerForTesting(GraphTransformerForTesting transfor this.graph = transformer.transform(this.graph); } - @Override - public boolean skyfocusSupported() { - return true; - } - @Override public InMemoryGraph getInMemoryGraph() { return graph; } - public Set getLatestTopLevelEvaluations() { - return latestTopLevelEvaluations; - } - - @Override - public void cleanupLatestTopLevelEvaluations() { - latestTopLevelEvaluations = new HashSet<>(); - } - - @Override - public void setSkyfocusEnabled(boolean enabled) { - this.skyfocusEnabled = enabled; - } - + @VisibleForTesting public ImmutableMap getSkyFunctionsForTesting() { return skyFunctions; }