From 1221742afb61926191f1f1393e9aaa92cd50666b Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 3 Apr 2023 16:14:44 -0700 Subject: [PATCH] Push down incremental logic from `InMemoryMemoizingEvaluator` to `AbstractIncrementalInMemoryMemoizingEvaluator`. PiperOrigin-RevId: 521590383 Change-Id: I3c0f4956119edf8a41c6871095cca9382bfc6322 --- ...IncrementalInMemoryMemoizingEvaluator.java | 151 ++++++++++++++++++ .../skyframe/InMemoryMemoizingEvaluator.java | 121 ++------------ 2 files changed, 161 insertions(+), 111 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/skyframe/AbstractIncrementalInMemoryMemoizingEvaluator.java diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractIncrementalInMemoryMemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/AbstractIncrementalInMemoryMemoizingEvaluator.java new file mode 100644 index 00000000000000..19786e948bc3be --- /dev/null +++ b/src/main/java/com/google/devtools/build/skyframe/AbstractIncrementalInMemoryMemoizingEvaluator.java @@ -0,0 +1,151 @@ +// Copyright 2023 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; + +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.collect.nestedset.NestedSetVisitor; +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.devtools.build.skyframe.QueryableGraph.Reason; +import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; + +/** + * Partial implementation of {@link MemoizingEvaluator} with expanded support for incremental and + * non-incremental evaluations on an {@link InMemoryGraph}. + */ +abstract class AbstractIncrementalInMemoryMemoizingEvaluator + extends AbstractInMemoryMemoizingEvaluator { + + final ImmutableMap skyFunctions; + final DirtyTrackingProgressReceiver progressReceiver; + + // State related to invalidation and deletion. + Set valuesToDelete = new LinkedHashSet<>(); + private Set valuesToDirty = new LinkedHashSet<>(); + Map valuesToInject = new HashMap<>(); + private final DeletingInvalidationState deleterState = new DeletingInvalidationState(); + final Differencer differencer; + final GraphInconsistencyReceiver graphInconsistencyReceiver; + final EventFilter eventFilter; + + // Keep edges in graph. Can be false to save memory, in which case incremental builds are + // not possible. + private final boolean keepEdges; + + // 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(); + + final NestedSetVisitor.VisitedState emittedEventState; + + AbstractIncrementalInMemoryMemoizingEvaluator( + ImmutableMap skyFunctions, + Differencer differencer, + DirtyTrackingProgressReceiver dirtyTrackingProgressReceiver, + EventFilter eventFilter, + NestedSetVisitor.VisitedState emittedEventState, + GraphInconsistencyReceiver graphInconsistencyReceiver, + boolean keepEdges) { + this.skyFunctions = checkNotNull(skyFunctions); + this.differencer = checkNotNull(differencer); + this.progressReceiver = checkNotNull(dirtyTrackingProgressReceiver); + this.emittedEventState = checkNotNull(emittedEventState); + this.eventFilter = checkNotNull(eventFilter); + this.graphInconsistencyReceiver = checkNotNull(graphInconsistencyReceiver); + this.keepEdges = keepEdges; + } + + void invalidate(Iterable diff) { + Iterables.addAll(valuesToDirty, diff); + } + + /** + * Removes entries in {@code valuesToInject} whose values are equal to the present values in the + * graph. + */ + void pruneInjectedValues(Map valuesToInject) { + for (Iterator> it = valuesToInject.entrySet().iterator(); + it.hasNext(); ) { + Map.Entry entry = it.next(); + SkyKey key = entry.getKey(); + SkyValue newValue = entry.getValue(); + NodeEntry prevEntry = getInMemoryGraph().get(null, Reason.OTHER, key); + if (prevEntry != null && prevEntry.isDone()) { + if (keepEdges) { + try { + if (!prevEntry.hasAtLeastOneDep()) { + if (newValue.equals(prevEntry.getValue()) + && !valuesToDirty.contains(key) + && !valuesToDelete.contains(key)) { + it.remove(); + } + } else { + // Rare situation of an injected dep that depends on another node. Usually the dep is + // the error transience node. When working with external repositories, it can also be + // an external workspace file. Don't bother injecting it, just invalidate it. + // We'll wastefully evaluate the node freshly during evaluation, but this happens very + // rarely. + valuesToDirty.add(key); + it.remove(); + } + } catch (InterruptedException e) { + throw new IllegalStateException( + "InMemoryGraph does not throw: " + entry + ", " + prevEntry, e); + } + } else { + // No incrementality. Just delete the old value from the graph. The new value is about to + // be injected. + getInMemoryGraph().remove(key); + } + } + } + } + + /** Injects values in {@code valuesToInject} into the graph. */ + void injectValues(IntVersion version) { + if (valuesToInject.isEmpty()) { + return; + } + try { + ParallelEvaluator.injectValues(valuesToInject, version, getInMemoryGraph(), progressReceiver); + } catch (InterruptedException e) { + throw new IllegalStateException("InMemoryGraph doesn't throw interrupts", e); + } + // Start with a new map to avoid bloat since clear() does not downsize the map. + valuesToInject = new HashMap<>(); + } + + 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 + // build has a large set of dirty values, subsequent operations (even clearing) will be slower. + // Instead, just start afresh with a new LinkedHashSet. + valuesToDelete = new LinkedHashSet<>(); + + EagerInvalidator.invalidate( + getInMemoryGraph(), valuesToDirty, progressReceiver, invalidatorState); + // Ditto. + valuesToDirty = new LinkedHashSet<>(); + } +} 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 1081fac5d98ca4..85473b54cd14dd 100644 --- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java @@ -15,7 +15,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Iterables; import com.google.common.collect.Sets; import com.google.devtools.build.lib.collect.nestedset.NestedSetVisitor; import com.google.devtools.build.lib.concurrent.AbstractQueueVisitor; @@ -25,14 +24,8 @@ 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.InvalidatingNodeVisitor.DeletingInvalidationState; -import com.google.devtools.build.skyframe.InvalidatingNodeVisitor.DirtyingInvalidationState; -import com.google.devtools.build.skyframe.InvalidatingNodeVisitor.InvalidationState; import com.google.devtools.build.skyframe.QueryableGraph.Reason; import java.time.Duration; -import java.util.HashMap; -import java.util.Iterator; -import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; @@ -47,33 +40,12 @@ * *

This memoizing evaluator uses a monotonically increasing {@link IntVersion}. */ -public final class InMemoryMemoizingEvaluator extends AbstractInMemoryMemoizingEvaluator { - - private final ImmutableMap skyFunctions; - private final DirtyTrackingProgressReceiver progressReceiver; +public final class InMemoryMemoizingEvaluator + extends AbstractIncrementalInMemoryMemoizingEvaluator { // Not final only for testing. private InMemoryGraph graph; private IntVersion lastGraphVersion = null; - // State related to invalidation and deletion. - private Set valuesToDelete = new LinkedHashSet<>(); - private Set valuesToDirty = new LinkedHashSet<>(); - private Map valuesToInject = new HashMap<>(); - private final DeletingInvalidationState deleterState = new DeletingInvalidationState(); - private final Differencer differencer; - private final GraphInconsistencyReceiver graphInconsistencyReceiver; - private final EventFilter eventFilter; - - // Keep edges in graph. Can be false to save memory, in which case incremental builds are - // not possible. - private final boolean keepEdges; - - // 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(); - - private final NestedSetVisitor.VisitedState emittedEventState; - private final AtomicBoolean evaluating = new AtomicBoolean(false); public InMemoryMemoizingEvaluator( @@ -105,21 +77,18 @@ public InMemoryMemoizingEvaluator( NestedSetVisitor.VisitedState emittedEventState, boolean keepEdges, boolean usePooledSkyKeyInterning) { - this.skyFunctions = ImmutableMap.copyOf(skyFunctions); - this.differencer = Preconditions.checkNotNull(differencer); - this.progressReceiver = new DirtyTrackingProgressReceiver(progressReceiver); - this.graphInconsistencyReceiver = Preconditions.checkNotNull(graphInconsistencyReceiver); - this.eventFilter = eventFilter; + super( + ImmutableMap.copyOf(skyFunctions), + differencer, + new DirtyTrackingProgressReceiver(progressReceiver), + eventFilter, + emittedEventState, + graphInconsistencyReceiver, + keepEdges); this.graph = keepEdges ? InMemoryGraph.create(usePooledSkyKeyInterning) : InMemoryGraph.createEdgeless(usePooledSkyKeyInterning); - this.emittedEventState = emittedEventState; - this.keepEdges = keepEdges; - } - - private void invalidate(Iterable diff) { - Iterables.addAll(valuesToDirty, diff); } private static final Duration MIN_TIME_TO_LOG_DELETION = Duration.ofMillis(10); @@ -219,76 +188,6 @@ public EvaluationResult evaluate( } } - /** - * Removes entries in {@code valuesToInject} whose values are equal to the present values in the - * graph. - */ - private void pruneInjectedValues(Map valuesToInject) { - for (Iterator> it = valuesToInject.entrySet().iterator(); - it.hasNext(); ) { - Map.Entry entry = it.next(); - SkyKey key = entry.getKey(); - SkyValue newValue = entry.getValue(); - NodeEntry prevEntry = graph.get(null, Reason.OTHER, key); - if (prevEntry != null && prevEntry.isDone()) { - if (keepEdges) { - try { - if (!prevEntry.hasAtLeastOneDep()) { - if (newValue.equals(prevEntry.getValue()) - && !valuesToDirty.contains(key) - && !valuesToDelete.contains(key)) { - it.remove(); - } - } else { - // Rare situation of an injected dep that depends on another node. Usually the dep is - // the error transience node. When working with external repositories, it can also be - // an external workspace file. Don't bother injecting it, just invalidate it. - // We'll wastefully evaluate the node freshly during evaluation, but this happens very - // rarely. - valuesToDirty.add(key); - it.remove(); - } - } catch (InterruptedException e) { - throw new IllegalStateException( - "InMemoryGraph does not throw: " + entry + ", " + prevEntry, e); - } - } else { - // No incrementality. Just delete the old value from the graph. The new value is about to - // be injected. - graph.remove(key); - } - } - } - } - - /** - * Injects values in {@code valuesToInject} into the graph. - */ - private void injectValues(IntVersion version) { - if (valuesToInject.isEmpty()) { - return; - } - try { - ParallelEvaluator.injectValues(valuesToInject, version, graph, progressReceiver); - } catch (InterruptedException e) { - throw new IllegalStateException("InMemoryGraph doesn't throw interrupts", e); - } - // Start with a new map to avoid bloat since clear() does not downsize the map. - valuesToInject = new HashMap<>(); - } - - private void performInvalidation() throws InterruptedException { - EagerInvalidator.delete(graph, valuesToDelete, progressReceiver, deleterState, keepEdges); - // Note that clearing the valuesToDelete would not do an internal resizing. Therefore, if any - // build has a large set of dirty values, subsequent operations (even clearing) will be slower. - // Instead, just start afresh with a new LinkedHashSet. - valuesToDelete = new LinkedHashSet<>(); - - EagerInvalidator.invalidate(graph, valuesToDirty, progressReceiver, invalidatorState); - // Ditto. - valuesToDirty = new LinkedHashSet<>(); - } - private void setAndCheckEvaluateState(boolean newValue, Object requestInfo) { Preconditions.checkState(evaluating.getAndSet(newValue) != newValue, "Re-entrant evaluation for request: %s", requestInfo);