Skip to content

Commit

Permalink
[6.2.0]Fix worker and multiplex workers for DexBuilder and Desugar ac…
Browse files Browse the repository at this point in the history
…tions (#17965)
  • Loading branch information
Bencodes authored Apr 19, 2023
1 parent 97312f3 commit 76ad4a9
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,15 @@ public static class BuildGraveyardOptions extends OptionsBase {
help = "Deprecated no-op. Use --experimental_dynamic_local_load_factor instead.")
@Deprecated
public boolean skipFirstBuild;

@Option(
name = "use_workers_with_dexbuilder",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.EXECUTION},
help = "This option is deprecated and has no effect.")
@Deprecated
public boolean useWorkersWithDexbuilder;
}

/** This is where deprecated Bazel-specific options only used by the build command go to die. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -533,14 +533,6 @@ public static class Options extends FragmentOptions {
help = "dx flags supported in tool that groups classes for inclusion in final .dex files.")
public List<String> dexoptsSupportedInDexSharder;

@Option(
name = "use_workers_with_dexbuilder",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.EXECUTION},
help = "Whether dexbuilder supports being run in local worker mode.")
public boolean useWorkersWithDexbuilder;

@Option(
name = "experimental_android_rewrite_dexes_with_rex",
defaultValue = "false",
Expand Down Expand Up @@ -905,10 +897,11 @@ public static class Options extends FragmentOptions {
},
help = "Enable persistent Android dex and desugar actions by using workers.",
expansion = {
"--internal_persistent_android_dex_desugar",
"--strategy=Desugar=worker",
"--strategy=DexBuilder=worker",
})
public Void persistentDexDesugar;
public Void persistentAndroidDexDesugar;

@Option(
name = "persistent_multiplex_android_dex_desugar",
Expand All @@ -921,10 +914,9 @@ public static class Options extends FragmentOptions {
help = "Enable persistent multiplexed Android dex and desugar actions by using workers.",
expansion = {
"--persistent_android_dex_desugar",
"--modify_execution_info=Desugar=+supports-multiplex-workers",
"--modify_execution_info=DexBuilder=+supports-multiplex-workers",
"--internal_persistent_multiplex_android_dex_desugar",
})
public Void persistentMultiplexDexDesugar;
public Void persistentMultiplexAndroidDexDesugar;

@Option(
name = "persistent_multiplex_android_tools",
Expand Down Expand Up @@ -974,6 +966,36 @@ public static class Options extends FragmentOptions {
help = "Tracking flag for when multiplexed busybox workers are enabled.")
public boolean persistentMultiplexBusyboxTools;

/**
* We use this option to decide when to enable workers for busybox tools. This flag is also a
* guard against enabling workers using nothing but --persistent_android_resource_processor.
*
* <p>Consequently, we use this option to decide between param files or regular command line
* parameters. If we're not using workers or on Windows, there's no need to always use param
* files for I/O performance reasons.
*/
@Option(
name = "internal_persistent_android_dex_desugar",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {
OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS,
OptionEffectTag.EXECUTION,
},
defaultValue = "false",
help = "Tracking flag for when dexing and desugaring workers are enabled.")
public boolean persistentDexDesugar;

@Option(
name = "internal_persistent_multiplex_android_dex_desugar",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {
OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS,
OptionEffectTag.EXECUTION,
},
defaultValue = "false",
help = "Tracking flag for when multiplexed dexing and desugaring workers are enabled.")
public boolean persistentMultiplexDexDesugar;

@Option(
name = "experimental_remove_r_classes_from_instrumentation_test_jar",
defaultValue = "true",
Expand Down Expand Up @@ -1100,7 +1122,6 @@ public FragmentOptions getHost() {
host.dexoptsSupportedInIncrementalDexing = dexoptsSupportedInIncrementalDexing;
host.dexoptsSupportedInDexMerger = dexoptsSupportedInDexMerger;
host.dexoptsSupportedInDexSharder = dexoptsSupportedInDexSharder;
host.useWorkersWithDexbuilder = useWorkersWithDexbuilder;
host.manifestMerger = manifestMerger;
host.manifestMergerOrder = manifestMergerOrder;
host.allowAndroidLibraryDepsWithoutSrcs = allowAndroidLibraryDepsWithoutSrcs;
Expand Down Expand Up @@ -1128,7 +1149,6 @@ public FragmentOptions getHost() {
private final ImmutableList<String> targetDexoptsThatPreventIncrementalDexing;
private final ImmutableList<String> dexoptsSupportedInDexMerger;
private final ImmutableList<String> dexoptsSupportedInDexSharder;
private final boolean useWorkersWithDexbuilder;
private final boolean desugarJava8;
private final boolean desugarJava8Libs;
private final boolean checkDesugarDeps;
Expand All @@ -1155,6 +1175,8 @@ public FragmentOptions getHost() {
private final boolean dataBindingAndroidX;
private final boolean persistentBusyboxTools;
private final boolean persistentMultiplexBusyboxTools;
private final boolean persistentDexDesugar;
private final boolean persistentMultiplexDexDesugar;
private final boolean filterRJarsFromAndroidTest;
private final boolean removeRClassesFromInstrumentationTestJar;
private final boolean alwaysFilterDuplicateClassesFromAndroidTest;
Expand Down Expand Up @@ -1184,7 +1206,6 @@ public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurati
ImmutableList.copyOf(options.nonIncrementalPerTargetDexopts);
this.dexoptsSupportedInDexMerger = ImmutableList.copyOf(options.dexoptsSupportedInDexMerger);
this.dexoptsSupportedInDexSharder = ImmutableList.copyOf(options.dexoptsSupportedInDexSharder);
this.useWorkersWithDexbuilder = options.useWorkersWithDexbuilder;
this.desugarJava8 = options.desugarJava8;
this.desugarJava8Libs = options.desugarJava8Libs;
this.checkDesugarDeps = options.checkDesugarDeps;
Expand Down Expand Up @@ -1216,6 +1237,8 @@ public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurati
this.dataBindingAndroidX = options.dataBindingAndroidX;
this.persistentBusyboxTools = options.persistentBusyboxTools;
this.persistentMultiplexBusyboxTools = options.persistentMultiplexBusyboxTools;
this.persistentDexDesugar = options.persistentDexDesugar;
this.persistentMultiplexDexDesugar = options.persistentMultiplexDexDesugar;
this.filterRJarsFromAndroidTest = options.filterRJarsFromAndroidTest;
this.removeRClassesFromInstrumentationTestJar =
options.removeRClassesFromInstrumentationTestJar;
Expand Down Expand Up @@ -1319,12 +1342,6 @@ public ImmutableList<String> getTargetDexoptsThatPreventIncrementalDexing() {
return targetDexoptsThatPreventIncrementalDexing;
}

/** Whether to assume the dexbuilder tool supports local worker mode. */
@Override
public boolean useWorkersWithDexbuilder() {
return useWorkersWithDexbuilder;
}

@Override
public boolean desugarJava8() {
return desugarJava8;
Expand Down Expand Up @@ -1473,6 +1490,16 @@ public boolean persistentMultiplexBusyboxTools() {
return persistentMultiplexBusyboxTools;
}

@Override
public boolean persistentDexDesugar() {
return persistentDexDesugar;
}

@Override
public boolean persistentMultiplexDexDesugar() {
return persistentMultiplexDexDesugar;
}

@Override
public boolean incompatibleUseToolchainResolution() {
return incompatibleUseToolchainResolution;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.common.base.Predicate;
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.collect.Iterables;
import com.google.common.collect.Sets;
Expand Down Expand Up @@ -526,7 +527,10 @@ private static Artifact createDesugarAction(
.addOutput(result)
.setMnemonic("Desugar")
.setProgressMessage("Desugaring %s for Android", jar.prettyPrint())
.setExecutionInfo(ExecutionRequirements.WORKER_MODE_ENABLED);
.setExecutionInfo(
createDexingDesugaringExecRequirements(ruleContext)
.putAll(ExecutionRequirements.WORKER_MODE_ENABLED)
.buildKeepingLast());

// SpawnAction.Builder.build() is documented as being safe for re-use. So we can call build here
// to get the action's inputs for vetting path stripping safety, then call it again later to
Expand Down Expand Up @@ -618,8 +622,11 @@ static Artifact createDexArchiveAction(
.useDefaultShellEnvironment()
.setExecutable(ruleContext.getExecutablePrerequisite(dexbuilderPrereq))
.setExecutionInfo(
TargetUtils.getExecutionInfo(
ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
createDexingDesugaringExecRequirements(ruleContext)
.putAll(
TargetUtils.getExecutionInfo(
ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
.buildKeepingLast())
// WorkerSpawnStrategy expects the last argument to be @paramfile
.addInput(jar)
.addOutput(dexArchive)
Expand Down Expand Up @@ -648,9 +655,6 @@ static Artifact createDexArchiveAction(
dexbuilder
.addCommandLine(args.build(), ParamFileInfo.builder(UNQUOTED).setUseAlways(true).build())
.stripOutputPaths(stripOutputPaths);
if (getAndroidConfig(ruleContext).useWorkersWithDexbuilder()) {
dexbuilder.setExecutionInfo(ExecutionRequirements.WORKER_MODE_ENABLED);
}
ruleContext.registerAction(dexbuilder.build(ruleContext));
return dexArchive;
}
Expand All @@ -660,6 +664,21 @@ private static Set<Set<String>> aspectDexopts(RuleContext ruleContext) {
normalizeDexopts(getAndroidConfig(ruleContext).getDexoptsSupportedInIncrementalDexing()));
}

/** Creates the execution requires for the DexBuilder and Desugar actions */
private static ImmutableMap.Builder<String, String> createDexingDesugaringExecRequirements(
RuleContext ruleContext) {
final ImmutableMap.Builder<String, String> executionInfo = ImmutableMap.builder();
AndroidConfiguration androidConfiguration = getAndroidConfig(ruleContext);
if (androidConfiguration.persistentDexDesugar()) {
executionInfo.putAll(ExecutionRequirements.WORKER_MODE_ENABLED);
if (androidConfiguration.persistentMultiplexDexDesugar()) {
executionInfo.putAll(ExecutionRequirements.WORKER_MULTIPLEX_MODE_ENABLED);
}
}

return executionInfo;
}

/**
* Derives options to use in incremental dexing actions from the given context and dx flags, where
* the latter typically come from a {@code dexopts} attribute on a top-level target. This method
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,6 @@ public interface AndroidConfigurationApi extends StarlarkValue {
documented = false)
ImmutableList<String> getTargetDexoptsThatPreventIncrementalDexing();

@StarlarkMethod(
name = "use_workers_with_dexbuilder",
structField = true,
doc = "",
documented = false)
boolean useWorkersWithDexbuilder();

@StarlarkMethod(name = "desugar_java8", structField = true, doc = "", documented = false)
boolean desugarJava8();

Expand Down Expand Up @@ -238,6 +231,20 @@ public interface AndroidConfigurationApi extends StarlarkValue {
documented = false)
boolean persistentMultiplexBusyboxTools();

@StarlarkMethod(
name = "persistent_android_dex_desugar",
structField = true,
doc = "",
documented = false)
boolean persistentDexDesugar();

@StarlarkMethod(
name = "persistent_multiplex_android_dex_desugar",
structField = true,
doc = "",
documented = false)
boolean persistentMultiplexDexDesugar();

@StarlarkMethod(
name = "get_output_directory_name",
structField = true,
Expand Down
22 changes: 18 additions & 4 deletions src/test/shell/bazel/android/desugarer_integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,24 @@ function test_java_8_android_binary_worker_strategy() {
setup_android_sdk_support
create_java_8_android_binary

bazel build \
--strategy=Desugar=worker \
--desugar_for_android //java/bazel:bin \
|| fail "build failed"
assert_build //java/bazel:bin \
--persistent_android_dex_desugar \
--worker_verbose &> $TEST_log
expect_log "Created new non-sandboxed Desugar worker (id [0-9]\+)"
expect_log "Created new non-sandboxed DexBuilder worker (id [0-9]\+)"
}

function test_java_8_android_binary_multiplex_worker_strategy() {
create_new_workspace
setup_android_sdk_support
create_java_8_android_binary

assert_build //java/bazel:bin \
--experimental_worker_multiplex \
--persistent_multiplex_android_dex_desugar \
--worker_verbose &> $TEST_log
expect_log "Created new non-sandboxed Desugar multiplex-worker (id [0-9]\+)"
expect_log "Created new non-sandboxed DexBuilder multiplex-worker (id [0-9]\+)"
}

run_suite "Android desugarer integration tests"
2 changes: 0 additions & 2 deletions tools/android/BUILD.tools
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ java_binary(
runtime_deps = ["//src/tools/android/java/com/google/devtools/build/android/r8"],
)

# NOTE: d8 dex builder doesn't support the persistent worker mode at the moment. To use this config,
# without a build error, --nouse_workers_with_dexbuilder flag must also be specified.
config_setting(
name = "d8_incremental_dexing",
values = {
Expand Down

0 comments on commit 76ad4a9

Please sign in to comment.