diff options
author | 2018-04-19 05:41:44 -0700 | |
---|---|---|
committer | 2018-04-19 05:42:58 -0700 | |
commit | 29e306d66cc4c441f0321a62101077647a4c95dc (patch) | |
tree | cabcf23c53c9cdcfe501bc5f17531f73e81c5a30 /src/main/java/com/google/devtools/build/lib | |
parent | 9566f677a1093e3a3c0ddaed3f9ab34dd98e5e26 (diff) |
Rename SpawnExecutionPolicy -> SpawnExecutionContext.
This class will be used to tie a Spawn to a SpawnRunner, and isn't really a policy object. It will carry state such as the expanded inputs and expanded command line.
Currently a context can be passed between different SpawnRunners. This will be addressed independently, so a context is tied to a particular spawn runner.
PiperOrigin-RevId: 193501918
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
13 files changed, 131 insertions, 135 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java index 0a0a2a811b..fee7f52e6b 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java @@ -33,7 +33,7 @@ import com.google.devtools.build.lib.actions.SpawnResult.Status; import com.google.devtools.build.lib.actions.Spawns; import com.google.devtools.build.lib.exec.SpawnCache.CacheHandle; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.rules.fileset.FilesetActionContext; import com.google.devtools.build.lib.util.CommandFailureUtils; import com.google.devtools.build.lib.util.io.FileOutErr; @@ -74,9 +74,8 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte actionExecutionContext.reportSubcommand(spawn); } final Duration timeout = Spawns.getTimeout(spawn); - SpawnExecutionPolicy policy = - new SpawnExecutionPolicyImpl( - spawn, actionExecutionContext, writeOutputFiles, timeout); + SpawnExecutionContext context = + new SpawnExecutionContextImpl(spawn, actionExecutionContext, writeOutputFiles, timeout); // TODO(ulfjack): Provide a way to disable the cache. We don't want the RemoteSpawnStrategy to // check the cache twice. Right now that can't happen because this is hidden behind an // experimental flag. @@ -88,12 +87,12 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte } SpawnResult spawnResult; try { - try (CacheHandle cacheHandle = cache.lookup(spawn, policy)) { + try (CacheHandle cacheHandle = cache.lookup(spawn, context)) { if (cacheHandle.hasResult()) { spawnResult = Preconditions.checkNotNull(cacheHandle.getResult()); } else { // Actual execution. - spawnResult = spawnRunner.exec(spawn, policy); + spawnResult = spawnRunner.exec(spawn, context); if (cacheHandle.willStore()) { cacheHandle.store( spawnResult, listExistingOutputFiles(spawn, actionExecutionContext.getExecRoot())); @@ -133,7 +132,7 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte return outputFiles; } - private final class SpawnExecutionPolicyImpl implements SpawnExecutionPolicy { + private final class SpawnExecutionContextImpl implements SpawnExecutionContext { private final Spawn spawn; private final ActionExecutionContext actionExecutionContext; private final AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles; @@ -144,7 +143,7 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte // TODO(ulfjack): Guard against client modification of this map. private SortedMap<PathFragment, ActionInput> lazyInputMapping; - public SpawnExecutionPolicyImpl( + public SpawnExecutionContextImpl( Spawn spawn, ActionExecutionContext actionExecutionContext, AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles, diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java index 1374b47fd8..afc9d9376b 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java @@ -18,7 +18,7 @@ import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.vfs.Path; import java.io.Closeable; import java.io.IOException; @@ -99,7 +99,7 @@ public interface SpawnCache extends ActionContext { ) public static class NoSpawnCache implements SpawnCache { @Override - public CacheHandle lookup(Spawn spawn, SpawnExecutionPolicy context) { + public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) { return SpawnCache.NO_RESULT_NO_STORE; } } @@ -156,12 +156,12 @@ public interface SpawnCache extends ActionContext { * Perform a spawn lookup. This method is similar to {@link SpawnRunner#exec}, taking the same * parameters and being allowed to throw the same exceptions. The intent for this method is to * compute a cache lookup key for the given spawn, looking it up in an implementation-dependent - * cache (can be either on the local or remote machine), and returning a non-null - * {@link CacheHandle} instance. + * cache (can be either on the local or remote machine), and returning a non-null {@link + * CacheHandle} instance. * * <p>If the lookup was successful, this method should write the cached outputs to their * corresponding output locations in the output tree, as well as stdout and stderr, after - * notifying {@link SpawnExecutionPolicy#lockOutputFiles}. + * notifying {@link SpawnExecutionContext#lockOutputFiles}. * * <p>If the lookup was unsuccessful, this method can return a {@link CacheHandle} instance that * has no result, but uploads the results of the execution to the cache. The reason for a callback @@ -173,6 +173,6 @@ public interface SpawnCache extends ActionContext { * <p>Note that cache stores may be disabled, in which case the returned {@link CacheHandle} * instance's {@link CacheHandle#store} is a no-op. */ - CacheHandle lookup(Spawn spawn, SpawnExecutionPolicy context) + CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) throws ExecException, IOException, InterruptedException; } diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java index 36346a556d..5ed43cd14e 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java @@ -30,6 +30,7 @@ import java.util.SortedMap; * or without sandboxing, on a remote machine, or only consult a remote cache. * * <h2>Environment Variables</h2> + * * <ul> * <li>Implementations MUST set the specified environment variables. * <li>Implementations MAY add TMPDIR as an additional env variable, if it is not set already. @@ -38,12 +39,14 @@ import java.util.SortedMap; * </ul> * * <h2>Command line</h2> + * * <ul> * <li>Implementations MUST use the specified command line unmodified by default. * <li>Implementations MAY modify the specified command line if explicitly requested by the user. * </ul> * * <h2>Process</h2> + * * <ul> * <li>Implementations MUST be thread-safe. * <li>Implementations MUST ensure that all child processes (including transitive) exit in all @@ -53,30 +56,32 @@ import java.util.SortedMap; * <li>Implementations MUST be interruptible; they MUST throw {@link InterruptedException} from * {@link #exec} when interrupted * <li>Implementations MUST apply the specified timeout to the execution of the subprocess - * <ul> - * <li>If no timeout is specified, the implementation MAY apply an implementation-specific - * timeout - * <li>If the specified timeout is larger than an implementation-dependent maximum, then the - * implementation MUST throw {@link IllegalArgumentException}; it MUST not silently change - * the timeout to a smaller value - * <li>If the timeout is exceeded, the implementation MUST throw TimeoutException, with the - * timeout that was applied to the subprocess (TODO) - * </ul> + * <ul> + * <li>If no timeout is specified, the implementation MAY apply an implementation-specific + * timeout + * <li>If the specified timeout is larger than an implementation-dependent maximum, then the + * implementation MUST throw {@link IllegalArgumentException}; it MUST not silently + * change the timeout to a smaller value + * <li>If the timeout is exceeded, the implementation MUST throw TimeoutException, with the + * timeout that was applied to the subprocess (TODO) + * </ul> * </ul> * * <h2>Optimistic Concurrency</h2> + * * Bazel may choose to execute a spawn using multiple {@link SpawnRunner} implementations * simultaneously in order to minimize total latency. This is especially useful for builds with few * actions where remotely executing the actions incurs high round trip times. + * * <ul> - * <li>All implementations MUST call {@link SpawnExecutionPolicy#lockOutputFiles} before writing + * <li>All implementations MUST call {@link SpawnExecutionContext#lockOutputFiles} before writing * to any of the output files, but may write to stdout and stderr without calling it. Instead, * all callers must provide temporary locations for stdout & stderr if they ever call multiple * {@link SpawnRunner} implementations concurrently. Spawn runners that use the local machine * MUST either call it before starting the subprocess, or ensure that subprocesses write to * temporary locations (for example by running in a mount namespace) and then copy or move the * outputs into place. - * <li>Implementations SHOULD delay calling {@link SpawnExecutionPolicy#lockOutputFiles} until + * <li>Implementations SHOULD delay calling {@link SpawnExecutionContext#lockOutputFiles} until * just before writing. * </ul> */ @@ -113,15 +118,15 @@ public interface SpawnRunner { } /** - * A helper class to provide additional tools and methods to {@link SpawnRunner} implementations. + * A context that binds a {@link Spawn} to a {@link SpawnRunner}. * * <p>This interface may change without notice. * - * <p>Implementations must be at least thread-compatible, i.e., they must be safe as long as - * each instance is only used within a single thread. Different instances of the same class may - * be used by different threads, so they MUST not call any shared non-thread-safe objects. + * <p>Implementations must be at least thread-compatible, i.e., they must be safe as long as each + * instance is only used within a single thread. Different instances of the same class may be used + * by different threads, so they MUST not call any shared non-thread-safe objects. */ - public interface SpawnExecutionPolicy { + interface SpawnExecutionContext { /** * Returns a unique id for this spawn, to be used for logging. Note that a single spawn may be * passed to multiple {@link SpawnRunner} implementations, so any log entries should also @@ -192,17 +197,15 @@ public interface SpawnRunner { * Run the given spawn. * * @param spawn the spawn to run - * @param policy a helper that provides additional parameters + * @param context the spawn execution context * @return the result from running the spawn * @throws InterruptedException if the calling thread was interrupted, or if the runner could not - * lock the output files (see {@link SpawnExecutionPolicy#lockOutputFiles()}) + * lock the output files (see {@link SpawnExecutionContext#lockOutputFiles()}) * @throws IOException if something went wrong reading or writing to the local file system * @throws ExecException if the request is malformed */ - SpawnResult exec( - Spawn spawn, - SpawnExecutionPolicy policy) - throws InterruptedException, IOException, ExecException; + SpawnResult exec(Spawn spawn, SpawnExecutionContext context) + throws InterruptedException, IOException, ExecException; /* Name of the SpawnRunner. */ String getName(); diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java index c9575d0679..e22da3ef85 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java @@ -118,16 +118,15 @@ public class LocalSpawnRunner implements SpawnRunner { } @Override - public SpawnResult exec( - Spawn spawn, - SpawnExecutionPolicy policy) throws IOException, InterruptedException { + public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) + throws IOException, InterruptedException { ActionExecutionMetadata owner = spawn.getResourceOwner(); - policy.report(ProgressStatus.SCHEDULING, getName()); + context.report(ProgressStatus.SCHEDULING, getName()); try (ResourceHandle handle = resourceManager.acquireResources(owner, spawn.getLocalResources())) { - policy.report(ProgressStatus.EXECUTING, getName()); - policy.lockOutputFiles(); - return new SubprocessHandler(spawn, policy).run(); + context.report(ProgressStatus.EXECUTING, getName()); + context.lockOutputFiles(); + return new SubprocessHandler(spawn, context).run(); } } @@ -141,7 +140,7 @@ public class LocalSpawnRunner implements SpawnRunner { private final class SubprocessHandler { private final Spawn spawn; - private final SpawnExecutionPolicy policy; + private final SpawnExecutionContext context; private final long creationTime = System.currentTimeMillis(); private long stateStartTime = creationTime; @@ -150,13 +149,11 @@ public class LocalSpawnRunner implements SpawnRunner { private final int id; - public SubprocessHandler( - Spawn spawn, - SpawnExecutionPolicy policy) { + public SubprocessHandler(Spawn spawn, SpawnExecutionContext context) { Preconditions.checkArgument(!spawn.getArguments().isEmpty()); this.spawn = spawn; - this.policy = policy; - this.id = policy.getId(); + this.context = context; + this.id = context.getId(); setState(State.PARSING); } @@ -222,7 +219,7 @@ public class LocalSpawnRunner implements SpawnRunner { private SpawnResult start() throws InterruptedException, IOException { logger.info(String.format("starting local subprocess #%d, argv: %s", id, debugCmdString())); - FileOutErr outErr = policy.getFileOutErr(); + FileOutErr outErr = context.getFileOutErr(); String actionType = spawn.getResourceOwner().getMnemonic(); if (localExecutionOptions.allowedLocalAction != null && !localExecutionOptions.allowedLocalAction.matcher(actionType).matches()) { @@ -241,7 +238,7 @@ public class LocalSpawnRunner implements SpawnRunner { if (Spawns.shouldPrefetchInputsForLocalExecution(spawn)) { stepLog(INFO, "prefetching inputs for local execution"); setState(State.PREFETCHING_LOCAL_INPUTS); - policy.prefetchInputs(); + context.prefetchInputs(); } stepLog(INFO, "running locally"); @@ -270,7 +267,7 @@ public class LocalSpawnRunner implements SpawnRunner { ProcessWrapperUtil.commandLineBuilder(processWrapper, spawn.getArguments()) .setStdoutPath(getPathOrDevNull(outErr.getOutputPath())) .setStderrPath(getPathOrDevNull(outErr.getErrorPath())) - .setTimeout(policy.getTimeout()) + .setTimeout(context.getTimeout()) .setKillDelay(Duration.ofSeconds(localExecutionOptions.localSigkillGraceSeconds)); if (localExecutionOptions.collectLocalExecutionStatistics) { statisticsPath = tmpDir.getRelative("stats.out"); @@ -290,7 +287,7 @@ public class LocalSpawnRunner implements SpawnRunner { spawn.getArguments().toArray(new String[0]), environment, execRoot.getPathFile(), - policy.getTimeout()); + context.getTimeout()); } long startTime = System.currentTimeMillis(); @@ -326,7 +323,7 @@ public class LocalSpawnRunner implements SpawnRunner { Duration wallTime = Duration.ofMillis(System.currentTimeMillis() - startTime); boolean wasTimeout = commandResult.getTerminationStatus().timedOut() - || (useProcessWrapper && wasTimeout(policy.getTimeout(), wallTime)); + || (useProcessWrapper && wasTimeout(context.getTimeout(), wallTime)); int exitCode = wasTimeout ? POSIX_TIMEOUT_EXIT_CODE diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 0cb40b167b..72766ced3e 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -26,7 +26,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.exec.SpawnCache; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.DigestUtil.ActionKey; @@ -89,12 +89,12 @@ final class RemoteSpawnCache implements SpawnCache { } @Override - public CacheHandle lookup(Spawn spawn, SpawnExecutionPolicy policy) + public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) throws InterruptedException, IOException, ExecException { // Temporary hack: the TreeNodeRepository should be created and maintained upstream! TreeNodeRepository repository = - new TreeNodeRepository(execRoot, policy.getActionInputFileCache(), digestUtil); - SortedMap<PathFragment, ActionInput> inputMap = policy.getInputMapping(); + new TreeNodeRepository(execRoot, context.getActionInputFileCache(), digestUtil); + SortedMap<PathFragment, ActionInput> inputMap = context.getInputMapping(); TreeNode inputRoot = repository.buildFromActionInputs(inputMap); repository.computeMerkleDigests(inputRoot); Command command = RemoteSpawnRunner.buildCommand(spawn.getArguments(), spawn.getEnvironment()); @@ -104,7 +104,7 @@ final class RemoteSpawnCache implements SpawnCache { digestUtil.compute(command), repository.getMerkleDigest(inputRoot), spawn.getExecutionPlatform(), - policy.getTimeout(), + context.getTimeout(), Spawns.mayBeCached(spawn)); // Look up action cache, and reuse the action output if it is found. @@ -123,7 +123,7 @@ final class RemoteSpawnCache implements SpawnCache { // We don't cache failed actions, so we know the outputs exist. // For now, download all outputs locally; in the future, we can reuse the digests to // just update the TreeNodeRepository and continue the build. - remoteCache.download(result, execRoot, policy.getFileOutErr()); + remoteCache.download(result, execRoot, context.getFileOutErr()); SpawnResult spawnResult = new SpawnResult.Builder() .setStatus(Status.SUCCESS) @@ -177,7 +177,7 @@ final class RemoteSpawnCache implements SpawnCache { && result.exitCode() == 0; Context previous = withMetadata.attach(); try { - remoteCache.upload(actionKey, execRoot, files, policy.getFileOutErr(), uploadAction); + remoteCache.upload(actionKey, execRoot, files, context.getFileOutErr(), uploadAction); } catch (IOException e) { if (verboseFailures) { report(Event.debug("Upload to remote cache failed: " + e.getMessage())); @@ -199,7 +199,7 @@ final class RemoteSpawnCache implements SpawnCache { if (input instanceof VirtualActionInput) { continue; } - Metadata metadata = policy.getActionInputFileCache().getMetadata(input); + Metadata metadata = context.getActionInputFileCache().getMetadata(input); if (metadata instanceof FileArtifactValue) { FileArtifactValue artifactValue = (FileArtifactValue) metadata; Path path = execRoot.getRelative(input.getExecPath()); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index cbbae37dfa..df20b4220d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -120,17 +120,17 @@ class RemoteSpawnRunner implements SpawnRunner { } @Override - public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy) + public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) throws ExecException, InterruptedException, IOException { if (!Spawns.mayBeExecutedRemotely(spawn) || remoteCache == null) { - return fallbackRunner.exec(spawn, policy); + return fallbackRunner.exec(spawn, context); } - policy.report(ProgressStatus.EXECUTING, getName()); + context.report(ProgressStatus.EXECUTING, getName()); // Temporary hack: the TreeNodeRepository should be created and maintained upstream! - ActionInputFileCache inputFileCache = policy.getActionInputFileCache(); + ActionInputFileCache inputFileCache = context.getActionInputFileCache(); TreeNodeRepository repository = new TreeNodeRepository(execRoot, inputFileCache, digestUtil); - SortedMap<PathFragment, ActionInput> inputMap = policy.getInputMapping(); + SortedMap<PathFragment, ActionInput> inputMap = context.getInputMapping(); TreeNode inputRoot = repository.buildFromActionInputs(inputMap); repository.computeMerkleDigests(inputRoot); Command command = buildCommand(spawn.getArguments(), spawn.getEnvironment()); @@ -140,7 +140,7 @@ class RemoteSpawnRunner implements SpawnRunner { digestUtil.compute(command), repository.getMerkleDigest(inputRoot), spawn.getExecutionPlatform(), - policy.getTimeout(), + context.getTimeout(), Spawns.mayBeCached(spawn)); // Look up action cache, and reuse the action output if it is found. @@ -165,7 +165,7 @@ class RemoteSpawnRunner implements SpawnRunner { + actionKey.getDigest()); } try { - return downloadRemoteResults(cachedResult, policy.getFileOutErr()) + return downloadRemoteResults(cachedResult, context.getFileOutErr()) .setCacheHit(true) .setRunnerName("remote cache hit") .build(); @@ -176,19 +176,19 @@ class RemoteSpawnRunner implements SpawnRunner { } } } catch (IOException e) { - return execLocallyOrFail(spawn, policy, inputMap, actionKey, uploadLocalResults, e); + return execLocallyOrFail(spawn, context, inputMap, actionKey, uploadLocalResults, e); } if (remoteExecutor == null) { // Remote execution is disabled and so execute the spawn on the local machine. - return execLocally(spawn, policy, inputMap, uploadLocalResults, remoteCache, actionKey); + return execLocally(spawn, context, inputMap, uploadLocalResults, remoteCache, actionKey); } try { // Upload the command and all the inputs into the remote cache. remoteCache.ensureInputsPresent(repository, execRoot, inputRoot, command); } catch (IOException e) { - return execLocallyOrFail(spawn, policy, inputMap, actionKey, uploadLocalResults, e); + return execLocallyOrFail(spawn, context, inputMap, actionKey, uploadLocalResults, e); } final ActionResult result; @@ -204,16 +204,16 @@ class RemoteSpawnRunner implements SpawnRunner { result = reply.getResult(); remoteCacheHit = reply.getCachedResult(); } catch (IOException e) { - return execLocallyOrFail(spawn, policy, inputMap, actionKey, uploadLocalResults, e); + return execLocallyOrFail(spawn, context, inputMap, actionKey, uploadLocalResults, e); } try { - return downloadRemoteResults(result, policy.getFileOutErr()) + return downloadRemoteResults(result, context.getFileOutErr()) .setRunnerName(remoteCacheHit ? "remote cache hit" : getName()) .setCacheHit(remoteCacheHit) .build(); } catch (IOException e) { - return execLocallyOrFail(spawn, policy, inputMap, actionKey, uploadLocalResults, e); + return execLocallyOrFail(spawn, context, inputMap, actionKey, uploadLocalResults, e); } } finally { withMetadata.detach(previous); @@ -257,7 +257,7 @@ class RemoteSpawnRunner implements SpawnRunner { private SpawnResult execLocallyOrFail( Spawn spawn, - SpawnExecutionPolicy policy, + SpawnExecutionContext context, SortedMap<PathFragment, ActionInput> inputMap, ActionKey actionKey, boolean uploadLocalResults, @@ -271,9 +271,9 @@ class RemoteSpawnRunner implements SpawnRunner { if (options.remoteLocalFallback && !(cause instanceof RetryException && RemoteRetrierUtils.causedByExecTimeout((RetryException) cause))) { - return execLocally(spawn, policy, inputMap, uploadLocalResults, remoteCache, actionKey); + return execLocally(spawn, context, inputMap, uploadLocalResults, remoteCache, actionKey); } - return handleError(cause, policy.getFileOutErr(), actionKey); + return handleError(cause, context.getFileOutErr(), actionKey); } private SpawnResult handleError(IOException exception, FileOutErr outErr, ActionKey actionKey) @@ -412,28 +412,28 @@ class RemoteSpawnRunner implements SpawnRunner { */ private SpawnResult execLocally( Spawn spawn, - SpawnExecutionPolicy policy, + SpawnExecutionContext context, SortedMap<PathFragment, ActionInput> inputMap, boolean uploadToCache, @Nullable AbstractRemoteActionCache remoteCache, @Nullable ActionKey actionKey) throws ExecException, IOException, InterruptedException { if (uploadToCache && remoteCache != null && actionKey != null) { - return execLocallyAndUpload(spawn, policy, inputMap, remoteCache, actionKey); + return execLocallyAndUpload(spawn, context, inputMap, remoteCache, actionKey); } - return fallbackRunner.exec(spawn, policy); + return fallbackRunner.exec(spawn, context); } @VisibleForTesting SpawnResult execLocallyAndUpload( Spawn spawn, - SpawnExecutionPolicy policy, + SpawnExecutionContext context, SortedMap<PathFragment, ActionInput> inputMap, AbstractRemoteActionCache remoteCache, ActionKey actionKey) throws ExecException, IOException, InterruptedException { Map<Path, Long> ctimesBefore = getInputCtimes(inputMap); - SpawnResult result = fallbackRunner.exec(spawn, policy); + SpawnResult result = fallbackRunner.exec(spawn, context); Map<Path, Long> ctimesAfter = getInputCtimes(inputMap); for (Map.Entry<Path, Long> e : ctimesBefore.entrySet()) { // Skip uploading to remote cache, because an input was modified during execution. @@ -447,7 +447,7 @@ class RemoteSpawnRunner implements SpawnRunner { Spawns.mayBeCached(spawn) && Status.SUCCESS.equals(result.status()) && result.exitCode() == 0; - remoteCache.upload(actionKey, execRoot, outputFiles, policy.getFileOutErr(), uploadAction); + remoteCache.upload(actionKey, execRoot, outputFiles, context.getFileOutErr(), uploadAction); } catch (IOException e) { if (verboseFailures) { report(Event.debug("Upload to remote cache failed: " + e.getMessage())); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java index 9fee6544dd..5610242eaa 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java @@ -62,14 +62,14 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { } @Override - public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy) + public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) throws ExecException, InterruptedException { ActionExecutionMetadata owner = spawn.getResourceOwner(); - policy.report(ProgressStatus.SCHEDULING, getName()); + context.report(ProgressStatus.SCHEDULING, getName()); try (ResourceHandle ignored = ResourceManager.instance().acquireResources(owner, spawn.getLocalResources())) { - policy.report(ProgressStatus.EXECUTING, getName()); - return actuallyExec(spawn, policy); + context.report(ProgressStatus.EXECUTING, getName()); + return actuallyExec(spawn, context); } catch (IOException e) { throw new UserExecException("I/O exception during sandboxed execution", e); } @@ -78,25 +78,25 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { // TODO(laszlocsomor): refactor this class to make `actuallyExec`'s contract clearer: the caller // of `actuallyExec` should not depend on `actuallyExec` calling `runSpawn` because it's easy to // forget to do so in `actuallyExec`'s implementations. - protected abstract SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) + protected abstract SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) throws ExecException, InterruptedException, IOException; protected SpawnResult runSpawn( Spawn originalSpawn, SandboxedSpawn sandbox, - SpawnExecutionPolicy policy, + SpawnExecutionContext context, Path execRoot, Duration timeout, Path statisticsPath) throws IOException, InterruptedException { try { sandbox.createFileSystem(); - OutErr outErr = policy.getFileOutErr(); - policy.prefetchInputs(); + OutErr outErr = context.getFileOutErr(); + context.prefetchInputs(); SpawnResult result = run(originalSpawn, sandbox, outErr, timeout, execRoot, statisticsPath); - policy.lockOutputFiles(); + context.lockOutputFiles(); try { // We copy the outputs even when the command failed. sandbox.copyOutputs(execRoot); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java index d20eec70b1..86631443bf 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java @@ -193,10 +193,10 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { } @Override - protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) + protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) throws IOException, InterruptedException { // Each invocation of "exec" gets its own sandbox. - Path sandboxPath = sandboxBase.getRelative(Integer.toString(policy.getId())); + Path sandboxPath = sandboxBase.getRelative(Integer.toString(context.getId())); sandboxPath.createDirectory(); // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like @@ -215,7 +215,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn); final Path sandboxConfigPath = sandboxPath.getRelative("sandbox.sb"); - Duration timeout = policy.getTimeout(); + Duration timeout = context.getTimeout(); ProcessWrapperUtil.CommandLineBuilder processWrapperCommandLineBuilder = ProcessWrapperUtil.commandLineBuilder(processWrapper.getPathString(), spawn.getArguments()) @@ -241,7 +241,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { boolean allowNetworkForThisSpawn = allowNetwork || Spawns.requiresNetwork(spawn); - Map<PathFragment, Path> inputs = SandboxHelpers.getInputFiles(spawn, policy, execRoot); + Map<PathFragment, Path> inputs = SandboxHelpers.getInputFiles(spawn, context, execRoot); SandboxedSpawn sandbox; if (sandboxfsProcess != null) { @@ -287,7 +287,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { } }; } - return runSpawn(spawn, sandbox, policy, execRoot, timeout, statisticsPath); + return runSpawn(spawn, sandbox, context, execRoot, timeout, statisticsPath); } private void writeConfig( diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java index f2aedf8615..888f9abab2 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java @@ -117,10 +117,10 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { } @Override - protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) + protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) throws IOException, ExecException, InterruptedException { // Each invocation of "exec" gets its own sandbox base, execroot and temporary directory. - Path sandboxPath = sandboxBase.getRelative(Integer.toString(policy.getId())); + Path sandboxPath = sandboxBase.getRelative(Integer.toString(context.getId())); sandboxPath.createDirectory(); // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like @@ -134,7 +134,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { ImmutableSet<Path> writableDirs = getWritableDirs(sandboxExecRoot, environment); ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn); - Duration timeout = policy.getTimeout(); + Duration timeout = context.getTimeout(); LinuxSandboxUtil.CommandLineBuilder commandLineBuilder = LinuxSandboxUtil.commandLineBuilder(linuxSandbox, spawn.getArguments()) @@ -170,7 +170,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { sandboxPath, commandLineBuilder.build(), environment, - SandboxHelpers.getInputFiles(spawn, policy, execRoot), + SandboxHelpers.getInputFiles(spawn, context, execRoot), outputs, ImmutableSet.of()); } else { @@ -180,12 +180,12 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { sandboxExecRoot, commandLineBuilder.build(), environment, - SandboxHelpers.getInputFiles(spawn, policy, execRoot), + SandboxHelpers.getInputFiles(spawn, context, execRoot), outputs, writableDirs); } - return runSpawn(spawn, sandbox, policy, execRoot, timeout, statisticsPath); + return runSpawn(spawn, sandbox, context, execRoot, timeout, statisticsPath); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java index e79b2abcd7..b3865a73a5 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java @@ -63,10 +63,10 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne } @Override - protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) + protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) throws ExecException, IOException, InterruptedException { // Each invocation of "exec" gets its own sandbox. - Path sandboxPath = sandboxBase.getRelative(Integer.toString(policy.getId())); + Path sandboxPath = sandboxBase.getRelative(Integer.toString(context.getId())); sandboxPath.createDirectory(); // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like @@ -78,7 +78,7 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne Map<String, String> environment = localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, "/tmp"); - Duration timeout = policy.getTimeout(); + Duration timeout = context.getTimeout(); ProcessWrapperUtil.CommandLineBuilder commandLineBuilder = ProcessWrapperUtil.commandLineBuilder(processWrapper.getPathString(), spawn.getArguments()) .setTimeout(timeout); @@ -97,11 +97,11 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne sandboxExecRoot, commandLineBuilder.build(), environment, - SandboxHelpers.getInputFiles(spawn, policy, execRoot), + SandboxHelpers.getInputFiles(spawn, context, execRoot), SandboxHelpers.getOutputFiles(spawn), getWritableDirs(sandboxExecRoot, environment)); - return runSpawn(spawn, sandbox, policy, execRoot, timeout, statisticsPath); + return runSpawn(spawn, sandbox, context, execRoot, timeout, statisticsPath); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java index cee91cebe1..0acbe064b4 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java @@ -127,12 +127,12 @@ final class SandboxActionContextProvider extends ActionContextProvider { } @Override - public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy) + public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) throws InterruptedException, IOException, ExecException { if (!Spawns.mayBeSandboxed(spawn)) { - return fallbackSpawnRunner.exec(spawn, policy); + return fallbackSpawnRunner.exec(spawn, context); } else { - return sandboxSpawnRunner.exec(spawn, policy); + return sandboxSpawnRunner.exec(spawn, context); } } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java index 31f2dcdfd1..5894680a1e 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java @@ -27,7 +27,7 @@ import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.cache.VirtualActionInput.EmptyActionInput; import com.google.devtools.build.lib.analysis.test.TestConfiguration; import com.google.devtools.build.lib.exec.SpawnInputExpander; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.rules.fileset.FilesetActionContext; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -65,11 +65,8 @@ public final class SandboxHelpers { * host filesystem where the input files can be found. */ public static Map<PathFragment, Path> getInputFiles( - Spawn spawn, - SpawnExecutionPolicy policy, - Path execRoot) - throws IOException { - return postProcess(policy.getInputMapping(), spawn, policy.getArtifactExpander(), execRoot); + Spawn spawn, SpawnExecutionContext context, Path execRoot) throws IOException { + return postProcess(context.getInputMapping(), spawn, context.getArtifactExpander(), execRoot); } /** diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java index a615cf76cd..7efe410b19 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java @@ -96,7 +96,7 @@ final class WorkerSpawnRunner implements SpawnRunner { } @Override - public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy) + public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) throws ExecException, IOException, InterruptedException { if (!spawn.getExecutionInfo().containsKey(ExecutionRequirements.SUPPORTS_WORKERS) || !spawn.getExecutionInfo().get(ExecutionRequirements.SUPPORTS_WORKERS).equals("1")) { @@ -105,19 +105,19 @@ final class WorkerSpawnRunner implements SpawnRunner { reporter.handle( Event.warn( String.format(ERROR_MESSAGE_PREFIX + REASON_NO_EXECUTION_INFO, spawn.getMnemonic()))); - return fallbackRunner.exec(spawn, policy); + return fallbackRunner.exec(spawn, context); } - policy.report(ProgressStatus.SCHEDULING, getName()); + context.report(ProgressStatus.SCHEDULING, getName()); ActionExecutionMetadata owner = spawn.getResourceOwner(); try (ResourceHandle handle = ResourceManager.instance().acquireResources(owner, spawn.getLocalResources())) { - policy.report(ProgressStatus.EXECUTING, getName()); - return actuallyExec(spawn, policy); + context.report(ProgressStatus.EXECUTING, getName()); + return actuallyExec(spawn, context); } } - private SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) + private SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) throws ExecException, IOException, InterruptedException { if (Iterables.isEmpty(spawn.getToolFiles())) { throw new UserExecException( @@ -132,15 +132,15 @@ final class WorkerSpawnRunner implements SpawnRunner { ImmutableList<String> workerArgs = splitSpawnArgsIntoWorkerArgsAndFlagFiles(spawn, flagFiles); ImmutableMap<String, String> env = spawn.getEnvironment(); - ActionInputFileCache inputFileCache = policy.getActionInputFileCache(); + ActionInputFileCache inputFileCache = context.getActionInputFileCache(); SortedMap<PathFragment, HashCode> workerFiles = WorkerFilesHash.getWorkerFilesWithHashes( - spawn, policy.getArtifactExpander(), policy.getActionInputFileCache()); + spawn, context.getArtifactExpander(), context.getActionInputFileCache()); HashCode workerFilesCombinedHash = WorkerFilesHash.getCombinedHash(workerFiles); - Map<PathFragment, Path> inputFiles = SandboxHelpers.getInputFiles(spawn, policy, execRoot); + Map<PathFragment, Path> inputFiles = SandboxHelpers.getInputFiles(spawn, context, execRoot); Set<PathFragment> outputFiles = SandboxHelpers.getOutputFiles(spawn); WorkerKey key = @@ -151,15 +151,15 @@ final class WorkerSpawnRunner implements SpawnRunner { spawn.getMnemonic(), workerFilesCombinedHash, workerFiles, - policy.speculating()); + context.speculating()); - WorkRequest workRequest = createWorkRequest(spawn, policy, flagFiles, inputFileCache); + WorkRequest workRequest = createWorkRequest(spawn, context, flagFiles, inputFileCache); long startTime = System.currentTimeMillis(); - WorkResponse response = execInWorker(key, workRequest, policy, inputFiles, outputFiles); + WorkResponse response = execInWorker(key, workRequest, context, inputFiles, outputFiles); Duration wallTime = Duration.ofMillis(System.currentTimeMillis() - startTime); - FileOutErr outErr = policy.getFileOutErr(); + FileOutErr outErr = context.getFileOutErr(); response.getOutputBytes().writeTo(outErr.getErrorStream()); int exitCode = response.getExitCode(); @@ -202,7 +202,7 @@ final class WorkerSpawnRunner implements SpawnRunner { private WorkRequest createWorkRequest( Spawn spawn, - SpawnExecutionPolicy policy, + SpawnExecutionContext context, List<String> flagfiles, ActionInputFileCache inputFileCache) throws IOException { @@ -212,7 +212,7 @@ final class WorkerSpawnRunner implements SpawnRunner { } List<ActionInput> inputs = - ActionInputHelper.expandArtifacts(spawn.getInputFiles(), policy.getArtifactExpander()); + ActionInputHelper.expandArtifacts(spawn.getInputFiles(), context.getArtifactExpander()); for (ActionInput input : inputs) { byte[] digestBytes = inputFileCache.getMetadata(input).getDigest(); @@ -258,7 +258,7 @@ final class WorkerSpawnRunner implements SpawnRunner { private WorkResponse execInWorker( WorkerKey key, WorkRequest request, - SpawnExecutionPolicy policy, + SpawnExecutionContext context, Map<PathFragment, Path> inputFiles, Set<PathFragment> outputFiles) throws InterruptedException, ExecException { @@ -327,7 +327,7 @@ final class WorkerSpawnRunner implements SpawnRunner { .toString()); } - policy.lockOutputFiles(); + context.lockOutputFiles(); if (response == null) { throw new UserExecException( |