Skip to content

Commit 76ad4a9

Browse files
BencodesBencodes
authored
[6.2.0]Fix worker and multiplex workers for DexBuilder and Desugar actions (#17965)
1 parent 97312f3 commit 76ad4a9

File tree

6 files changed

+114
-40
lines changed

6 files changed

+114
-40
lines changed

‎src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,15 @@ public static class BuildGraveyardOptions extends OptionsBase {
273273
help = "Deprecated no-op. Use --experimental_dynamic_local_load_factor instead.")
274274
@Deprecated
275275
public boolean skipFirstBuild;
276+
277+
@Option(
278+
name = "use_workers_with_dexbuilder",
279+
defaultValue = "true",
280+
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
281+
effectTags = {OptionEffectTag.EXECUTION},
282+
help = "This option is deprecated and has no effect.")
283+
@Deprecated
284+
public boolean useWorkersWithDexbuilder;
276285
}
277286

278287
/** This is where deprecated Bazel-specific options only used by the build command go to die. */

‎src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java‎

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -533,14 +533,6 @@ public static class Options extends FragmentOptions {
533533
help = "dx flags supported in tool that groups classes for inclusion in final .dex files.")
534534
public List<String> dexoptsSupportedInDexSharder;
535535

536-
@Option(
537-
name = "use_workers_with_dexbuilder",
538-
defaultValue = "true",
539-
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
540-
effectTags = {OptionEffectTag.EXECUTION},
541-
help = "Whether dexbuilder supports being run in local worker mode.")
542-
public boolean useWorkersWithDexbuilder;
543-
544536
@Option(
545537
name = "experimental_android_rewrite_dexes_with_rex",
546538
defaultValue = "false",
@@ -905,10 +897,11 @@ public static class Options extends FragmentOptions {
905897
},
906898
help = "Enable persistent Android dex and desugar actions by using workers.",
907899
expansion = {
900+
"--internal_persistent_android_dex_desugar",
908901
"--strategy=Desugar=worker",
909902
"--strategy=DexBuilder=worker",
910903
})
911-
public Void persistentDexDesugar;
904+
public Void persistentAndroidDexDesugar;
912905

913906
@Option(
914907
name = "persistent_multiplex_android_dex_desugar",
@@ -921,10 +914,9 @@ public static class Options extends FragmentOptions {
921914
help = "Enable persistent multiplexed Android dex and desugar actions by using workers.",
922915
expansion = {
923916
"--persistent_android_dex_desugar",
924-
"--modify_execution_info=Desugar=+supports-multiplex-workers",
925-
"--modify_execution_info=DexBuilder=+supports-multiplex-workers",
917+
"--internal_persistent_multiplex_android_dex_desugar",
926918
})
927-
public Void persistentMultiplexDexDesugar;
919+
public Void persistentMultiplexAndroidDexDesugar;
928920

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

969+
/**
970+
* We use this option to decide when to enable workers for busybox tools. This flag is also a
971+
* guard against enabling workers using nothing but --persistent_android_resource_processor.
972+
*
973+
* <p>Consequently, we use this option to decide between param files or regular command line
974+
* parameters. If we're not using workers or on Windows, there's no need to always use param
975+
* files for I/O performance reasons.
976+
*/
977+
@Option(
978+
name = "internal_persistent_android_dex_desugar",
979+
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
980+
effectTags = {
981+
OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS,
982+
OptionEffectTag.EXECUTION,
983+
},
984+
defaultValue = "false",
985+
help = "Tracking flag for when dexing and desugaring workers are enabled.")
986+
public boolean persistentDexDesugar;
987+
988+
@Option(
989+
name = "internal_persistent_multiplex_android_dex_desugar",
990+
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
991+
effectTags = {
992+
OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS,
993+
OptionEffectTag.EXECUTION,
994+
},
995+
defaultValue = "false",
996+
help = "Tracking flag for when multiplexed dexing and desugaring workers are enabled.")
997+
public boolean persistentMultiplexDexDesugar;
998+
977999
@Option(
9781000
name = "experimental_remove_r_classes_from_instrumentation_test_jar",
9791001
defaultValue = "true",
@@ -1100,7 +1122,6 @@ public FragmentOptions getHost() {
11001122
host.dexoptsSupportedInIncrementalDexing = dexoptsSupportedInIncrementalDexing;
11011123
host.dexoptsSupportedInDexMerger = dexoptsSupportedInDexMerger;
11021124
host.dexoptsSupportedInDexSharder = dexoptsSupportedInDexSharder;
1103-
host.useWorkersWithDexbuilder = useWorkersWithDexbuilder;
11041125
host.manifestMerger = manifestMerger;
11051126
host.manifestMergerOrder = manifestMergerOrder;
11061127
host.allowAndroidLibraryDepsWithoutSrcs = allowAndroidLibraryDepsWithoutSrcs;
@@ -1128,7 +1149,6 @@ public FragmentOptions getHost() {
11281149
private final ImmutableList<String> targetDexoptsThatPreventIncrementalDexing;
11291150
private final ImmutableList<String> dexoptsSupportedInDexMerger;
11301151
private final ImmutableList<String> dexoptsSupportedInDexSharder;
1131-
private final boolean useWorkersWithDexbuilder;
11321152
private final boolean desugarJava8;
11331153
private final boolean desugarJava8Libs;
11341154
private final boolean checkDesugarDeps;
@@ -1155,6 +1175,8 @@ public FragmentOptions getHost() {
11551175
private final boolean dataBindingAndroidX;
11561176
private final boolean persistentBusyboxTools;
11571177
private final boolean persistentMultiplexBusyboxTools;
1178+
private final boolean persistentDexDesugar;
1179+
private final boolean persistentMultiplexDexDesugar;
11581180
private final boolean filterRJarsFromAndroidTest;
11591181
private final boolean removeRClassesFromInstrumentationTestJar;
11601182
private final boolean alwaysFilterDuplicateClassesFromAndroidTest;
@@ -1184,7 +1206,6 @@ public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurati
11841206
ImmutableList.copyOf(options.nonIncrementalPerTargetDexopts);
11851207
this.dexoptsSupportedInDexMerger = ImmutableList.copyOf(options.dexoptsSupportedInDexMerger);
11861208
this.dexoptsSupportedInDexSharder = ImmutableList.copyOf(options.dexoptsSupportedInDexSharder);
1187-
this.useWorkersWithDexbuilder = options.useWorkersWithDexbuilder;
11881209
this.desugarJava8 = options.desugarJava8;
11891210
this.desugarJava8Libs = options.desugarJava8Libs;
11901211
this.checkDesugarDeps = options.checkDesugarDeps;
@@ -1216,6 +1237,8 @@ public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurati
12161237
this.dataBindingAndroidX = options.dataBindingAndroidX;
12171238
this.persistentBusyboxTools = options.persistentBusyboxTools;
12181239
this.persistentMultiplexBusyboxTools = options.persistentMultiplexBusyboxTools;
1240+
this.persistentDexDesugar = options.persistentDexDesugar;
1241+
this.persistentMultiplexDexDesugar = options.persistentMultiplexDexDesugar;
12191242
this.filterRJarsFromAndroidTest = options.filterRJarsFromAndroidTest;
12201243
this.removeRClassesFromInstrumentationTestJar =
12211244
options.removeRClassesFromInstrumentationTestJar;
@@ -1319,12 +1342,6 @@ public ImmutableList<String> getTargetDexoptsThatPreventIncrementalDexing() {
13191342
return targetDexoptsThatPreventIncrementalDexing;
13201343
}
13211344

1322-
/** Whether to assume the dexbuilder tool supports local worker mode. */
1323-
@Override
1324-
public boolean useWorkersWithDexbuilder() {
1325-
return useWorkersWithDexbuilder;
1326-
}
1327-
13281345
@Override
13291346
public boolean desugarJava8() {
13301347
return desugarJava8;
@@ -1473,6 +1490,16 @@ public boolean persistentMultiplexBusyboxTools() {
14731490
return persistentMultiplexBusyboxTools;
14741491
}
14751492

1493+
@Override
1494+
public boolean persistentDexDesugar() {
1495+
return persistentDexDesugar;
1496+
}
1497+
1498+
@Override
1499+
public boolean persistentMultiplexDexDesugar() {
1500+
return persistentMultiplexDexDesugar;
1501+
}
1502+
14761503
@Override
14771504
public boolean incompatibleUseToolchainResolution() {
14781505
return incompatibleUseToolchainResolution;

‎src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.google.common.base.Predicate;
2929
import com.google.common.base.Predicates;
3030
import com.google.common.collect.ImmutableList;
31+
import com.google.common.collect.ImmutableMap;
3132
import com.google.common.collect.ImmutableSet;
3233
import com.google.common.collect.Iterables;
3334
import com.google.common.collect.Sets;
@@ -526,7 +527,10 @@ private static Artifact createDesugarAction(
526527
.addOutput(result)
527528
.setMnemonic("Desugar")
528529
.setProgressMessage("Desugaring %s for Android", jar.prettyPrint())
529-
.setExecutionInfo(ExecutionRequirements.WORKER_MODE_ENABLED);
530+
.setExecutionInfo(
531+
createDexingDesugaringExecRequirements(ruleContext)
532+
.putAll(ExecutionRequirements.WORKER_MODE_ENABLED)
533+
.buildKeepingLast());
530534

531535
// SpawnAction.Builder.build() is documented as being safe for re-use. So we can call build here
532536
// to get the action's inputs for vetting path stripping safety, then call it again later to
@@ -618,8 +622,11 @@ static Artifact createDexArchiveAction(
618622
.useDefaultShellEnvironment()
619623
.setExecutable(ruleContext.getExecutablePrerequisite(dexbuilderPrereq))
620624
.setExecutionInfo(
621-
TargetUtils.getExecutionInfo(
622-
ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
625+
createDexingDesugaringExecRequirements(ruleContext)
626+
.putAll(
627+
TargetUtils.getExecutionInfo(
628+
ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
629+
.buildKeepingLast())
623630
// WorkerSpawnStrategy expects the last argument to be @paramfile
624631
.addInput(jar)
625632
.addOutput(dexArchive)
@@ -648,9 +655,6 @@ static Artifact createDexArchiveAction(
648655
dexbuilder
649656
.addCommandLine(args.build(), ParamFileInfo.builder(UNQUOTED).setUseAlways(true).build())
650657
.stripOutputPaths(stripOutputPaths);
651-
if (getAndroidConfig(ruleContext).useWorkersWithDexbuilder()) {
652-
dexbuilder.setExecutionInfo(ExecutionRequirements.WORKER_MODE_ENABLED);
653-
}
654658
ruleContext.registerAction(dexbuilder.build(ruleContext));
655659
return dexArchive;
656660
}
@@ -660,6 +664,21 @@ private static Set<Set<String>> aspectDexopts(RuleContext ruleContext) {
660664
normalizeDexopts(getAndroidConfig(ruleContext).getDexoptsSupportedInIncrementalDexing()));
661665
}
662666

667+
/** Creates the execution requires for the DexBuilder and Desugar actions */
668+
private static ImmutableMap.Builder<String, String> createDexingDesugaringExecRequirements(
669+
RuleContext ruleContext) {
670+
final ImmutableMap.Builder<String, String> executionInfo = ImmutableMap.builder();
671+
AndroidConfiguration androidConfiguration = getAndroidConfig(ruleContext);
672+
if (androidConfiguration.persistentDexDesugar()) {
673+
executionInfo.putAll(ExecutionRequirements.WORKER_MODE_ENABLED);
674+
if (androidConfiguration.persistentMultiplexDexDesugar()) {
675+
executionInfo.putAll(ExecutionRequirements.WORKER_MULTIPLEX_MODE_ENABLED);
676+
}
677+
}
678+
679+
return executionInfo;
680+
}
681+
663682
/**
664683
* Derives options to use in incremental dexing actions from the given context and dx flags, where
665684
* the latter typically come from a {@code dexopts} attribute on a top-level target. This method

‎src/main/java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidConfigurationApi.java‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,6 @@ public interface AndroidConfigurationApi extends StarlarkValue {
9494
documented = false)
9595
ImmutableList<String> getTargetDexoptsThatPreventIncrementalDexing();
9696

97-
@StarlarkMethod(
98-
name = "use_workers_with_dexbuilder",
99-
structField = true,
100-
doc = "",
101-
documented = false)
102-
boolean useWorkersWithDexbuilder();
103-
10497
@StarlarkMethod(name = "desugar_java8", structField = true, doc = "", documented = false)
10598
boolean desugarJava8();
10699

@@ -238,6 +231,20 @@ public interface AndroidConfigurationApi extends StarlarkValue {
238231
documented = false)
239232
boolean persistentMultiplexBusyboxTools();
240233

234+
@StarlarkMethod(
235+
name = "persistent_android_dex_desugar",
236+
structField = true,
237+
doc = "",
238+
documented = false)
239+
boolean persistentDexDesugar();
240+
241+
@StarlarkMethod(
242+
name = "persistent_multiplex_android_dex_desugar",
243+
structField = true,
244+
doc = "",
245+
documented = false)
246+
boolean persistentMultiplexDexDesugar();
247+
241248
@StarlarkMethod(
242249
name = "get_output_directory_name",
243250
structField = true,

‎src/test/shell/bazel/android/desugarer_integration_test.sh‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,24 @@ function test_java_8_android_binary_worker_strategy() {
124124
setup_android_sdk_support
125125
create_java_8_android_binary
126126

127-
bazel build \
128-
--strategy=Desugar=worker \
129-
--desugar_for_android //java/bazel:bin \
130-
|| fail "build failed"
127+
assert_build //java/bazel:bin \
128+
--persistent_android_dex_desugar \
129+
--worker_verbose &> $TEST_log
130+
expect_log "Created new non-sandboxed Desugar worker (id [0-9]\+)"
131+
expect_log "Created new non-sandboxed DexBuilder worker (id [0-9]\+)"
132+
}
133+
134+
function test_java_8_android_binary_multiplex_worker_strategy() {
135+
create_new_workspace
136+
setup_android_sdk_support
137+
create_java_8_android_binary
138+
139+
assert_build //java/bazel:bin \
140+
--experimental_worker_multiplex \
141+
--persistent_multiplex_android_dex_desugar \
142+
--worker_verbose &> $TEST_log
143+
expect_log "Created new non-sandboxed Desugar multiplex-worker (id [0-9]\+)"
144+
expect_log "Created new non-sandboxed DexBuilder multiplex-worker (id [0-9]\+)"
131145
}
132146

133147
run_suite "Android desugarer integration tests"

‎tools/android/BUILD.tools‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@ java_binary(
4747
runtime_deps = ["//src/tools/android/java/com/google/devtools/build/android/r8"],
4848
)
4949

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

0 commit comments

Comments
 (0)