Skip to content

Commit

Permalink
Add 'standalone' in the defaults for --dynamic_local_strategy when …
Browse files Browse the repository at this point in the history
…we have local lockfree. Once we have flipped and removed the local lockfree flag, this can just be the default.

PiperOrigin-RevId: 527854309
Change-Id: Ia934f794d7d6e32bb4f46df4e0966802390c1e09
  • Loading branch information
larsrc-google authored and copybara-github committed Apr 28, 2023
1 parent e4f3d4d commit 40cf855
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 13 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/dynamic/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/exec:execution_options",
"//src/main/java/com/google/devtools/build/lib/exec:execution_policy",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_registry",
"//src/main/java/com/google/devtools/build/lib/exec/local:options",
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.exec.ExecutionPolicy;
import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
import com.google.devtools.build.lib.exec.local.LocalExecutionOptions;
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.Command;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
Expand All @@ -56,6 +57,7 @@ public class DynamicExecutionModule extends BlazeModule {
Set<Integer> ignoreLocalSignals = ImmutableSet.of();
protected Reporter reporter;
protected boolean verboseFailures;
private LocalExecutionOptions localOptions;

public DynamicExecutionModule() {}

Expand All @@ -82,6 +84,7 @@ public void beforeCommand(CommandEnvironment env) {
verboseFailures = executionOptions != null && executionOptions.verboseFailures;
DynamicExecutionOptions dynamicOptions =
env.getOptions().getOptions(DynamicExecutionOptions.class);
localOptions = env.getOptions().getOptions(LocalExecutionOptions.class);
ignoreLocalSignals =
dynamicOptions != null && dynamicOptions.ignoreLocalSignals != null
? dynamicOptions.ignoreLocalSignals
Expand All @@ -94,17 +97,19 @@ ImmutableMap<String, List<String>> getLocalStrategies(DynamicExecutionOptions op
throws AbruptExitException {
// Options that set "allowMultiple" to true ignore the default value, so we replicate that
// functionality here.
// ImmutableMap.Builder fails on duplicates, so we use a regular map first to remove dups.
Map<String, List<String>> localAndWorkerStrategies = new HashMap<>();
localAndWorkerStrategies.put("", ImmutableList.of("worker", "sandboxed"));
ImmutableMap.Builder<String, List<String>> localAndWorkerStrategies = ImmutableMap.builder();
if (localOptions != null && localOptions.localLockfreeOutput) {
localAndWorkerStrategies.put("", ImmutableList.of("worker", "sandboxed", "standalone"));
} else {
// Without local lock free, having standalone execution risks very bad performance.
localAndWorkerStrategies.put("", ImmutableList.of("worker", "sandboxed"));
}

if (!options.dynamicLocalStrategy.isEmpty()) {
for (Map.Entry<String, List<String>> entry : options.dynamicLocalStrategy) {
localAndWorkerStrategies.put(entry.getKey(), entry.getValue());
throwIfContainsDynamic(entry.getValue(), "--dynamic_local_strategy");
}
for (Map.Entry<String, List<String>> entry : options.dynamicLocalStrategy) {
localAndWorkerStrategies.put(entry);
throwIfContainsDynamic(entry.getValue(), "--dynamic_local_strategy");
}
return ImmutableMap.copyOf(localAndWorkerStrategies);
return localAndWorkerStrategies.buildKeepingLast();
}

private ImmutableMap<String, List<String>> getRemoteStrategies(DynamicExecutionOptions options)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ public class DynamicExecutionOptions extends OptionsBase {
+ "strategy is used. For example, `worker,sandboxed` runs actions that support "
+ "persistent workers using the worker strategy, and all others using the sandboxed "
+ "strategy. If no mnemonic is given, the list of strategies is used as the "
+ "fallback for all mnemonics. The default fallback list is `worker,sandboxed`. "
+ "fallback for all mnemonics. The default fallback list is `worker,sandboxed`, or"
+ "`worker,sandboxed,standalone` if `experimental_local_lockfree_output` is set. "
+ "Takes [mnemonic=]local_strategy[,local_strategy,...]")
public List<Map.Entry<String, List<String>>> dynamicLocalStrategy;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ public class LocalExecutionOptions extends OptionsBase {
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.EXECUTION},
help =
"When true, the local spawn runner does lock the output tree during dynamic execution. "
+ "Instead, spawns are allowed to execute until they are explicitly interrupted by a "
+ "faster remote action.")
"When true, the local spawn runner doesn't lock the output tree during dynamic "
+ "execution. Instead, spawns are allowed to execute until they are explicitly "
+ "interrupted by a faster remote action.")
public boolean localLockfreeOutput;

@Option(
Expand Down

0 comments on commit 40cf855

Please sign in to comment.