Split lifecycle tracking from action context provider.
Action context providers grew some additional functionality over the years which had little to do with providing anything but instead did lifecycle management for various objects. This change extracts that behavior into a separate interface so that the the provider logic can later be removed.
RELNOTES: None
PiperOrigin-RevId: 290133070
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
index 22eedbf..d7c50bf 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -58,11 +58,11 @@
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.EventKind;
import com.google.devtools.build.lib.events.Reporter;
-import com.google.devtools.build.lib.exec.ActionContextProvider;
import com.google.devtools.build.lib.exec.BlazeExecutor;
import com.google.devtools.build.lib.exec.CheckUpToDateFilter;
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.exec.ExecutorBuilder;
+import com.google.devtools.build.lib.exec.ExecutorLifecycleListener;
import com.google.devtools.build.lib.exec.SpawnActionContextMaps;
import com.google.devtools.build.lib.exec.SymlinkTreeStrategy;
import com.google.devtools.build.lib.profiler.AutoProfiler;
@@ -121,8 +121,8 @@
private final BuildRequest request;
private BlazeExecutor executor;
private final ActionInputPrefetcher prefetcher;
- private final ImmutableList<ActionContextProvider> actionContextProviders;
- private SpawnActionContextMaps spawnActionContextMaps;
+ private final ImmutableSet<ExecutorLifecycleListener> executorLifecycleListeners;
+ private final SpawnActionContextMaps spawnActionContextMaps;
ExecutionTool(CommandEnvironment env, BuildRequest request) throws ExecutorInitException {
this.env = env;
@@ -152,7 +152,7 @@
.addStrategyByContext(SymlinkTreeActionContext.class, "");
this.prefetcher = builder.getActionInputPrefetcher();
- this.actionContextProviders = builder.getActionContextProviders();
+ this.executorLifecycleListeners = builder.getExecutorLifecycleListeners();
// There are many different SpawnActions, and we want to control the action context they use
// independently from each other, for example, to run genrules locally and Java compile action
@@ -162,7 +162,7 @@
// TODO(jmmv): This should live in some testing-related Blaze module, not here.
builder.addStrategyByContext(TestActionContext.class, options.testStrategy);
spawnActionContextMaps =
- builder.getSpawnActionContextMapsBuilder().build(actionContextProviders);
+ builder.getSpawnActionContextMapsBuilder().build(builder.getActionContextProviders());
if (options.availableResources != null && options.removeLocalResources) {
throw new ExecutorInitException(
@@ -174,6 +174,9 @@
Executor getExecutor() throws ExecutorInitException {
if (executor == null) {
executor = createExecutor();
+ for (ExecutorLifecycleListener executorLifecycleListener : executorLifecycleListeners) {
+ executorLifecycleListener.executorCreated();
+ }
}
return executor;
}
@@ -186,8 +189,7 @@
getReporter(),
runtime.getClock(),
request,
- spawnActionContextMaps,
- actionContextProviders);
+ spawnActionContextMaps);
}
void init() throws ExecutorInitException {
@@ -195,8 +197,8 @@
}
void shutdown() {
- for (ActionContextProvider actionContextProvider : actionContextProviders) {
- actionContextProvider.executionPhaseEnding();
+ for (ExecutorLifecycleListener executorLifecycleListener : executorLifecycleListeners) {
+ executorLifecycleListener.executionPhaseEnding();
}
}
@@ -294,10 +296,10 @@
}
}
- for (ActionContextProvider actionContextProvider : actionContextProviders) {
+ for (ExecutorLifecycleListener executorLifecycleListener : executorLifecycleListeners) {
try (SilentCloseable c =
- Profiler.instance().profile(actionContextProvider + ".executionPhaseStarting")) {
- actionContextProvider.executionPhaseStarting(
+ Profiler.instance().profile(executorLifecycleListener + ".executionPhaseStarting")) {
+ executorLifecycleListener.executionPhaseStarting(
actionGraph,
// If this supplier is ever consumed by more than one ActionContextProvider, it can be
// pulled out of the loop and made a memoizing supplier.
@@ -335,8 +337,8 @@
catastrophe = e;
} finally {
// These may flush logs, which may help if there is a catastrophic failure.
- for (ActionContextProvider actionContextProvider : actionContextProviders) {
- actionContextProvider.executionPhaseEnding();
+ for (ExecutorLifecycleListener executorLifecycleListener : executorLifecycleListeners) {
+ executorLifecycleListener.executionPhaseEnding();
}
// Handlers process these events and others (e.g. CommandCompleteEvent), even in the event of
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/exec/ActionContextProvider.java
index c47a57f..e4369eb 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ActionContextProvider.java
@@ -13,11 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.exec;
-import com.google.common.base.Supplier;
import com.google.devtools.build.lib.actions.ActionContext;
-import com.google.devtools.build.lib.actions.ActionGraph;
-import com.google.devtools.build.lib.actions.ExecutorInitException;
-import com.google.devtools.build.lib.analysis.ArtifactsToOwnerLabels;
/**
* An object that provides execution strategies to {@link BlazeExecutor}.
@@ -26,19 +22,6 @@
*/
public abstract class ActionContextProvider {
- /** Called when the executor is constructed. */
- public void executorCreated() throws ExecutorInitException {}
-
- /** Called when the execution phase is started. */
- public void executionPhaseStarting(
- ActionGraph actionGraph, Supplier<ArtifactsToOwnerLabels> topLevelArtifactsToOwnerLabels)
- throws ExecutorInitException, InterruptedException {}
-
- /**
- * Called when the execution phase is finished.
- */
- public void executionPhaseEnding() {}
-
/** Registers any action contexts originating with this provider with the given collector. */
public abstract void registerActionContexts(ActionContextCollector collector);
diff --git a/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java b/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java
index af5124c..96e24fa 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java
@@ -16,7 +16,6 @@
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContext.ShowSubcommands;
import com.google.devtools.build.lib.actions.Executor;
-import com.google.devtools.build.lib.actions.ExecutorInitException;
import com.google.devtools.build.lib.clock.Clock;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.events.Reporter;
@@ -61,9 +60,7 @@
Reporter reporter,
Clock clock,
OptionsProvider options,
- SpawnActionContextMaps spawnActionContextMaps,
- Iterable<ActionContextProvider> contextProviders)
- throws ExecutorInitException {
+ SpawnActionContextMaps spawnActionContextMaps) {
ExecutionOptions executionOptions = options.getOptions(ExecutionOptions.class);
this.verboseFailures = executionOptions.verboseFailures;
this.showSubcommands = executionOptions.showSubcommands;
@@ -78,10 +75,6 @@
}
spawnActionContextMaps.notifyUsed();
-
- for (ActionContextProvider factory : contextProviders) {
- factory.executorCreated();
- }
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutorBuilder.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutorBuilder.java
index 21e5b3b..bd5674b 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ExecutorBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutorBuilder.java
@@ -15,6 +15,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.Executor;
@@ -22,7 +23,9 @@
import com.google.devtools.build.lib.actions.SpawnActionContext;
import com.google.devtools.build.lib.util.RegexFilter;
import java.util.ArrayList;
+import java.util.LinkedHashSet;
import java.util.List;
+import java.util.Set;
/**
* Builder class to create an {@link Executor} instance. This class is part of the module API,
@@ -32,6 +35,7 @@
private final List<ActionContextProvider> actionContextProviders = new ArrayList<>();
private final SpawnActionContextMaps.Builder spawnActionContextMapsBuilder =
new SpawnActionContextMaps.Builder();
+ private final Set<ExecutorLifecycleListener> executorLifecycleListeners = new LinkedHashSet<>();
private ActionInputPrefetcher prefetcher;
// These methods shouldn't be public, but they have to be right now as ExecutionTool is in another
@@ -44,6 +48,11 @@
return spawnActionContextMapsBuilder;
}
+ /** Returns all executor lifecycle listeners registered with this builder so far. */
+ public ImmutableSet<ExecutorLifecycleListener> getExecutorLifecycleListeners() {
+ return ImmutableSet.copyOf(executorLifecycleListeners);
+ }
+
public ActionInputPrefetcher getActionInputPrefetcher() {
return prefetcher == null ? ActionInputPrefetcher.NONE : prefetcher;
}
@@ -157,4 +166,15 @@
this.prefetcher = Preconditions.checkNotNull(prefetcher);
return this;
}
+
+ /**
+ * Registers an executor lifecycle listener which will receive notifications throughout the
+ * execution phase (if one occurs).
+ *
+ * @see ExecutorLifecycleListener for events that can be listened to
+ */
+ public ExecutorBuilder addExecutorLifecycleListener(ExecutorLifecycleListener listener) {
+ executorLifecycleListeners.add(listener);
+ return this;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutorLifecycleListener.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutorLifecycleListener.java
new file mode 100644
index 0000000..93578d4
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutorLifecycleListener.java
@@ -0,0 +1,50 @@
+// Copyright 2020 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.lib.exec;
+
+import com.google.common.base.Supplier;
+import com.google.devtools.build.lib.actions.ActionGraph;
+import com.google.devtools.build.lib.actions.ExecutorInitException;
+import com.google.devtools.build.lib.analysis.ArtifactsToOwnerLabels;
+
+/**
+ * Type that can get informed about executor lifecycle events.
+ *
+ * <p>Notifications occur in this order:
+ *
+ * <ol>
+ * <li>{@link #executorCreated}
+ * <li>{@link #executionPhaseStarting}
+ * <li>{@link #executionPhaseEnding}
+ * </ol>
+ */
+public interface ExecutorLifecycleListener {
+
+ /** Handles executor creation. */
+ void executorCreated() throws ExecutorInitException;
+
+ /**
+ * Handles the start of the execution phase.
+ *
+ * @param actionGraph actions as calcuated in the analysis phase
+ * @param topLevelArtifactsToOwnerLabels supplier of all output artifacts from top-level targets
+ * and aspects, mapped to their owners
+ */
+ void executionPhaseStarting(
+ ActionGraph actionGraph, Supplier<ArtifactsToOwnerLabels> topLevelArtifactsToOwnerLabels)
+ throws ExecutorInitException, InterruptedException;
+
+ /** Handles the end of the execution phase. */
+ void executionPhaseEnding();
+}
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
index 44127d1..77f98e4 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
@@ -36,6 +36,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadHostile;
import com.google.devtools.build.lib.exec.ActionContextProvider;
import com.google.devtools.build.lib.exec.ExecutorBuilder;
+import com.google.devtools.build.lib.exec.ExecutorLifecycleListener;
import com.google.devtools.build.lib.includescanning.IncludeParser.Inclusion;
import com.google.devtools.build.lib.rules.cpp.CppIncludeExtractionContext;
import com.google.devtools.build.lib.rules.cpp.CppIncludeScanningContext;
@@ -82,8 +83,10 @@
@Override
@ThreadHostile
public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
- builder.addActionContextProvider(
- new IncludeScanningActionContextProvider(env, request, spawnIncludeScannerSupplier));
+ IncludeScanningActionContextProvider provider =
+ new IncludeScanningActionContextProvider(env, request, spawnIncludeScannerSupplier);
+ builder.addActionContextProvider(provider);
+ builder.addExecutorLifecycleListener(provider);
builder
.addStrategyByContext(CppIncludeExtractionContext.class, "")
.addStrategyByContext(SwigIncludeScanningContext.class, "")
@@ -212,10 +215,9 @@
}
}
- /**
- * Factory for execution strategies related to include scanning.
- */
- public static class IncludeScanningActionContextProvider extends ActionContextProvider {
+ /** Factory for execution strategies related to include scanning. */
+ public static class IncludeScanningActionContextProvider extends ActionContextProvider
+ implements ExecutorLifecycleListener {
private final CommandEnvironment env;
private final BuildRequest buildRequest;
@@ -276,6 +278,9 @@
}
@Override
+ public void executionPhaseEnding() {}
+
+ @Override
public void executorCreated() throws ExecutorInitException {
IncludeScanningOptions options = buildRequest.getOptions(IncludeScanningOptions.class);
int threads = options.includeScanningParallelism;
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
index bbd2466..159ef561 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
@@ -16,12 +16,17 @@
import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.base.Preconditions;
+import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.ListeningScheduledExecutorService;
+import com.google.devtools.build.lib.actions.ActionGraph;
import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ExecutorInitException;
import com.google.devtools.build.lib.actions.SpawnActionContext;
+import com.google.devtools.build.lib.analysis.ArtifactsToOwnerLabels;
import com.google.devtools.build.lib.exec.ActionContextProvider;
import com.google.devtools.build.lib.exec.ExecutionOptions;
+import com.google.devtools.build.lib.exec.ExecutorLifecycleListener;
import com.google.devtools.build.lib.exec.SpawnCache;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.DigestUtil;
@@ -29,10 +34,9 @@
import com.google.devtools.build.lib.vfs.Path;
import javax.annotation.Nullable;
-/**
- * Provide a remote execution context.
- */
-final class RemoteActionContextProvider extends ActionContextProvider {
+/** Provide a remote execution context. */
+final class RemoteActionContextProvider extends ActionContextProvider
+ implements ExecutorLifecycleListener {
private final CommandEnvironment env;
private final RemoteCache cache;
@Nullable private final GrpcRemoteExecutor executor;
@@ -128,6 +132,14 @@
}
@Override
+ public void executorCreated() throws ExecutorInitException {}
+
+ @Override
+ public void executionPhaseStarting(
+ ActionGraph actionGraph, Supplier<ArtifactsToOwnerLabels> topLevelArtifactsToOwnerLabels)
+ throws ExecutorInitException, InterruptedException {}
+
+ @Override
public void executionPhaseEnding() {
cache.close();
if (executor != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
index 96dc3d9..af05d87 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -493,6 +493,7 @@
return;
}
builder.addActionContextProvider(actionContextProvider);
+ builder.addExecutorLifecycleListener(actionContextProvider);
RemoteOptions remoteOptions =
Preconditions.checkNotNull(
env.getOptions().getOptions(RemoteOptions.class), "RemoteOptions");
diff --git a/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java
index ff83d3c..7a69144 100644
--- a/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java
@@ -347,8 +347,7 @@
OptionsParser.builder()
.optionsClasses(ImmutableList.of(ExecutionOptions.class))
.build(),
- spawnActionContextMaps,
- executorBuilder.getActionContextProviders());
+ spawnActionContextMaps);
ActionExecutionContext actionExecutionContext =
ActionsTestUtil.createContext(
diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/TestExecutorBuilder.java b/src/test/java/com/google/devtools/build/lib/exec/util/TestExecutorBuilder.java
index c3be34c..d16848f 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/util/TestExecutorBuilder.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/util/TestExecutorBuilder.java
@@ -107,7 +107,6 @@
reporter,
BlazeClock.instance(),
optionsParser,
- spawnActionContextMaps,
- ImmutableList.of());
+ spawnActionContextMaps);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
index 5dc0237..af2e899 100644
--- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
@@ -170,8 +170,7 @@
}
},
BinTools.forIntegrationTesting(directories, ImmutableList.of()),
- Mockito.mock(RunfilesTreeUpdater.class)))))),
- ImmutableList.of());
+ Mockito.mock(RunfilesTreeUpdater.class)))))));
executor.getExecRoot().createDirectoryAndParents();
}
