Filter out events from analysis when constructing execution-phase values in Skyframe.
This avoids some unnecessary iteration over already-emitted events that can show up in profiles, and allows us to store execution-phase values a bit more compactly, since we don't need to carry around wrapper objects and nested sets everywhere.
This crucially depends on the fact that we can't build a target in the execution phase without first having analyzed it in a separate Skyframe call. Skyframe normally propagates all events/posts up the graph because it must be able to emit them if a user requests a node that only transitively depends on the node that emitted an event. However, because we do analysis in a separate Skyframe call, any warnings/posts associated with the analysis nodes will be emitted then, and we don't need to propagate them into execution.
PiperOrigin-RevId: 208767078
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 5e8f708..028d4c8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -19,6 +19,7 @@
import com.google.common.base.Function;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
import com.google.common.base.Stopwatch;
import com.google.common.base.Throwables;
import com.google.common.cache.Cache;
@@ -88,6 +89,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import com.google.devtools.build.lib.events.ErrorSensingEventHandler;
+import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.events.Reporter;
@@ -146,8 +148,10 @@
import com.google.devtools.build.skyframe.ErrorInfo;
import com.google.devtools.build.skyframe.EvaluationProgressReceiver;
import com.google.devtools.build.skyframe.EvaluationResult;
+import com.google.devtools.build.skyframe.EventFilter;
import com.google.devtools.build.skyframe.GraphInconsistencyReceiver;
import com.google.devtools.build.skyframe.ImmutableDiff;
+import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator;
import com.google.devtools.build.skyframe.Injectable;
import com.google.devtools.build.skyframe.MemoizingEvaluator;
import com.google.devtools.build.skyframe.MemoizingEvaluator.EvaluatorSupplier;
@@ -670,11 +674,45 @@
evaluatorDiffer(),
progressReceiver,
graphInconsistencyReceiver,
+ DEFAULT_FILTER_WITH_ACTIONS,
emittedEventState,
tracksStateForIncrementality());
buildDriver = getBuildDriver();
}
+ /**
+ * Use the fact that analysis of a target must occur before execution of that target, and in a
+ * separate Skyframe evaluation, to avoid propagating events from configured target nodes (and
+ * more generally action lookup nodes) to action execution nodes. We take advantage of the fact
+ * that if a node depends on an action lookup node and is not itself an action lookup node, then
+ * it is an execution-phase node: the action lookup nodes are terminal in the analysis phase.
+ */
+ private static final EventFilter DEFAULT_FILTER_WITH_ACTIONS =
+ new EventFilter() {
+ @Override
+ public boolean storeEventsAndPosts() {
+ return true;
+ }
+
+ @Override
+ public boolean apply(Event input) {
+ // Use the filtering defined in the default filter: no info/progress messages.
+ return InMemoryMemoizingEvaluator.DEFAULT_STORED_EVENT_FILTER.apply(input);
+ }
+
+ @Override
+ public Predicate<SkyKey> depEdgeFilterForEventsAndPosts(SkyKey primaryKey) {
+ if (primaryKey instanceof ActionLookupValue.ActionLookupKey) {
+ return Predicates.alwaysTrue();
+ } else {
+ return NO_ACTION_LOOKUP;
+ }
+ }
+ };
+
+ private static final Predicate<SkyKey> NO_ACTION_LOOKUP =
+ (key) -> !(key instanceof ActionLookupValue.ActionLookupKey);
+
protected SkyframeProgressReceiver newSkyframeProgressReceiver() {
return new SkyframeProgressReceiver();
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
index 242a5ac..9fad67e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
@@ -342,6 +342,7 @@
preinjectedDifferencer,
new EvaluationProgressReceiver.NullEvaluationProgressReceiver(),
GraphInconsistencyReceiver.THROWING,
+ InMemoryMemoizingEvaluator.DEFAULT_STORED_EVENT_FILTER,
new MemoizingEvaluator.EmittedEventState(),
/*keepEdges=*/ false));
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/EventFilter.java b/src/main/java/com/google/devtools/build/skyframe/EventFilter.java
index 0b859d2..3dc527e 100644
--- a/src/main/java/com/google/devtools/build/skyframe/EventFilter.java
+++ b/src/main/java/com/google/devtools/build/skyframe/EventFilter.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.skyframe;
import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
import com.google.devtools.build.lib.events.Event;
/** Filters out events which should not be stored during evaluation in {@link ParallelEvaluator}. */
@@ -23,4 +24,8 @@
* avoid doing unnecessary work when evaluating node entries.
*/
boolean storeEventsAndPosts();
+
+ default Predicate<SkyKey> depEdgeFilterForEventsAndPosts(SkyKey primaryKey) {
+ return Predicates.alwaysTrue();
+ }
}
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 e8f9a91..42ab317 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
@@ -63,6 +63,7 @@
private final InvalidationState 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.
@@ -90,6 +91,7 @@
differencer,
progressReceiver,
GraphInconsistencyReceiver.THROWING,
+ DEFAULT_STORED_EVENT_FILTER,
new EmittedEventState(),
true);
}
@@ -99,12 +101,14 @@
Differencer differencer,
@Nullable EvaluationProgressReceiver progressReceiver,
GraphInconsistencyReceiver graphInconsistencyReceiver,
+ EventFilter eventFilter,
EmittedEventState emittedEventState,
boolean keepEdges) {
this.skyFunctions = ImmutableMap.copyOf(skyFunctions);
this.differencer = Preconditions.checkNotNull(differencer);
this.progressReceiver = new DirtyTrackingProgressReceiver(progressReceiver);
this.graphInconsistencyReceiver = Preconditions.checkNotNull(graphInconsistencyReceiver);
+ this.eventFilter = eventFilter;
this.graph = new InMemoryGraphImpl(keepEdges);
this.emittedEventState = emittedEventState;
this.keepEdges = keepEdges;
@@ -187,7 +191,7 @@
skyFunctions,
eventHandler,
emittedEventState,
- DEFAULT_STORED_EVENT_FILTER,
+ eventFilter,
ErrorInfoManager.UseChildErrorInfoIfNecessary.INSTANCE,
keepGoing,
numThreads,
diff --git a/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java
index cc0332a..73f39ac 100644
--- a/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java
@@ -166,6 +166,7 @@
Differencer differencer,
EvaluationProgressReceiver progressReceiver,
GraphInconsistencyReceiver graphInconsistencyReceiver,
+ EventFilter eventFilter,
EmittedEventState emittedEventState,
boolean keepEdges);
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
index f879a18..fc808b0 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
@@ -266,7 +266,8 @@
NestedSet<TaggedEvents> buildAndReportEvents(NodeEntry entry, boolean expectDoneDeps)
throws InterruptedException {
- if (!evaluatorContext.getStoredEventFilter().storeEventsAndPosts()) {
+ EventFilter eventFilter = evaluatorContext.getStoredEventFilter();
+ if (!eventFilter.storeEventsAndPosts()) {
return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
}
NestedSetBuilder<TaggedEvents> eventBuilder = NestedSetBuilder.stableOrder();
@@ -278,7 +279,11 @@
GroupedList<SkyKey> depKeys = entry.getTemporaryDirectDeps();
Collection<SkyValue> deps =
getDepValuesForDoneNodeFromErrorOrDepsOrGraph(
- depKeys.getAllElementsAsIterable(), expectDoneDeps, depKeys.numElements());
+ Iterables.filter(
+ depKeys.getAllElementsAsIterable(),
+ eventFilter.depEdgeFilterForEventsAndPosts(skyKey)),
+ expectDoneDeps,
+ depKeys.numElements());
for (SkyValue value : deps) {
eventBuilder.addTransitive(ValueWithMetadata.getEvents(value));
}
@@ -289,7 +294,8 @@
NestedSet<Postable> buildAndReportPostables(NodeEntry entry, boolean expectDoneDeps)
throws InterruptedException {
- if (!evaluatorContext.getStoredEventFilter().storeEventsAndPosts()) {
+ EventFilter eventFilter = evaluatorContext.getStoredEventFilter();
+ if (!eventFilter.storeEventsAndPosts()) {
return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
}
NestedSetBuilder<Postable> postBuilder = NestedSetBuilder.stableOrder();
@@ -298,7 +304,11 @@
GroupedList<SkyKey> depKeys = entry.getTemporaryDirectDeps();
Collection<SkyValue> deps =
getDepValuesForDoneNodeFromErrorOrDepsOrGraph(
- depKeys.getAllElementsAsIterable(), expectDoneDeps, depKeys.numElements());
+ Iterables.filter(
+ depKeys.getAllElementsAsIterable(),
+ eventFilter.depEdgeFilterForEventsAndPosts(skyKey)),
+ expectDoneDeps,
+ depKeys.numElements());
for (SkyValue value : deps) {
postBuilder.addTransitive(ValueWithMetadata.getPosts(value));
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/ValueWithMetadata.java b/src/main/java/com/google/devtools/build/skyframe/ValueWithMetadata.java
index 8254f2e..805029a 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ValueWithMetadata.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ValueWithMetadata.java
@@ -279,7 +279,8 @@
return null;
}
- static NestedSet<TaggedEvents> getEvents(SkyValue value) {
+ @VisibleForTesting
+ public static NestedSet<TaggedEvents> getEvents(SkyValue value) {
if (value instanceof ValueWithMetadata) {
return ((ValueWithMetadata) value).getTransitiveEvents();
}
