From 74bc04653cd8635f19a887bd1171d2e8566166c1 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 17 Aug 2023 13:39:43 -0700 Subject: [PATCH] support persistent worker for aar extractors This PR supports persistent worker for Android aar extractors, it'll significantly improve the aar extractors performance especially in the first time compilation without cache. It's not enable by default, can use flag `build --experimental_persistent_aar_extractor=true` to enable it. using worker: ![image](https://github.com/bazelbuild/bazel/assets/10076965/804f8167-4a52-4321-ba4a-9d1f1dd57644) not using worker: ![image](https://github.com/bazelbuild/bazel/assets/10076965/26ef4047-4ff7-45f3-9767-ce48418aa089) Closes #18496. PiperOrigin-RevId: 557923249 Change-Id: Ica6b6e0a6e3f1dcb68bb9aba941d538888ca013d --- .../build/lib/rules/android/AarImport.java | 95 ++++++++++++++----- .../rules/android/AndroidConfiguration.java | 18 ++++ .../android/AndroidConfigurationApi.java | 7 ++ .../bazel/android/aar_integration_test.sh | 34 +++++++ tools/android/BUILD | 11 +++ tools/android/BUILD.tools | 11 +++ tools/android/aar_embedded_jars_extractor.py | 5 +- .../aar_embedded_proguard_extractor.py | 5 +- tools/android/aar_native_libs_zip_creator.py | 4 +- tools/android/aar_resources_extractor.py | 5 +- tools/android/json_worker_wrapper.py | 59 ++++++++++++ 11 files changed, 219 insertions(+), 35 deletions(-) create mode 100644 tools/android/json_worker_wrapper.py diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java index c160a981aa9fe7..4f302f60cfa23b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java @@ -14,10 +14,14 @@ package com.google.devtools.build.lib.rules.android; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; +import com.google.devtools.build.lib.actions.ExecutionRequirements; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; +import com.google.devtools.build.lib.actions.ParamFileInfo; +import com.google.devtools.build.lib.actions.ParameterFile; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FileProvider; import com.google.devtools.build.lib.analysis.FilesToRunProvider; @@ -49,6 +53,7 @@ import com.google.devtools.build.lib.rules.java.ProguardSpecProvider; import com.google.devtools.build.lib.starlarkbuildapi.android.DataBindingV2ProviderApi; import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.List; import javax.annotation.Nullable; /** @@ -65,6 +70,9 @@ public class AarImport implements RuleConfiguredTargetFactory { private static final String MERGED_JAR = "classes_and_libs_merged.jar"; private static final String PROGUARD_SPEC = "proguard.txt"; + private static final ParamFileInfo WORKERS_FORCED_PARAM_FILE_INFO = + ParamFileInfo.builder(ParameterFile.ParameterFileType.UNQUOTED).setUseAlways(true).build(); + private final JavaSemantics javaSemantics; private final AndroidSemantics androidSemantics; @@ -272,13 +280,41 @@ private NestedSet extractProguardSpecs(RuleContext ruleContext, Artifa return builder.addTransitive(proguardSpecs).add(proguardSpecArtifact).build(); } + private static boolean isPersistentAarExtractor(RuleContext ruleContext) { + AndroidConfiguration androidConfig = + ruleContext.getConfiguration().getFragment(AndroidConfiguration.class); + return androidConfig.persistentAarExtractor(); + } + + /** + * Returns either the worker param file or null depending on whether persistent worker mode is on. + */ + @Nullable + private static ParamFileInfo getParamFileInfo(RuleContext ruleContext) { + return isPersistentAarExtractor(ruleContext) ? WORKERS_FORCED_PARAM_FILE_INFO : null; + } + + /** Adds worker-related execution requirements if worker mode enabled. */ + private static void modifyExecutionInfo( + RuleContext ruleContext, SpawnAction.Builder actionBuilder) { + if (isPersistentAarExtractor(ruleContext)) { + ImmutableMap.Builder executionInfo = ImmutableMap.builder(); + executionInfo.putAll(ExecutionRequirements.WORKER_MODE_ENABLED); + executionInfo.put(ExecutionRequirements.REQUIRES_WORKER_PROTOCOL, "json"); + actionBuilder.setExecutionInfo(executionInfo.buildOrThrow()); + } + } + /** * Creates action to extract embedded Proguard.txt from an AAR. If the file is not found, an empty * file will be created */ private static SpawnAction createAarEmbeddedProguardExtractorActions( RuleContext ruleContext, Artifact aar, Artifact proguardSpecArtifact) { - return new SpawnAction.Builder() + SpawnAction.Builder actionBuilder = new SpawnAction.Builder(); + ParamFileInfo paramFileInfo = getParamFileInfo(ruleContext); + modifyExecutionInfo(ruleContext, actionBuilder); + return actionBuilder .useDefaultShellEnvironment() .setExecutable( ruleContext.getExecutablePrerequisite(AarImportBaseRule.AAR_EMBEDDED_PROGUARD_EXTACTOR)) @@ -290,7 +326,8 @@ private static SpawnAction createAarEmbeddedProguardExtractorActions( CustomCommandLine.builder() .addExecPath("--input_aar", aar) .addExecPath("--output_proguard_file", proguardSpecArtifact) - .build()) + .build(), + paramFileInfo) .build(ruleContext); } @@ -352,7 +389,10 @@ private static SpawnAction createAarResourcesExtractorActions( Artifact databindingBrFiles, Artifact databindingSetterStoreFiles) { - return new SpawnAction.Builder() + SpawnAction.Builder actionBuilder = new SpawnAction.Builder(); + ParamFileInfo paramFileInfo = getParamFileInfo(ruleContext); + modifyExecutionInfo(ruleContext, actionBuilder); + return actionBuilder .useDefaultShellEnvironment() .setExecutable( ruleContext.getExecutablePrerequisite(AarImportBaseRule.AAR_RESOURCES_EXTRACTOR)) @@ -370,7 +410,8 @@ private static SpawnAction createAarResourcesExtractorActions( .addExecPath("--output_assets_dir", assetsDir) .addExecPath("--output_databinding_br_dir", databindingBrFiles) .addExecPath("--output_databinding_setter_store_dir", databindingSetterStoreFiles) - .build()) + .build(), + paramFileInfo) .build(ruleContext); } @@ -379,7 +420,10 @@ private static SpawnAction createAarEmbeddedJarsExtractorActions( Artifact aar, Artifact jarsTreeArtifact, Artifact singleJarParamFile) { - return new SpawnAction.Builder() + SpawnAction.Builder actionBuilder = new SpawnAction.Builder(); + ParamFileInfo paramFileInfo = getParamFileInfo(ruleContext); + modifyExecutionInfo(ruleContext, actionBuilder); + return actionBuilder .useDefaultShellEnvironment() .setExecutable( ruleContext.getExecutablePrerequisite(AarImportBaseRule.AAR_EMBEDDED_JARS_EXTACTOR)) @@ -393,7 +437,8 @@ private static SpawnAction createAarEmbeddedJarsExtractorActions( .addExecPath("--input_aar", aar) .addExecPath("--output_dir", jarsTreeArtifact) .addExecPath("--output_singlejar_param_file", singleJarParamFile) - .build()) + .build(), + paramFileInfo) .build(ruleContext); } @@ -420,23 +465,25 @@ private static SpawnAction createAarJarsMergingActions( private static SpawnAction createAarNativeLibsFilterActions( RuleContext ruleContext, Artifact aar, Artifact outputZip) { - SpawnAction.Builder actionBuilder = - new SpawnAction.Builder() - .useDefaultShellEnvironment() - .setExecutable( - ruleContext.getExecutablePrerequisite( - AarImportBaseRule.AAR_NATIVE_LIBS_ZIP_CREATOR)) - .setMnemonic("AarNativeLibsFilter") - .setProgressMessage("Filtering AAR native libs by architecture for %{label}") - .addInput(aar) - .addOutput(outputZip) - .addCommandLine( - CustomCommandLine.builder() - .addExecPath("--input_aar", aar) - .add("--cpu", ruleContext.getConfiguration().getCpu()) - .addExecPath("--output_zip", outputZip) - .build()); - return actionBuilder.build(ruleContext); + SpawnAction.Builder actionBuilder = new SpawnAction.Builder(); + ParamFileInfo paramFileInfo = getParamFileInfo(ruleContext); + modifyExecutionInfo(ruleContext, actionBuilder); + return actionBuilder + .useDefaultShellEnvironment() + .setExecutable( + ruleContext.getExecutablePrerequisite(AarImportBaseRule.AAR_NATIVE_LIBS_ZIP_CREATOR)) + .setMnemonic("AarNativeLibsFilter") + .setProgressMessage("Filtering AAR native libs by architecture for %{label}") + .addInput(aar) + .addOutput(outputZip) + .addCommandLine( + CustomCommandLine.builder() + .addExecPath("--input_aar", aar) + .add("--cpu", ruleContext.getConfiguration().getCpu()) + .addExecPath("--output_zip", outputZip) + .build(), + paramFileInfo) + .build(ruleContext); } private static DataBindingV2Provider createDatabindingProvider( @@ -444,7 +491,7 @@ private static DataBindingV2Provider createDatabindingProvider( SpecialArtifact databindingBrFiles, SpecialArtifact databindingSetterStoreFiles) { - Iterable> databindingProvidersFromDeps = + List> databindingProvidersFromDeps = ruleContext.getPrerequisites("deps", DataBindingV2Provider.PROVIDER); Iterable> databindingProvidersFromExports = diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java index 896e59e5b6fd98..e8a1d3cbf8fdfd 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java @@ -825,6 +825,16 @@ public static class Options extends FragmentOptions { + "transitive classpath, otherwise it uses the deploy jar") public boolean oneVersionEnforcementUseTransitiveJarsForBinaryUnderTest; + @Option( + name = "experimental_persistent_aar_extractor", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, + effectTags = { + OptionEffectTag.EXECUTION, + }, + help = "Enable persistent aar extractor by using workers.") + public boolean persistentAarExtractor; + @Option( name = "persistent_android_resource_processor", defaultValue = "null", @@ -1132,6 +1142,7 @@ public FragmentOptions getExec() { exec.manifestMergerOrder = manifestMergerOrder; exec.oneVersionEnforcementUseTransitiveJarsForBinaryUnderTest = oneVersionEnforcementUseTransitiveJarsForBinaryUnderTest; + exec.persistentAarExtractor = persistentAarExtractor; exec.persistentBusyboxTools = persistentBusyboxTools; exec.persistentMultiplexBusyboxTools = persistentMultiplexBusyboxTools; exec.disableNativeAndroidRules = disableNativeAndroidRules; @@ -1177,6 +1188,7 @@ public FragmentOptions getExec() { private final boolean dataBindingV2; private final boolean dataBindingUpdatedArgs; private final boolean dataBindingAndroidX; + private final boolean persistentAarExtractor; private final boolean persistentBusyboxTools; private final boolean persistentMultiplexBusyboxTools; private final boolean persistentDexDesugar; @@ -1239,6 +1251,7 @@ public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurati this.dataBindingV2 = options.dataBindingV2; this.dataBindingUpdatedArgs = options.dataBindingUpdatedArgs; this.dataBindingAndroidX = options.dataBindingAndroidX; + this.persistentAarExtractor = options.persistentAarExtractor; this.persistentBusyboxTools = options.persistentBusyboxTools; this.persistentMultiplexBusyboxTools = options.persistentMultiplexBusyboxTools; this.persistentDexDesugar = options.persistentDexDesugar; @@ -1481,6 +1494,11 @@ public boolean useDataBindingAndroidX() { return dataBindingAndroidX; } + @Override + public boolean persistentAarExtractor() { + return persistentAarExtractor; + } + @Override public boolean persistentBusyboxTools() { return persistentBusyboxTools; diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidConfigurationApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidConfigurationApi.java index 159822b31f51bc..be9248a01ec421 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidConfigurationApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidConfigurationApi.java @@ -231,6 +231,13 @@ public interface AndroidConfigurationApi extends StarlarkValue { documented = false) boolean useDataBindingAndroidX(); + @StarlarkMethod( + name = "persistent_aar_extractor", + structField = true, + doc = "", + documented = false) + boolean persistentAarExtractor(); + @StarlarkMethod( name = "persistent_busybox_tools", structField = true, diff --git a/src/test/shell/bazel/android/aar_integration_test.sh b/src/test/shell/bazel/android/aar_integration_test.sh index 283e1311e58740..31025bb9fd022e 100755 --- a/src/test/shell/bazel/android/aar_integration_test.sh +++ b/src/test/shell/bazel/android/aar_integration_test.sh @@ -147,4 +147,38 @@ EOF assert_one_of $apk_contents "lib/armeabi-v7a/libapp.so" } +function test_aar_extractor_worker() { + create_new_workspace + setup_android_sdk_support + cat > AndroidManifest.xml < +EOF + mkdir -p res/layout + cat > res/layout/mylayout.xml < + +EOF + mkdir assets + echo "some asset" > assets/a + zip example.aar AndroidManifest.xml res/layout/mylayout.xml assets/a + cat > BUILD <