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
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 ee193ba..5346817 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 @@
* 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 fd56653..a15165b 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.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.Set;
import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
-
import javax.annotation.Nullable;
/**
@@ -116,10 +114,6 @@
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 6fd5966..e3143fd 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 @@
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 2f0483e..60863dc 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 @@
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 @@
}
}
- private EventBus eventBus;
-
private final ThreadLocal<Boolean> threadLocked = new ThreadLocal<Boolean>() {
@Override
protected Boolean initialValue() {
@@ -209,7 +206,6 @@
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 @@
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 @@
*
* @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 @@
if (acquired) {
threadLocked.set(resources != ResourceSet.ZERO);
- acquired(owner);
return new ResourceHandle(this, owner, resources);
}
@@ -292,36 +287,13 @@
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 @@
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 7dcd76f..dc398a4 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.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 @@
skyframeExecutor
.getEventBus()
.post(new ExecutionProgressReceiverAvailableEvent(executionProgressReceiver));
- ResourceManager.instance().setEventBus(skyframeExecutor.getEventBus());
List<ExitCode> exitCodes = new LinkedList<>();
EvaluationResult<?> result;
@@ -208,7 +206,6 @@
}
} 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 332cb8a..6604cbb 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.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 @@
@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 0a1ff97..7e862d6 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.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 @@
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 c5a70c6..904c4b6 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.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 @@
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 b38e29c..6782e2d 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 @@
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 @@
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 @@
}
}
- 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 905bf75..40f3194 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 @@
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 5888307..c88a3c6 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 @@
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 8d464ea..926e23a 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.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 @@
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 e2a81c8..e7d742c 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 @@
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 @@
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 bdedfad..2a4c7f7 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 @@
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 @@
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 @@
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 1d23376..1396334 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.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.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 @@
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 @@
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 @@
}
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 b54b32d..2dc660f 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 @@
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 9dc4b75..6e2b3ce 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 @@
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 @@
@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 @@
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 dab387d..a0fadc8 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.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.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 @@
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 @@
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 =
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java
index 9605c31..32d7115 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java
@@ -21,7 +21,6 @@
import static org.junit.Assert.fail;
import com.google.common.collect.ImmutableSet;
-import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle;
import com.google.devtools.build.lib.testutil.TestThread;
import com.google.devtools.build.lib.testutil.TestUtils;
@@ -53,16 +52,15 @@
rm.setAvailableResources(
ResourceSet.create(/*memoryMb=*/1000.0, /*cpuUsage=*/1.0, /*ioUsage=*/1.0,
/*testCount=*/2));
- rm.setEventBus(new EventBus());
counter = new AtomicInteger(0);
sync = new CyclicBarrier(2);
sync2 = new CyclicBarrier(2);
rm.resetResourceUsage();
}
- private void acquire(double ram, double cpu, double io, int tests)
+ private ResourceHandle acquire(double ram, double cpu, double io, int tests)
throws InterruptedException {
- rm.acquireResources(resourceOwner, ResourceSet.create(ram, cpu, io, tests));
+ return rm.acquireResources(resourceOwner, ResourceSet.create(ram, cpu, io, tests));
}
private ResourceHandle acquireNonblocking(double ram, double cpu, double io, int tests) {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ParallelBuilderTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ParallelBuilderTest.java
index 159c3c1..75dcc14 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ParallelBuilderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ParallelBuilderTest.java
@@ -32,10 +32,6 @@
import com.google.devtools.build.lib.actions.ActionExecutionException;
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.LocalHostCapacity;
-import com.google.devtools.build.lib.actions.ResourceManager;
-import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.cache.ActionCache;
import com.google.devtools.build.lib.actions.util.TestAction;
import com.google.devtools.build.lib.events.Event;
@@ -87,10 +83,6 @@
@Before
public final void setUp() throws Exception {
this.cache = new InMemoryActionCache();
- ResourceManager.instance().setAvailableResources(LocalHostCapacity.getLocalHostCapacity());
- ResourceManager.instance().setRamUtilizationPercentage(
- ResourceManager.DEFAULT_RAM_UTILIZATION_PERCENTAGE);
- ResourceManager.instance().resetResourceUsage();
}
@SafeVarargs
@@ -571,79 +563,6 @@
}
@Test
- public void testSchedulingOfMemoryResources() throws Exception {
- // The action graph consists of 100 independent actions, but execution is
- // memory limited: only 6 TestActions can run concurrently:
- ResourceManager.instance().setRamUtilizationPercentage(50);
- ResourceManager.instance().setAvailableResources(
- ResourceSet.createWithRamCpuIo(/*memoryMb=*/12.8, /*cpu=*/Integer.MAX_VALUE, /*io=*/0.0));
- ResourceManager.instance().resetResourceUsage();
-
- class Counter {
- int currentlyRunning = 0;
- int maxConcurrent = 0;
- synchronized void increment() {
- ++currentlyRunning;
- if (currentlyRunning > maxConcurrent) {
- maxConcurrent = currentlyRunning;
- }
- }
- synchronized void decrement() {
- currentlyRunning--;
- }
- }
- final Counter counter = new Counter();
-
- Artifact[] outputs = new Artifact[100];
- for (int ii = 0; ii < outputs.length; ++ii) {
- Artifact artifact = createDerivedArtifact("file" + ii);
- Callable<Void> callable = new Callable<Void>() {
- @Override
- public Void call() throws Exception{
- counter.increment();
- Thread.sleep(100); // 100ms
- counter.decrement();
- return null;
- }
- };
- registerAction(new TestAction(callable, Artifact.NO_ARTIFACTS, ImmutableList.of(artifact)));
- outputs[ii] = artifact;
- }
-
- buildArtifacts(outputs);
-
- assertEquals(0, counter.currentlyRunning);
- assertEquals(6, counter.maxConcurrent);
- }
-
- @Test
- public void testEstimateExceedsAvailableRam() throws Exception {
- // Pretend that the machine has only 1MB of RAM available,
- // then test running an action that we estimate requires 2MB of RAM.
-
- ResourceManager.instance().setAvailableResources(
- ResourceSet.createWithRamCpuIo(/*memoryMb=*/1.0, /*cpuUsage=*/4, /*ioUsage=*/0));
- ResourceManager.instance().resetResourceUsage();
-
- final boolean[] finished = { false };
- Artifact foo = createDerivedArtifact("foo");
- Runnable makeFoo = new Runnable() {
- @Override
- public void run() {
- finished[0] = true;
- }
- };
- registerAction(new TestAction(makeFoo, emptySet, asSet(foo)) {
- @Override
- public ResourceSet estimateResourceConsumption(Executor executor) {
- return ResourceSet.createWithRamCpuIo(/*memoryMb=*/2.0, /*cpuUsage=*/1, /*ioUsage=*/0);
- }
- });
- buildArtifacts(foo);
- assertTrue(finished[0]);
- }
-
- @Test
public void testCyclicActionGraph() throws Exception {
// foo -> [action] -> bar
// bar -> [action] -> baz
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
index cccb456..4110752 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -40,7 +40,6 @@
import com.google.devtools.build.lib.actions.Root;
import com.google.devtools.build.lib.actions.TestExecException;
import com.google.devtools.build.lib.actions.cache.ActionCache;
-import com.google.devtools.build.lib.actions.cache.ActionCache.Entry;
import com.google.devtools.build.lib.actions.util.DummyExecutor;
import com.google.devtools.build.lib.actions.util.TestAction;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
@@ -162,8 +161,7 @@
ActionExecutionStatusReporter statusReporter =
ActionExecutionStatusReporter.create(new StoredEventHandler());
final SkyframeActionExecutor skyframeActionExecutor =
- new SkyframeActionExecutor(
- ResourceManager.instance(), eventBusRef, new AtomicReference<>(statusReporter));
+ new SkyframeActionExecutor(eventBusRef, new AtomicReference<>(statusReporter));
Path actionOutputBase = scratch.dir("/usr/local/google/_blaze_jrluser/FAKEMD5/action_out/");
skyframeActionExecutor.setActionLogBufferPathGenerator(
diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
index c9578bf..10e76cd 100644
--- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
@@ -26,6 +26,8 @@
import com.google.devtools.build.lib.actions.BaseSpawn;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.Executor.ActionContext;
+import com.google.devtools.build.lib.actions.ResourceManager;
+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.util.ActionsTestUtil;
@@ -96,6 +98,9 @@
EventBus bus = new EventBus();
+ ResourceManager resourceManager = ResourceManager.instanceForTestingOnly();
+ resourceManager.setAvailableResources(
+ ResourceSet.create(/*memoryMb=*/1, /*cpuUsage=*/1, /*ioUsage=*/1, /*localTestCount=*/1));
this.executor =
new BlazeExecutor(
directories.getExecRoot(),
@@ -106,15 +111,19 @@
ImmutableList.<ActionContext>of(),
ImmutableMap.<String, SpawnActionContext>of(
"",
- new StandaloneSpawnStrategy(directories.getExecRoot(), false, "mock-product-name")),
+ new StandaloneSpawnStrategy(
+ directories.getExecRoot(), false, "mock-product-name", resourceManager)),
ImmutableList.<ActionContextProvider>of());
executor.getExecRoot().createDirectory();
}
private Spawn createSpawn(String... arguments) {
- return new BaseSpawn.Local(Arrays.asList(arguments), ImmutableMap.<String, String>of(),
- new ActionsTestUtil.NullAction());
+ return new BaseSpawn.Local(
+ Arrays.asList(arguments),
+ ImmutableMap.<String, String>of(),
+ new ActionsTestUtil.NullAction(),
+ ResourceSet.ZERO);
}
private TestFileOutErr outErr = new TestFileOutErr();
@@ -185,9 +194,11 @@
@Test
public void testCommandHonorsEnvironment() throws Exception {
- Spawn spawn = new BaseSpawn.Local(Arrays.asList("/usr/bin/env"),
+ Spawn spawn = new BaseSpawn.Local(
+ Arrays.asList("/usr/bin/env"),
ImmutableMap.of("foo", "bar", "baz", "boo"),
- new ActionsTestUtil.NullAction());
+ new ActionsTestUtil.NullAction(),
+ ResourceSet.ZERO);
run(spawn);
assertEquals(Sets.newHashSet("foo=bar", "baz=boo"), Sets.newHashSet(out().split("\n")));
}
@@ -208,10 +219,12 @@
if (OS.getCurrent() == OS.DARWIN) {
return;
}
- Spawn spawn = new BaseSpawn.Local(Arrays.asList("/bin/sh", "-c", "echo $SDKROOT"),
+ Spawn spawn = new BaseSpawn.Local(
+ Arrays.asList("/bin/sh", "-c", "echo $SDKROOT"),
ImmutableMap.<String, String>of(AppleConfiguration.APPLE_SDK_VERSION_ENV_NAME, "8.4",
AppleConfiguration.APPLE_SDK_PLATFORM_ENV_NAME, "iPhoneSimulator"),
- new ActionsTestUtil.NullAction());
+ new ActionsTestUtil.NullAction(),
+ ResourceSet.ZERO);
try {
run(spawn);
