Experimental: spawn actions can execute asynchronously
The new code path is only enabled if experimental_async_execution is set
to true. It is also not particularly useful at this time, as it only
covers a subset of spawn actions. However, I think having this checked
in (even without tests) makes it clearer how the subsequent changes need
to work.
My plan for test coverage is to incrementally refactor the code such
that we only have a single code path rather than two, for which the
existing integration tests should provide plenty of coverage.
There are two parallel threads for refactoring:
1. Extend the concept of action continuations to the Action interface
(thereby generalizing the rudimentary interface added here). Unfortunately,
doing that naively will add a third level of continuations (there's one level
in ActionExecutionFunction, and one in SkyframeActionExecutor), which I'm
reluctant to do. I think the ideal solution is to unify the two existing
continuation levels into one and then find a way to extend it to Action.
I am concerned about the current complexity of exception handling in this area,
which probably needs to be simplified in order to be able to straighten the
continuations story.
On the plus side, moving more code into continuations should reduce the
amount of duplicate work we're doing on function restarts, although that
may not be a significant performance win.
2. Start integrating between the new ListenableFuture support in Skyframe and
ActionExecutionFunction. That means we'll start relying on the newly-added
Skyframe support for _all_ builds, not just when the flag is set. All
existing test coverage for shared actions implicitly applies here.
The first step is to do this for shared actions, which seems fine since it
neither affects profiling nor maximum execution load. While builds with a lot
of shared actions may start to run more actions in parallel, they're still
limited by --jobs.
Progress on #6394.
PiperOrigin-RevId: 234572533
diff --git a/src/main/java/com/google/devtools/build/lib/actions/SpawnActionContext.java b/src/main/java/com/google/devtools/build/lib/actions/SpawnActionContext.java
index ce8da0c..421d8e4 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/SpawnActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/SpawnActionContext.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;
+import com.google.common.collect.Iterables;
import java.util.List;
/**
@@ -24,4 +25,14 @@
/** Executes the given spawn and returns metadata about the execution. */
List<SpawnResult> exec(Spawn spawn, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException;
+
+ /**
+ * Executes the given spawn, possibly asynchronously, and returns a FutureSpawn to represent the
+ * execution, which can be listened to / registered with Skyframe.
+ */
+ default FutureSpawn execMaybeAsync(Spawn spawn, ActionExecutionContext actionExecutionContext)
+ throws ExecException, InterruptedException {
+ SpawnResult result = Iterables.getOnlyElement(exec(spawn, actionExecutionContext));
+ return FutureSpawn.immediate(result);
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index b2fd6cf..f100886 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
@@ -47,6 +47,7 @@
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionInfoSpecifier;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
+import com.google.devtools.build.lib.actions.FutureSpawn;
import com.google.devtools.build.lib.actions.ParamFileInfo;
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
@@ -283,6 +284,54 @@
.exec(spawn, actionExecutionContext);
}
+ public boolean mayExecuteAsync() {
+ return true;
+ }
+
+ public FutureSpawn execMaybeAsync(ActionExecutionContext actionExecutionContext)
+ throws ActionExecutionException, ExecException, InterruptedException {
+ Spawn spawn;
+ try {
+ spawn = getSpawn(actionExecutionContext);
+ } catch (CommandLineExpansionException e) {
+ throw new ActionExecutionException(e, this, /*catastrophe=*/ false);
+ }
+ SpawnActionContext context = actionExecutionContext.getContext(SpawnActionContext.class);
+ return context.execMaybeAsync(spawn, actionExecutionContext);
+ }
+
+ public ActionResult finishSync(FutureSpawn futureSpawn, boolean verboseFailures)
+ throws ActionExecutionException, InterruptedException {
+ try {
+ return ActionResult.create(ImmutableList.of(futureSpawn.get()));
+ } catch (ExecException e) {
+ String failMessage;
+ if (isShellCommand()) {
+ // The possible reasons it could fail are: shell executable not found, shell
+ // exited non-zero, or shell died from signal. The first is impossible
+ // and the second two aren't very interesting, so in the interests of
+ // keeping the noise-level down, we don't print a reason why, just the
+ // command that failed.
+ //
+ // 0=shell executable, 1=shell command switch, 2=command
+ try {
+ failMessage =
+ "error executing shell command: "
+ + "'"
+ + truncate(Joiner.on(" ").join(getArguments()), 200)
+ + "'";
+ } catch (CommandLineExpansionException commandLineExpansionException) {
+ failMessage =
+ "error executing shell command, and error expanding command line: "
+ + commandLineExpansionException;
+ }
+ } else {
+ failMessage = getRawProgressMessage();
+ }
+ throw e.toActionExecutionException(failMessage, verboseFailures, this);
+ }
+ }
+
@Override
public ActionResult execute(ActionExecutionContext actionExecutionContext)
throws ActionExecutionException, InterruptedException {
@@ -315,7 +364,7 @@
throw e.toActionExecutionException(
failMessage, actionExecutionContext.getVerboseFailures(), this);
} catch (CommandLineExpansionException e) {
- throw new ActionExecutionException(e, this, false);
+ throw new ActionExecutionException(e, this, /*catastrophe=*/ false);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java b/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java
index c3f573d..7ea7bbb 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java
@@ -104,6 +104,11 @@
}
@Override
+ public boolean mayExecuteAsync() {
+ return false;
+ }
+
+ @Override
public boolean discoversInputs() {
return shadowedAction.discoversInputs();
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java
index 78eb45b..79845b2 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java
@@ -316,6 +316,20 @@
help = "This option is deprecated and has no effect.")
public boolean discardActionsAfterExecution;
+ @Option(
+ name = "experimental_async_execution",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+ metadataTags = OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION},
+ help =
+ "If set to true, Bazel is allowed to run aynchronously, i.e., without reserving a local "
+ + "thread. This only has an effect if the action implementation and the lower-level "
+ + "strategy support it. This setting effectively circumvents the implicit limit of "
+ + "number of concurrently running actions otherwise imposed by the --jobs flag. Use "
+ + "with caution.")
+ public boolean useAsyncExecution;
+
/**
* Converter for jobs: Takes keyword ({@value #FLAG_SYNTAX}). Values must be between 1 and
* MAX_JOBS.
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ProxySpawnActionContext.java b/src/main/java/com/google/devtools/build/lib/exec/ProxySpawnActionContext.java
index 4b1ce23..143e605 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ProxySpawnActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ProxySpawnActionContext.java
@@ -16,6 +16,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.FutureSpawn;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnActionContext;
import com.google.devtools.build.lib.actions.SpawnResult;
@@ -44,6 +45,13 @@
.exec(spawn, actionExecutionContext);
}
+ @Override
+ public FutureSpawn execMaybeAsync(Spawn spawn, ActionExecutionContext actionExecutionContext)
+ throws ExecException, InterruptedException {
+ return resolve(spawn, actionExecutionContext.getEventHandler())
+ .execMaybeAsync(spawn, actionExecutionContext);
+ }
+
/**
* Returns the {@link SpawnActionContext} that should be used to execute the given spawn.
*
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java
index 8a592d1..e3f1d8e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java
@@ -104,6 +104,11 @@
}
@Override
+ public boolean mayExecuteAsync() {
+ return false;
+ }
+
+ @Override
public boolean discoversInputs() {
return imports != null;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index eb92dd6..cadc6dc 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -552,8 +552,13 @@
long actionStartTime)
throws ActionExecutionException, InterruptedException {
if (previousAction != null) {
- // If this is a shared action and the other action is the one that executed, we must use that
- // other action's value, provided here, since it is populated with metadata for the outputs.
+ // There are two cases where we can already have an executing action for a specific output:
+ // 1. Another instance of a shared action won the race and got executed first.
+ // 2. The action was already started earlier, and this SkyFunction got restarted since
+ // there's progress to be made.
+ // In either case, we must use this continuation to continue. Note that in the first case,
+ // we don't have any input metadata available, so we couldn't re-execute the action even if we
+ // wanted to.
return previousAction.getResultOrDependOnFuture(env, actionLookupData, action);
}
// The metadataHandler may be recreated if we discover inputs.
@@ -700,6 +705,11 @@
actionStartTime,
actionExecutionContext,
actionLookupData);
+ // If an action is executed asynchronously, we may not have a result. In that case we return
+ // null here and expect the function to be restarted.
+ if (env.valuesMissing()) {
+ return null;
+ }
}
} catch (IOException e) {
throw new ActionExecutionException(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index f3f5a3d..23f3072 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -55,9 +55,11 @@
import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException;
import com.google.devtools.build.lib.actions.CachedActionEvent;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
+import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.Executor;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
+import com.google.devtools.build.lib.actions.FutureSpawn;
import com.google.devtools.build.lib.actions.LostInputsExecException.LostInputsActionExecutionException;
import com.google.devtools.build.lib.actions.MapBasedActionGraph;
import com.google.devtools.build.lib.actions.MetadataConsumer;
@@ -69,6 +71,7 @@
import com.google.devtools.build.lib.actions.PackageRootResolver;
import com.google.devtools.build.lib.actions.TargetOutOfDateException;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
+import com.google.devtools.build.lib.analysis.actions.SpawnAction;
import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ExecutorUtil;
@@ -179,6 +182,7 @@
// findAndStoreArtifactConflicts, and is preserved across builds otherwise.
private ImmutableMap<ActionAnalysisMetadata, ConflictException> badActionMap = ImmutableMap.of();
private OptionsProvider options;
+ private boolean useAsyncExecution;
private boolean hadExecutionError;
private MetadataProvider perBuildFileCache;
private ActionInputPrefetcher actionInputPrefetcher;
@@ -392,7 +396,8 @@
this.actionCacheChecker = Preconditions.checkNotNull(actionCacheChecker);
// Don't cache possibly stale data from the last build.
this.options = options;
- // Cache the finalizeActions value for performance, since we consult it on every action.
+ // Cache some option values for performance, since we consult them on every action.
+ this.useAsyncExecution = options.getOptions(BuildRequestOptions.class).useAsyncExecution;
this.finalizeActions = options.getOptions(BuildRequestOptions.class).finalizeActions;
this.outputService = outputService;
}
@@ -1017,6 +1022,29 @@
notifyActionCompletion(env.getListener(), /*postActionCompletionEvent=*/ true);
return new ExceptionalActionStepOrResult(e);
}
+
+ // This is the first iteration of the async action execution framework. It is currently only
+ // implemented for SpawnAction (and subclasses), and will need to be extended for all other
+ // action types.
+ if (useAsyncExecution
+ && (action instanceof SpawnAction)
+ && ((SpawnAction) action).mayExecuteAsync()) {
+ SpawnAction spawnAction = (SpawnAction) action;
+ try {
+ FutureSpawn futureSpawn;
+ try {
+ futureSpawn = spawnAction.execMaybeAsync(actionExecutionContext);
+ } catch (InterruptedException e) {
+ return new ExceptionalActionStepOrResult(e);
+ } catch (ActionExecutionException e) {
+ return new ExceptionalActionStepOrResult(e);
+ }
+ return new ActionCompletionStep(futureSpawn, spawnAction);
+ } catch (ExecException e) {
+ return new ExceptionalActionStepOrResult(e.toActionExecutionException(spawnAction));
+ }
+ }
+
return completeAction(env.getListener(), () -> executeAction(env.getListener()));
}
}
@@ -1179,6 +1207,33 @@
: null,
ActionExecutionFunction.actionDependsOnBuildId(action));
}
+
+ /** A closure to complete an asynchronously running action. */
+ private class ActionCompletionStep implements ActionStepOrResult {
+ private final FutureSpawn futureSpawn;
+ private final SpawnAction spawnAction;
+
+ public ActionCompletionStep(FutureSpawn futureSpawn, SpawnAction spawnAction) {
+ this.futureSpawn = futureSpawn;
+ this.spawnAction = spawnAction;
+ }
+
+ @Override
+ public boolean isDone() {
+ return false;
+ }
+
+ @Override
+ public ActionStepOrResult run(Environment env) {
+ if (!futureSpawn.getFuture().isDone()) {
+ env.dependOnFuture(futureSpawn.getFuture());
+ return this;
+ }
+ return completeAction(
+ actionExecutionContext.getEventHandler(),
+ () -> spawnAction.finishSync(futureSpawn, actionExecutionContext.getVerboseFailures()));
+ }
+ }
}
private void createOutputDirectories(Action action, ActionExecutionContext context)
