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 a932a7aaca5a5b..70983904485a37 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,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.actions; -import com.google.common.collect.Iterables; import java.util.List; /** @@ -22,18 +21,24 @@ @ActionContextMarker(name = "spawn") public interface SpawnActionContext extends ActionContext { - /** Executes the given spawn and returns metadata about the execution. */ + /** + * Executes the given spawn and returns metadata about the execution. Implementations must + * guarantee that the first list entry represents the successful execution of the given spawn (if + * no execution was successful, the method must throw an exception instead). The list may contain + * further entries for (unsuccessful) retries as well as tree artifact management (which may + * require additional spawn executions). + */ List 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. + * Executes the given spawn, possibly asynchronously, and returns a SpawnContinuation to represent + * the execution. Otherwise all requirements from {@link #exec} apply. */ - default FutureSpawn execMaybeAsync(Spawn spawn, ActionExecutionContext actionExecutionContext) + default SpawnContinuation beginExecution( + Spawn spawn, ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException { - SpawnResult result = Iterables.getOnlyElement(exec(spawn, actionExecutionContext)); - return FutureSpawn.immediate(result); + return SpawnContinuation.immediate(exec(spawn, actionExecutionContext)); } /** Returns whether this SpawnActionContext supports executing the given Spawn. */ diff --git a/src/main/java/com/google/devtools/build/lib/actions/SpawnContinuation.java b/src/main/java/com/google/devtools/build/lib/actions/SpawnContinuation.java new file mode 100644 index 00000000000000..7669565a829df6 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/actions/SpawnContinuation.java @@ -0,0 +1,100 @@ +// Copyright 2019 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.actions; + +import com.google.common.collect.ImmutableList; +import com.google.common.util.concurrent.ListenableFuture; +import java.util.List; + +/** + * A representation of a (potentially) multi-step spawn execution, which can return multiple + * results. This covers cases like remote/local fallback as well as tree artifact packing/unpacking, + * which both require multiple attempts at running a spawn, and therefore can have multiple results. + * + *

This is intentionally similar to {@link ActionContinuationOrResult}, which will often wrap one + * of these. + * + *

Any client of this class must first call {@link #isDone} before calling any of the + * other methods. If {@link #isDone} returns true, then {@link #getFuture} and {@link #execute} must + * throw {@link IllegalStateException}, but {@link #get} must return a valid value. Use {@link + * #immediate} to construct such an instance. + * + *

Otherwise, {@link #getFuture} must return a non-null value, and {@link #execute} must not + * throw {@link IllegalStateException}, whereas {@link #get} must throw {@link + * IllegalStateException}. + */ +public abstract class SpawnContinuation { + public static SpawnContinuation immediate(SpawnResult... spawnResults) { + return new Finished(ImmutableList.copyOf(spawnResults)); + } + + public static SpawnContinuation immediate(List spawnResults) { + return new Finished(ImmutableList.copyOf(spawnResults)); + } + + /** + * Runs the state machine represented by the given continuation to completion, blocking as + * necessary until all asynchronous computations finish, and the final continuation is done. Then + * returns the list of spawn results (calling {@link SpawnContinuation#get}). + * + *

This method provides backwards compatibility for the cases where a method that's defined as + * blocking obtains a continuation and needs the result before it can return. Over time, this + * method should become less common as more actions are rewritten to support async execution. + */ + public static List completeBlocking(SpawnContinuation continuation) + throws ExecException, InterruptedException { + while (!continuation.isDone()) { + continuation = continuation.execute(); + } + return continuation.get(); + } + + public boolean isDone() { + return false; + } + + public abstract ListenableFuture getFuture(); + + public abstract SpawnContinuation execute() throws ExecException, InterruptedException; + + public List get() { + throw new IllegalStateException(); + } + + private static final class Finished extends SpawnContinuation { + private final List spawnResults; + + Finished(List spawnResults) { + this.spawnResults = spawnResults; + } + + public boolean isDone() { + return true; + } + + @Override + public ListenableFuture getFuture() { + throw new IllegalStateException(); + } + + @Override + public SpawnContinuation execute() { + throw new IllegalStateException(); + } + + public List get() { + return spawnResults; + } + } +} 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 3b9cd73bcfa53e..c54459b60790b0 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 @@ -51,13 +51,13 @@ 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; import com.google.devtools.build.lib.actions.SingleStringArgFormatter; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; +import com.google.devtools.build.lib.actions.SpawnContinuation; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.extra.EnvironmentVariable; import com.google.devtools.build.lib.actions.extra.ExtraActionInfo; @@ -296,28 +296,8 @@ public final ActionContinuationOrResult beginExecution( beforeExecute(actionExecutionContext); Spawn spawn = getSpawn(actionExecutionContext); SpawnActionContext context = actionExecutionContext.getContext(SpawnActionContext.class); - FutureSpawn futureSpawn = context.execMaybeAsync(spawn, actionExecutionContext); - return new ActionContinuationOrResult() { - @Override - public ListenableFuture getFuture() { - return futureSpawn.getFuture(); - } - - @Override - public ActionContinuationOrResult execute() - throws ActionExecutionException, InterruptedException { - try { - SpawnResult spawnResult = futureSpawn.get(); - afterExecute(actionExecutionContext); - return ActionContinuationOrResult.of( - ActionResult.create(ImmutableList.of(spawnResult))); - } catch (IOException e) { - throw warnUnexpectedIOException(actionExecutionContext, e); - } catch (ExecException e) { - throw toActionExecutionException(e, actionExecutionContext.getVerboseFailures()); - } - } - }; + SpawnContinuation spawnContinuation = context.beginExecution(spawn, actionExecutionContext); + return new SpawnActionContinuation(spawnContinuation, actionExecutionContext); } catch (IOException e) { throw warnUnexpectedIOException(actionExecutionContext, e); } catch (ExecException e) { @@ -1362,4 +1342,37 @@ private Iterable expandArguments(@Nullable ArtifactExpander artifactExpa return result.build(); } } + + private final class SpawnActionContinuation extends ActionContinuationOrResult { + private final SpawnContinuation spawnContinuation; + private final ActionExecutionContext actionExecutionContext; + + public SpawnActionContinuation( + SpawnContinuation spawnContinuation, ActionExecutionContext actionExecutionContext) { + this.spawnContinuation = spawnContinuation; + this.actionExecutionContext = actionExecutionContext; + } + + @Override + public ListenableFuture getFuture() { + return spawnContinuation.getFuture(); + } + + @Override + public ActionContinuationOrResult execute() + throws ActionExecutionException, InterruptedException { + try { + SpawnContinuation nextContinuation = spawnContinuation.execute(); + if (nextContinuation.isDone()) { + afterExecute(actionExecutionContext); + return ActionContinuationOrResult.of(ActionResult.create(nextContinuation.get())); + } + return new SpawnActionContinuation(nextContinuation, actionExecutionContext); + } catch (IOException e) { + throw warnUnexpectedIOException(actionExecutionContext, e); + } catch (ExecException e) { + throw toActionExecutionException(e, actionExecutionContext.getVerboseFailures()); + } + } + } } 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 f337b12f40e592..38a36b2ce65034 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,9 +16,9 @@ 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.SpawnContinuation; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.events.EventHandler; @@ -48,23 +48,26 @@ public ProxySpawnActionContext( @Override public List exec(Spawn spawn, ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException { - List strategies = resolve(spawn, actionExecutionContext.getEventHandler()); - - // Because the strategies are ordered by preference, we can execute the spawn with the best - // possible one by simply filtering out the ones that can't execute it and then picking the - // first one from the remaining strategies in the list. - return strategies.get(0).exec(spawn, actionExecutionContext); + return resolveOne(spawn, actionExecutionContext.getEventHandler()) + .exec(spawn, actionExecutionContext); } @Override - public FutureSpawn execMaybeAsync(Spawn spawn, ActionExecutionContext actionExecutionContext) + public SpawnContinuation beginExecution( + Spawn spawn, ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException { - List strategies = resolve(spawn, actionExecutionContext.getEventHandler()); + return resolveOne(spawn, actionExecutionContext.getEventHandler()) + .beginExecution(spawn, actionExecutionContext); + } + + private SpawnActionContext resolveOne(Spawn spawn, EventHandler eventHandler) + throws UserExecException { + List strategies = resolve(spawn, eventHandler); // Because the strategies are ordered by preference, we can execute the spawn with the best // possible one by simply filtering out the ones that can't execute it and then picking the // first one from the remaining strategies in the list. - return strategies.get(0).execMaybeAsync(spawn, actionExecutionContext); + return strategies.get(0); } /**