aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/main/java/com/google
diff options
context:
space:
mode:
authorGravatar Ulf Adams <ulfjack@google.com>2017-03-02 17:32:28 +0000
committerGravatar Yue Gan <yueg@google.com>2017-03-03 10:36:39 +0000
commitcddaaa66b9aac7c4ea4772ae2a23b97426d267c2 (patch)
treeeb018b744eff97144ccc2fb3850dfc184b907236 /src/main/java/com/google
parent64f3eb0492f675d0e4713989f2bdd175ff078758 (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')
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/Action.java1
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/ActionExecutionStatusReporter.java6
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java12
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java39
-rw-r--r--src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java3
-rw-r--r--src/main/java/com/google/devtools/build/lib/exec/FileWriteStrategy.java3
-rw-r--r--src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java8
-rw-r--r--src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java11
-rw-r--r--src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java13
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java2
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnLinkStrategy.java2
-rw-r--r--src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java23
-rw-r--r--src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java23
-rw-r--r--src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java9
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java23
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java3
-rw-r--r--src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java31
-rw-r--r--src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java42
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 =