Skip to content

Commit

Permalink
Experimental: spawn actions can execute asynchronously
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ulfjack authored and copybara-github committed Feb 19, 2019
1 parent 2d1186b commit 1de1e64
Show file tree
Hide file tree
Showing 8 changed files with 161 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -24,4 +25,14 @@ public interface SpawnActionContext extends ActionContext {
/** 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -283,6 +284,54 @@ protected List<SpawnResult> internalExecute(ActionExecutionContext actionExecuti
.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 {
Expand Down Expand Up @@ -315,7 +364,7 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext)
throw e.toActionExecutionException(
failMessage, actionExecutionContext.getVerboseFailures(), this);
} catch (CommandLineExpansionException e) {
throw new ActionExecutionException(e, this, false);
throw new ActionExecutionException(e, this, /*catastrophe=*/ false);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ public Action apply(@Nullable ExtraAction extraAction) {
}
}

@Override
public boolean mayExecuteAsync() {
return false;
}

@Override
public boolean discoversInputs() {
return shadowedAction.discoversInputs();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,20 @@ public boolean useTopLevelTargetsForSymlinks() {
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -44,6 +45,13 @@ public List<SpawnResult> exec(Spawn spawn, ActionExecutionContext actionExecutio
.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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ public LtoBackendAction(
imports = importsFile;
}

@Override
public boolean mayExecuteAsync() {
return false;
}

@Override
public boolean discoversInputs() {
return imports != null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -552,8 +552,13 @@ private ActionExecutionValue checkCacheAndExecuteIfNeeded(
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.
Expand Down Expand Up @@ -700,6 +705,11 @@ private ActionExecutionValue checkCacheAndExecuteIfNeeded(
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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -179,6 +182,7 @@ enum ProgressEventBehavior {
// 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;
Expand Down Expand Up @@ -392,7 +396,8 @@ void prepareForExecution(
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;
}
Expand Down Expand Up @@ -1017,6 +1022,29 @@ public ActionStepOrResult run(SkyFunction.Environment env) {
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()));
}
}
Expand Down Expand Up @@ -1179,6 +1207,33 @@ private ActionExecutionValue actuallyCompleteAction(
: 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)
Expand Down

0 comments on commit 1de1e64

Please sign in to comment.