diff options
author | Ulf Adams <ulfjack@google.com> | 2017-03-02 17:32:28 +0000 |
---|---|---|
committer | Yue Gan <yueg@google.com> | 2017-03-03 10:36:39 +0000 |
commit | cddaaa66b9aac7c4ea4772ae2a23b97426d267c2 (patch) | |
tree | eb018b744eff97144ccc2fb3850dfc184b907236 /src/main/java/com/google | |
parent | 64f3eb0492f675d0e4713989f2bdd175ff078758 (diff) |
Rationalize local resource acquisition
Move all local resource acquisition to where local execution actually happens.
Don't attempt to acquire resources per action, but only for individual spawns.
This significantly simplifies the code.
The downside is that we don't account for action-level work anymore. In
general, actions should not perform any process execution themselves, but
always delegate such work to a SpawnStrategy implementation.
This change makes sure that every Spawn has local resources set in a way that
is consistent with the previous state.
However, there are two actions - Fileset and FileWrite -, which are not spawns,
and so we now don't limit their concurrent execution anymore. For Fileset, all
work is done in a custom Fileset-specific thread pool, so this shouldn't be a
problem. I'm not sure about FileWriteAction.
--
PiperOrigin-RevId: 149012600
MOS_MIGRATED_REVID=149012600
Diffstat (limited to 'src/main/java/com/google')
18 files changed, 133 insertions, 121 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Action.java b/src/main/java/com/google/devtools/build/lib/actions/Action.java index ee193ba943..53468179e2 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Action.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Action.java @@ -188,6 +188,7 @@ public interface Action extends ActionExecutionMetadata, Describable { * executor parameter of the top-level call to * Builder.buildArtifacts(). */ + @Deprecated // TODO(ulfjack): Remove this. @Nullable ResourceSet estimateResourceConsumption(Executor executor); /** diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionStatusReporter.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionStatusReporter.java index fd56653208..a15165bd68 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionStatusReporter.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionStatusReporter.java @@ -23,7 +23,6 @@ import com.google.devtools.build.lib.util.BlazeClock; import com.google.devtools.build.lib.util.Clock; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; - import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -33,7 +32,6 @@ import java.util.Map; import java.util.Set; import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; - import javax.annotation.Nullable; /** @@ -116,10 +114,6 @@ public final class ActionExecutionStatusReporter { updateStatus(ActionStatusMessage.preparingStrategy(action)); } - public void setRunningFromBuildData(ActionExecutionMetadata action) { - updateStatus(ActionStatusMessage.runningStrategy(action, "unknown")); - } - @Subscribe public void updateStatus(ActionStatusMessage statusMsg) { String message = statusMsg.getMessage(); diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java index 6fd5966e26..e3143fddcc 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java @@ -189,19 +189,21 @@ public class BaseSpawn implements Spawn { return action.getMnemonic(); } - /** A local spawn requiring zero resources. */ + /** A local spawn. */ public static class Local extends BaseSpawn { public Local( - List<String> arguments, Map<String, String> environment, ActionExecutionMetadata action) { - this(arguments, environment, ImmutableMap.<String, String>of(), action); + List<String> arguments, Map<String, String> environment, ActionExecutionMetadata action, + ResourceSet localResources) { + this(arguments, environment, ImmutableMap.<String, String>of(), action, localResources); } public Local( List<String> arguments, Map<String, String> environment, Map<String, String> executionInfo, - ActionExecutionMetadata action) { - super(arguments, environment, buildExecutionInfo(executionInfo), action, ResourceSet.ZERO); + ActionExecutionMetadata action, + ResourceSet localResources) { + super(arguments, environment, buildExecutionInfo(executionInfo), action, localResources); } private static ImmutableMap<String, String> buildExecutionInfo( diff --git a/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java b/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java index 2f0483e7eb..60863dc5bc 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java @@ -17,7 +17,6 @@ package com.google.devtools.build.lib.actions; import static com.google.devtools.build.lib.profiler.AutoProfiler.profiled; import com.google.common.annotations.VisibleForTesting; -import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.profiler.AutoProfiler; import com.google.devtools.build.lib.profiler.ProfilerTask; @@ -84,8 +83,6 @@ public class ResourceManager { } } - private EventBus eventBus; - private final ThreadLocal<Boolean> threadLocked = new ThreadLocal<Boolean>() { @Override protected Boolean initialValue() { @@ -209,7 +206,6 @@ public class ResourceManager { AutoProfiler p = profiled(owner, ProfilerTask.ACTION_LOCK); CountDownLatch latch = null; try { - waiting(owner); latch = acquire(resources); if (latch != null) { latch.await(); @@ -229,8 +225,7 @@ public class ResourceManager { throw e; } - threadLocked.set(resources != ResourceSet.ZERO); - acquired(owner); + threadLocked.set(true); // Profile acquisition only if it waited for resource to become available. if (latch != null) { @@ -245,7 +240,8 @@ public class ResourceManager { * * @return a ResourceHandle iff the given resources were locked (all or nothing), null otherwise. */ - public ResourceHandle tryAcquire(ActionExecutionMetadata owner, ResourceSet resources) { + @VisibleForTesting + ResourceHandle tryAcquire(ActionExecutionMetadata owner, ResourceSet resources) { Preconditions.checkNotNull( resources, "tryAcquire called with resources == NULL during %s", owner); Preconditions.checkState( @@ -262,7 +258,6 @@ public class ResourceManager { if (acquired) { threadLocked.set(resources != ResourceSet.ZERO); - acquired(owner); return new ResourceHandle(this, owner, resources); } @@ -292,36 +287,13 @@ public class ResourceManager { return threadLocked.get(); } - public void setEventBus(EventBus eventBus) { - Preconditions.checkState(this.eventBus == null); - this.eventBus = Preconditions.checkNotNull(eventBus); - } - - public void unsetEventBus() { - Preconditions.checkState(this.eventBus != null); - this.eventBus = null; - } - - private void waiting(ActionExecutionMetadata owner) { - if (eventBus != null) { - // Null only in tests. - eventBus.post(ActionStatusMessage.schedulingStrategy(owner)); - } - } - - private void acquired(ActionExecutionMetadata owner) { - if (eventBus != null) { - // Null only in tests. - eventBus.post(ActionStatusMessage.runningStrategy(owner, "unknown")); - } - } - /** * Releases previously requested resource =. * * <p>NB! This method must be thread-safe! */ - public void releaseResources(ActionExecutionMetadata owner, ResourceSet resources) { + @VisibleForTesting + void releaseResources(ActionExecutionMetadata owner, ResourceSet resources) { Preconditions.checkNotNull( resources, "releaseResources called with resources == NULL during %s", owner); Preconditions.checkState( @@ -377,7 +349,6 @@ public class ResourceManager { return false; } - /** * Tries to unblock one or more waiting threads if there are sufficient resources available. */ diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java index 7dcd76f902..dc398a474d 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java @@ -26,7 +26,6 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.BuildFailedException; import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.MissingInputFileException; -import com.google.devtools.build.lib.actions.ResourceManager; import com.google.devtools.build.lib.actions.TestExecException; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.TopLevelArtifactContext; @@ -120,7 +119,6 @@ public class SkyframeBuilder implements Builder { skyframeExecutor .getEventBus() .post(new ExecutionProgressReceiverAvailableEvent(executionProgressReceiver)); - ResourceManager.instance().setEventBus(skyframeExecutor.getEventBus()); List<ExitCode> exitCodes = new LinkedList<>(); EvaluationResult<?> result; @@ -208,7 +206,6 @@ public class SkyframeBuilder implements Builder { } } finally { watchdog.stop(); - ResourceManager.instance().unsetEventBus(); skyframeExecutor.setActionExecutionProgressReportingObjects(null, null, null); statusReporter.unregisterFromEventBus(); } diff --git a/src/main/java/com/google/devtools/build/lib/exec/FileWriteStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/FileWriteStrategy.java index 332cb8a783..6604cbbcc8 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/FileWriteStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/FileWriteStrategy.java @@ -24,7 +24,6 @@ import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction; import com.google.devtools.build.lib.analysis.actions.FileWriteActionContext; import com.google.devtools.build.lib.profiler.AutoProfiler; import com.google.devtools.build.lib.vfs.Path; - import java.io.BufferedOutputStream; import java.io.IOException; import java.io.OutputStream; @@ -44,7 +43,7 @@ public final class FileWriteStrategy implements FileWriteActionContext { @Override public void exec(AbstractFileWriteAction action, ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException { - + // TODO(ulfjack): Consider acquiring local resources here before trying to write the file. try (AutoProfiler p = AutoProfiler.logged( "running " + action.prettyPrint(), LOG, /*minTimeForLoggingInMilliseconds=*/ 100)) { diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java index 0a1ff9777c..7e862d64ce 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java @@ -23,9 +23,6 @@ import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.actions.Executor; -import com.google.devtools.build.lib.actions.ResourceManager; -import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle; -import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.actions.TestExecException; @@ -246,13 +243,10 @@ public class StandaloneTestStrategy extends TestStrategy { Path workingDirectory) throws IOException, ExecException, InterruptedException { prepareFileSystem(action, tmpDir, coverageDir, workingDirectory); - ResourceSet resources = - action.getTestProperties().getLocalResourceUsage(executionOptions.usingLocalTestJobs()); try (FileOutErr fileOutErr = new FileOutErr( - action.getTestLog().getPath(), action.resolve(execRoot).getTestStderr()); - ResourceHandle handle = ResourceManager.instance().acquireResources(action, resources)) { + action.getTestLog().getPath(), action.resolve(execRoot).getTestStderr())) { TestResultData data = executeTest( action, diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java index c5a70c61e8..904c4b6407 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java @@ -21,8 +21,6 @@ import com.google.devtools.build.lib.actions.AbstractAction; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.BaseSpawn; import com.google.devtools.build.lib.actions.ExecException; -import com.google.devtools.build.lib.actions.ResourceManager; -import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.analysis.config.BinTools; @@ -120,12 +118,9 @@ public final class SymlinkTreeHelper { List<String> args = getSpawnArgumentList( actionExecutionContext.getExecutor().getExecRoot(), binTools); - try (ResourceHandle handle = - ResourceManager.instance().acquireResources(action, RESOURCE_SET)) { - actionExecutionContext.getExecutor().getSpawnActionContext(action.getMnemonic()).exec( - new BaseSpawn.Local(args, shellEnvironment, action), - actionExecutionContext); - } + actionExecutionContext.getExecutor().getSpawnActionContext(action.getMnemonic()).exec( + new BaseSpawn.Local(args, shellEnvironment, action, RESOURCE_SET), + actionExecutionContext); } else { // Pretend we created the runfiles tree by copying the manifest try { diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java index b38e29cecc..6782e2d578 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java @@ -164,8 +164,6 @@ final class RemoteSpawnStrategy implements SpawnActionContext { String mnemonic = spawn.getMnemonic(); Executor executor = actionExecutionContext.getExecutor(); EventHandler eventHandler = executor.getEventHandler(); - executor.getEventBus().post( - ActionStatusMessage.runningStrategy(spawn.getResourceOwner(), "remote")); RemoteActionCache actionCache = null; RemoteWorkExecutor workExecutor = null; @@ -191,6 +189,12 @@ final class RemoteSpawnStrategy implements SpawnActionContext { standaloneStrategy.exec(spawn, actionExecutionContext); return; } + if (workExecutor == null) { + execLocally(spawn, actionExecutionContext, actionCache, actionKey); + return; + } + executor.getEventBus().post( + ActionStatusMessage.runningStrategy(spawn.getResourceOwner(), "remote")); try { // Temporary hack: the TreeNodeRepository should be created and maintained upstream! @@ -224,11 +228,6 @@ final class RemoteSpawnStrategy implements SpawnActionContext { } } - if (workExecutor == null) { - execLocally(spawn, actionExecutionContext, actionCache, actionKey); - return; - } - // Upload the command and all the inputs into the remote cache. actionCache.uploadBlob(command.toByteArray()); // TODO(olaola): this should use the ActionInputFileCache for SHA1 digests! diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java index 905bf75ee0..40f3194828 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java @@ -70,7 +70,7 @@ public class SpawnGccStrategy implements CppCompileActionContext { throws ExecException, InterruptedException { Executor executor = actionExecutionContext.getExecutor(); SpawnActionContext spawnActionContext = executor.getSpawnActionContext(action.getMnemonic()); - Spawn spawn = new GccSpawn(action, estimateResourceConsumption(action)); + Spawn spawn = new GccSpawn(action, action.estimateResourceConsumptionLocal()); spawnActionContext.exec(spawn, actionExecutionContext); return null; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnLinkStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnLinkStrategy.java index 58883074e0..c88a3c6447 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnLinkStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnLinkStrategy.java @@ -44,7 +44,7 @@ public final class SpawnLinkStrategy implements CppLinkActionContext { action.getEnvironment(), action.getExecutionInfo(), action, - estimateResourceConsumption(action)); + action.estimateResourceConsumptionLocal()); spawnActionContext.exec(spawn, actionExecutionContext); } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java index 8d464ea171..926e23a3af 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java @@ -20,11 +20,16 @@ import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.ActionExecutionMetadata; +import com.google.devtools.build.lib.actions.ActionStatusMessage; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.actions.Executor; +import com.google.devtools.build.lib.actions.ResourceManager; +import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.actions.UserExecException; @@ -155,15 +160,29 @@ public class DarwinSandboxedStrategy extends SandboxStrategy { AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles) throws ExecException, InterruptedException { Executor executor = actionExecutionContext.getExecutor(); - // Certain actions can't run remotely or in a sandbox - pass them on to the standalone strategy. if (!spawn.isRemotable() || spawn.hasNoSandbox()) { SandboxHelpers.fallbackToNonSandboxedExecution(spawn, actionExecutionContext, executor); return; } + EventBus eventBus = actionExecutionContext.getExecutor().getEventBus(); + ActionExecutionMetadata owner = spawn.getResourceOwner(); + eventBus.post(ActionStatusMessage.schedulingStrategy(owner)); + try (ResourceHandle handle = + ResourceManager.instance().acquireResources(owner, spawn.getLocalResources())) { + SandboxHelpers.postActionStatusMessage(eventBus, spawn); + actuallyExec(spawn, actionExecutionContext, writeOutputFiles); + } + } + + private void actuallyExec( + Spawn spawn, + ActionExecutionContext actionExecutionContext, + AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles) + throws ExecException, InterruptedException { + Executor executor = actionExecutionContext.getExecutor(); SandboxHelpers.reportSubcommand(executor, spawn); - SandboxHelpers.postActionStatusMessage(executor, spawn); PrintWriter errWriter = sandboxDebug diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java index e2a81c8af5..e7d742c2ed 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java @@ -17,10 +17,15 @@ package com.google.devtools.build.lib.sandbox; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; +import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.ActionExecutionMetadata; +import com.google.devtools.build.lib.actions.ActionStatusMessage; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.actions.Executor; +import com.google.devtools.build.lib.actions.ResourceManager; +import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.actions.UserExecException; @@ -97,15 +102,29 @@ public class LinuxSandboxedStrategy extends SandboxStrategy { AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles) throws ExecException, InterruptedException { Executor executor = actionExecutionContext.getExecutor(); - // Certain actions can't run remotely or in a sandbox - pass them on to the standalone strategy. if (!spawn.isRemotable() || spawn.hasNoSandbox()) { SandboxHelpers.fallbackToNonSandboxedExecution(spawn, actionExecutionContext, executor); return; } + EventBus eventBus = actionExecutionContext.getExecutor().getEventBus(); + ActionExecutionMetadata owner = spawn.getResourceOwner(); + eventBus.post(ActionStatusMessage.schedulingStrategy(owner)); + try (ResourceHandle handle = + ResourceManager.instance().acquireResources(owner, spawn.getLocalResources())) { + SandboxHelpers.postActionStatusMessage(eventBus, spawn); + actuallyExec(spawn, actionExecutionContext, writeOutputFiles); + } + } + + public void actuallyExec( + Spawn spawn, + ActionExecutionContext actionExecutionContext, + AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles) + throws ExecException, InterruptedException { + Executor executor = actionExecutionContext.getExecutor(); SandboxHelpers.reportSubcommand(executor, spawn); - SandboxHelpers.postActionStatusMessage(executor, spawn); // Each invocation of "exec" gets its own sandbox. Path sandboxPath = SandboxHelpers.getSandboxRoot(blazeDirs, productName, uuid, execCounter); 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 bdedfade73..2a4c7f75bf 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 @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.sandbox; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet.Builder; +import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionStatusMessage; @@ -37,7 +38,7 @@ public final class SandboxHelpers { static void fallbackToNonSandboxedExecution( Spawn spawn, ActionExecutionContext actionExecutionContext, Executor executor) - throws ExecException { + throws ExecException, InterruptedException { StandaloneSpawnStrategy standaloneStrategy = Preconditions.checkNotNull(executor.getContext(StandaloneSpawnStrategy.class)); standaloneStrategy.exec(spawn, actionExecutionContext); @@ -81,10 +82,8 @@ public final class SandboxHelpers { return true; } - static void postActionStatusMessage(Executor executor, Spawn spawn) { - executor - .getEventBus() - .post(ActionStatusMessage.runningStrategy(spawn.getResourceOwner(), "sandbox")); + static void postActionStatusMessage(EventBus eventBus, Spawn spawn) { + eventBus.post(ActionStatusMessage.runningStrategy(spawn.getResourceOwner(), "sandbox")); } static Path getSandboxRoot( 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 1d23376469..1396334445 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 @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.ActionLogBufferPathGenerator; import com.google.devtools.build.lib.actions.ActionMiddlemanEvent; import com.google.devtools.build.lib.actions.ActionStartedEvent; +import com.google.devtools.build.lib.actions.ActionStatusMessage; import com.google.devtools.build.lib.actions.Actions; import com.google.devtools.build.lib.actions.AlreadyReportedActionExecutionException; import com.google.devtools.build.lib.actions.Artifact; @@ -51,9 +52,6 @@ import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictEx import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit; import com.google.devtools.build.lib.actions.PackageRootResolutionException; import com.google.devtools.build.lib.actions.PackageRootResolver; -import com.google.devtools.build.lib.actions.ResourceManager; -import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle; -import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.TargetOutOfDateException; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.cmdline.Label; @@ -104,7 +102,6 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto private Reporter reporter; private final AtomicReference<EventBus> eventBus; - private final ResourceManager resourceManager; private Map<String, String> clientEnv = ImmutableMap.of(); private Executor executorEngine; private ActionLogBufferPathGenerator actionLogBufferPathGenerator; @@ -139,10 +136,9 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto private final AtomicReference<ActionExecutionStatusReporter> statusReporterRef; private OutputService outputService; - SkyframeActionExecutor(ResourceManager resourceManager, + SkyframeActionExecutor( AtomicReference<EventBus> eventBus, AtomicReference<ActionExecutionStatusReporter> statusReporterRef) { - this.resourceManager = resourceManager; this.eventBus = eventBus; this.statusReporterRef = statusReporterRef; } @@ -703,24 +699,13 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto } postEvent(new ActionStartedEvent(action, actionStartTime)); - ResourceSet estimate = - Preconditions.checkNotNull(action.estimateResourceConsumption(executorEngine)); ActionExecutionStatusReporter statusReporter = statusReporterRef.get(); - ResourceHandle handle = null; try { - if (estimate == ResourceSet.ZERO) { - statusReporter.setRunningFromBuildData(action); - } else { - // If estimated resource consumption is null, action will manually call - // resource manager when it knows what resources are needed. - handle = resourceManager.acquireResources(action, estimate); - } + // Mark the current action as being prepared. + statusReporter.updateStatus(ActionStatusMessage.preparingStrategy(action)); boolean outputDumped = executeActionTask(action, context); completeAction(action, context.getMetadataHandler(), context.getFileOutErr(), outputDumped); } finally { - if (handle != null) { - handle.close(); - } statusReporter.remove(action); postEvent(new ActionCompletionEvent(actionStartTime, action)); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index b54b32daf9..2dc660ff68 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -316,8 +316,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { new SkyframePackageLoader(), new SkyframeTransitivePackageLoader(), syscalls, cyclesReporter, pkgLocator, numPackagesLoaded, this); this.resourceManager = ResourceManager.instance(); - this.skyframeActionExecutor = new SkyframeActionExecutor( - resourceManager, eventBus, statusReporterRef); + this.skyframeActionExecutor = new SkyframeActionExecutor(eventBus, statusReporterRef); this.directories = Preconditions.checkNotNull(directories); this.buildInfoFactories = buildInfoFactories; this.allowedMissingInputs = allowedMissingInputs; diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java index 9dc4b757aa..6e2b3ce4ae 100644 --- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java @@ -14,11 +14,15 @@ package com.google.devtools.build.lib.standalone; import com.google.common.collect.ImmutableMap; +import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.ActionStatusMessage; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.actions.Executor; +import com.google.devtools.build.lib.actions.ResourceManager; +import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.actions.Spawns; @@ -48,13 +52,20 @@ public class StandaloneSpawnStrategy implements SpawnActionContext { private final Path processWrapper; private final Path execRoot; private final String productName; + private final ResourceManager resourceManager; public StandaloneSpawnStrategy(Path execRoot, boolean verboseFailures, String productName) { + this(execRoot, verboseFailures, productName, ResourceManager.instance()); + } + + public StandaloneSpawnStrategy( + Path execRoot, boolean verboseFailures, String productName, ResourceManager resourceManager) { this.verboseFailures = verboseFailures; this.execRoot = execRoot; this.processWrapper = execRoot.getRelative( "_bin/process-wrapper" + OsUtils.executableExtension()); this.productName = productName; + this.resourceManager = resourceManager; } /** @@ -63,6 +74,22 @@ public class StandaloneSpawnStrategy implements SpawnActionContext { @Override public void exec(Spawn spawn, ActionExecutionContext actionExecutionContext) + throws ExecException, InterruptedException { + EventBus eventBus = actionExecutionContext.getExecutor().getEventBus(); + ActionExecutionMetadata owner = spawn.getResourceOwner(); + eventBus.post(ActionStatusMessage.schedulingStrategy(owner)); + try (ResourceHandle handle = + resourceManager.acquireResources(owner, spawn.getLocalResources())) { + eventBus.post(ActionStatusMessage.runningStrategy(owner, "standalone")); + actuallyExec(spawn, actionExecutionContext); + } + } + + /** + * Executes the given {@code spawn}. + */ + private void actuallyExec(Spawn spawn, + ActionExecutionContext actionExecutionContext) throws ExecException { Executor executor = actionExecutionContext.getExecutor(); @@ -70,10 +97,6 @@ public class StandaloneSpawnStrategy implements SpawnActionContext { executor.reportSubcommand(spawn); } - executor - .getEventBus() - .post(ActionStatusMessage.runningStrategy(spawn.getResourceOwner(), "standalone")); - int timeoutSeconds = Spawns.getTimeoutSeconds(spawn); // We must wrap the subprocess with process-wrapper to kill the process tree. diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java index dab387db1d..a0fadc8722 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java @@ -22,8 +22,10 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.Multimap; +import com.google.common.eventbus.EventBus; import com.google.common.hash.HashCode; import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputFileCache; import com.google.devtools.build.lib.actions.ActionInputHelper; @@ -31,6 +33,8 @@ import com.google.devtools.build.lib.actions.ActionStatusMessage; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.actions.Executor; +import com.google.devtools.build.lib.actions.ResourceManager; +import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle; import com.google.devtools.build.lib.actions.SandboxedSpawnActionContext; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; @@ -204,23 +208,39 @@ public final class WorkerSpawnStrategy implements SandboxedSpawnActionContext { AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles) throws ExecException, InterruptedException { Executor executor = actionExecutionContext.getExecutor(); - EventHandler eventHandler = executor.getEventHandler(); - StandaloneSpawnStrategy standaloneStrategy = - Preconditions.checkNotNull(executor.getContext(StandaloneSpawnStrategy.class)); - - if (executor.reportsSubcommands()) { - executor.reportSubcommand(spawn); - } - if (!spawn.getExecutionInfo().containsKey("supports-workers") || !spawn.getExecutionInfo().get("supports-workers").equals("1")) { - eventHandler.handle( + StandaloneSpawnStrategy standaloneStrategy = + Preconditions.checkNotNull(executor.getContext(StandaloneSpawnStrategy.class)); + executor.getEventHandler().handle( Event.warn( String.format(ERROR_MESSAGE_PREFIX + REASON_NO_EXECUTION_INFO, spawn.getMnemonic()))); standaloneStrategy.exec(spawn, actionExecutionContext); return; } + EventBus eventBus = actionExecutionContext.getExecutor().getEventBus(); + ActionExecutionMetadata owner = spawn.getResourceOwner(); + eventBus.post(ActionStatusMessage.schedulingStrategy(owner)); + try (ResourceHandle handle = + ResourceManager.instance().acquireResources(owner, spawn.getLocalResources())) { + eventBus.post(ActionStatusMessage.runningStrategy(spawn.getResourceOwner(), "worker")); + actuallyExec(spawn, actionExecutionContext, writeOutputFiles); + } + } + + private void actuallyExec( + Spawn spawn, + ActionExecutionContext actionExecutionContext, + AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles) + throws ExecException, InterruptedException { + Executor executor = actionExecutionContext.getExecutor(); + EventHandler eventHandler = executor.getEventHandler(); + + if (executor.reportsSubcommands()) { + executor.reportSubcommand(spawn); + } + // We assume that the spawn to be executed always gets a @flagfile argument, which contains the // flags related to the work itself (as opposed to start-up options for the executed tool). // Thus, we can extract the last element from its args (which will be the @flagfile), expand it @@ -235,10 +255,6 @@ public final class WorkerSpawnStrategy implements SandboxedSpawnActionContext { String.format(ERROR_MESSAGE_PREFIX + REASON_NO_TOOLS, spawn.getMnemonic())); } - executor - .getEventBus() - .post(ActionStatusMessage.runningStrategy(spawn.getResourceOwner(), "worker")); - FileOutErr outErr = actionExecutionContext.getFileOutErr(); ImmutableList<String> args = |