Refactor tracking of `EvaluationStats` into `DirtyAndInflightTrackingProgressReceiver` so that alternate evaluator implementations can get the stats.
PiperOrigin-RevId: 842988880
Change-Id: I916c903cadcdc1ee339bcd5b449ece842879a197
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 8b9c261..93444a7 100644
--- a/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryMemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryMemoizingEvaluator.java
@@ -26,6 +26,7 @@
import com.google.common.collect.Multisets;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.concurrent.AbstractQueueVisitor;
+import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.profiler.AutoProfiler;
import com.google.devtools.build.lib.profiler.GoogleAutoProfilerUtils;
import com.google.devtools.build.lib.profiler.Profiler;
@@ -35,6 +36,7 @@
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.SkyframeGraphStatsEvent.EvaluationStats;
import com.google.errorprone.annotations.ForOverride;
import java.io.PrintStream;
import java.time.Duration;
@@ -510,4 +512,11 @@
public void cleanupLatestTopLevelEvaluations() {
latestTopLevelEvaluations = new HashSet<>();
}
+
+ @Override
+ public void postLoggingStats(ExtendedEventHandler eventHandler) {
+ EvaluationStats evaluationStats = progressReceiver.aggregateAndReset();
+ eventHandler.post(
+ new SkyframeGraphStatsEvent(getInMemoryGraph().valuesSize(), evaluationStats));
+ }
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/DirtyAndInflightTrackingProgressReceiver.java b/src/main/java/com/google/devtools/build/skyframe/DirtyAndInflightTrackingProgressReceiver.java
index cc4a952..256659d 100644
--- a/src/main/java/com/google/devtools/build/skyframe/DirtyAndInflightTrackingProgressReceiver.java
+++ b/src/main/java/com/google/devtools/build/skyframe/DirtyAndInflightTrackingProgressReceiver.java
@@ -14,11 +14,17 @@
package com.google.devtools.build.skyframe;
import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
+import com.google.common.collect.ConcurrentHashMultiset;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Multiset;
import com.google.common.collect.Sets;
import com.google.devtools.build.skyframe.NodeEntry.DirtyType;
+import com.google.devtools.build.skyframe.SkyframeGraphStatsEvent.EvaluationStats;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.Nullable;
/**
@@ -31,8 +37,30 @@
private Set<SkyKey> inflightKeys = Sets.newConcurrentHashSet();
private Set<SkyKey> unsuccessfullyRewoundKeys = Sets.newConcurrentHashSet();
+ // Nodes that were dirtied because one of their transitive dependencies changed
+ private final ConcurrentHashMultiset<SkyFunctionName> dirtied;
+ // Nodes that were dirtied because they themselves changed (for example, a leaf node that
+ // represents a file and that changed between builds)
+ private final ConcurrentHashMultiset<SkyFunctionName> changed;
+ // Nodes that were built and found different from the previous version
+ private final ConcurrentHashMultiset<SkyFunctionName> built;
+ // Nodes that were built and found to be same as the previous version
+ private final ConcurrentHashMultiset<SkyFunctionName> cleaned;
+ // Nodes that were computed during the build
+ private final ConcurrentHashMultiset<SkyFunctionName> evaluated;
+
+ private static ConcurrentHashMultiset<SkyFunctionName> createMultiset() {
+ return ConcurrentHashMultiset.create(
+ new ConcurrentHashMap<>(Runtime.getRuntime().availableProcessors(), 0.75f));
+ }
+
public DirtyAndInflightTrackingProgressReceiver(EvaluationProgressReceiver progressReceiver) {
this.progressReceiver = checkNotNull(progressReceiver);
+ this.dirtied = createMultiset();
+ this.changed = createMultiset();
+ this.built = createMultiset();
+ this.cleaned = createMultiset();
+ this.evaluated = createMultiset();
}
@Override
@@ -46,6 +74,12 @@
public void dirtied(SkyKey skyKey, DirtyType dirtyType) {
progressReceiver.dirtied(skyKey, dirtyType);
addToDirtySet(skyKey, dirtyType);
+
+ switch (dirtyType) {
+ case DIRTY -> dirtied.add(skyKey.functionName());
+ case CHANGE -> changed.add(skyKey.functionName());
+ case REWIND -> {}
+ }
}
@Override
@@ -103,6 +137,9 @@
@Override
public void stateEnding(SkyKey skyKey, NodeState nodeState) {
progressReceiver.stateEnding(skyKey, nodeState);
+ if (nodeState == NodeState.COMPUTE) {
+ evaluated.add(skyKey.functionName());
+ }
}
@Override
@@ -124,6 +161,14 @@
// Leave unsuccessful keys in unsuccessfullyRewoundKeys. Only remove them from dirtyKeys.
dirtyKeys.remove(skyKey);
}
+
+ if (directDeps == null) {
+ // In this case, no actual evaluation work was done so let's not record it.
+ } else if (state.versionChanged()) {
+ built.add(skyKey.functionName(), 1);
+ } else {
+ cleaned.add(skyKey.functionName(), 1);
+ }
}
/** Returns if the key is enqueued for evaluation. */
@@ -156,7 +201,7 @@
* <li>evaluated to an error
* </ul>
*/
- public final Set<SkyKey> getAndClearUnsuccessfullyRewoundKeys() {
+ final Set<SkyKey> getAndClearUnsuccessfullyRewoundKeys() {
Set<SkyKey> keys = unsuccessfullyRewoundKeys;
unsuccessfullyRewoundKeys = Sets.newConcurrentHashSet();
return keys;
@@ -187,4 +232,26 @@
unsuccessfullyRewoundKeys.remove(skyKey);
}
}
+
+ private static ImmutableMap<SkyFunctionName, Integer> fromMultiset(
+ ConcurrentHashMultiset<SkyFunctionName> s) {
+ return s.entrySet().stream()
+ .collect(toImmutableMap(Multiset.Entry::getElement, Multiset.Entry::getCount));
+ }
+
+ final EvaluationStats aggregateAndReset() {
+ EvaluationStats result =
+ new EvaluationStats(
+ fromMultiset(dirtied),
+ fromMultiset(changed),
+ fromMultiset(built),
+ fromMultiset(cleaned),
+ fromMultiset(evaluated));
+ dirtied.clear();
+ changed.clear();
+ built.clear();
+ cleaned.clear();
+ evaluated.clear();
+ return result;
+ }
}
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 b1648ce..4a5d7be 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
@@ -16,15 +16,9 @@
import static com.google.common.base.Preconditions.checkState;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ConcurrentHashMultiset;
import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.util.TestType;
-import com.google.devtools.build.skyframe.NodeEntry.DirtyType;
-import com.google.devtools.build.skyframe.SkyframeGraphStatsEvent.EvaluationStats;
import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import javax.annotation.Nullable;
/**
* An in-memory {@link MemoizingEvaluator} that uses the eager invalidation strategy. This class is,
@@ -37,102 +31,6 @@
*/
public final class InMemoryMemoizingEvaluator extends AbstractInMemoryMemoizingEvaluator {
- /**
- * A progress receiver that, in addition to tracking dirty and inflight notes, also collects
- * stats.
- */
- private static final class ProgressReceiver extends DirtyAndInflightTrackingProgressReceiver {
- // Nodes that were dirtied because one of their transitive dependencies changed
- private final ConcurrentHashMultiset<SkyFunctionName> dirtied;
-
- // Nodes that were dirtied because they themselves changed (for example, a leaf node that
- // represents a file and that changed between builds)
- private final ConcurrentHashMultiset<SkyFunctionName> changed;
-
- // Nodes that were built and found different from the previous version
- private final ConcurrentHashMultiset<SkyFunctionName> built;
-
- // Nodes that were built and found to be same as the previous version
- private final ConcurrentHashMultiset<SkyFunctionName> cleaned;
-
- // Nodes that were computed during the build
- private final ConcurrentHashMultiset<SkyFunctionName> evaluated;
-
- private static ConcurrentHashMultiset<SkyFunctionName> createMultiset() {
- return ConcurrentHashMultiset.create(
- new ConcurrentHashMap<>(Runtime.getRuntime().availableProcessors(), 0.75f));
- }
-
- private ProgressReceiver(EvaluationProgressReceiver progressReceiver) {
- super(progressReceiver);
-
- dirtied = createMultiset();
- changed = createMultiset();
- built = createMultiset();
- cleaned = createMultiset();
- evaluated = createMultiset();
- }
-
- private static ImmutableMap<SkyFunctionName, Integer> fromMultiset(
- ConcurrentHashMultiset<SkyFunctionName> s) {
- return s.entrySet().stream()
- .collect(ImmutableMap.toImmutableMap(e -> e.getElement(), e -> e.getCount()));
- }
-
- private EvaluationStats aggregateAndReset() {
- EvaluationStats result =
- new EvaluationStats(
- fromMultiset(dirtied),
- fromMultiset(changed),
- fromMultiset(built),
- fromMultiset(cleaned),
- fromMultiset(evaluated));
- dirtied.clear();
- changed.clear();
- built.clear();
- cleaned.clear();
- evaluated.clear();
- return result;
- }
-
- @Override
- public void dirtied(SkyKey skyKey, DirtyType dirtyType) {
- super.dirtied(skyKey, dirtyType);
-
- switch (dirtyType) {
- case DIRTY -> dirtied.add(skyKey.functionName());
- case CHANGE -> changed.add(skyKey.functionName());
- case REWIND -> {} // Should not happen but let's not crash the server due to logging
- }
- }
-
- @Override
- public void evaluated(
- SkyKey skyKey,
- EvaluationState state,
- @Nullable SkyValue newValue,
- @Nullable ErrorInfo newError,
- @Nullable GroupedDeps directDeps) {
- super.evaluated(skyKey, state, newValue, newError, directDeps);
-
- if (directDeps == null) {
- // In this case, no actual evaluation work was done so let's not record it.
- } else if (state.versionChanged()) {
- built.add(skyKey.functionName(), 1);
- } else {
- cleaned.add(skyKey.functionName(), 1);
- }
- }
-
- @Override
- public void stateEnding(SkyKey skyKey, NodeState nodeState) {
- super.stateEnding(skyKey, nodeState);
- if (nodeState == NodeState.COMPUTE) {
- evaluated.add(skyKey.functionName());
- }
- }
- }
-
// Not final only for testing.
private InMemoryGraph graph;
@@ -168,7 +66,7 @@
super(
ImmutableMap.copyOf(skyFunctions),
differencer,
- new ProgressReceiver(progressReceiver),
+ new DirtyAndInflightTrackingProgressReceiver(progressReceiver),
eventFilter,
emittedEventState,
graphInconsistencyReceiver,
@@ -181,12 +79,6 @@
}
@Override
- public void postLoggingStats(ExtendedEventHandler eventHandler) {
- EvaluationStats evaluationStats = ((ProgressReceiver) progressReceiver).aggregateAndReset();
- eventHandler.post(new SkyframeGraphStatsEvent(graph.valuesSize(), evaluationStats));
- }
-
- @Override
public void injectGraphTransformerForTesting(GraphTransformerForTesting transformer) {
checkState(TestType.isInTest());
this.graph = transformer.transform(this.graph);