Reduce AndroidAssetMerger intermediate outputs

jongerrish · copybara-github · commit b76cc3a2ec49 · 2022-07-28T13:32:12.000-07:00
They are not required in the final build. For our builds:- This results in 503MB of outputs (assets.zip files) - each of these includes the transitive closure of all assets. To put in perspective we have only 2.5MB of assets. This is very costly for remote builds with caches especially on slower connections If resource conflict checking is not enabled (it is not for our build) this will also avoid running AndroidAssetMerger all together saving 896 actions ~3mins total CPU time. bazelbuild/rules_android#10 Closes #11303. PiperOrigin-RevId: 463912957 Change-Id: I61b0cdd03ce5bdfd70f17feaf03529a683cddefc
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java
@@ -180,7 +180,15 @@ ParsedAndroidAssets parse(AndroidDataContext dataContext) throws InterruptedExce
   /** Convenience method to do all of asset processing - parsing and merging. */
   public MergedAndroidAssets process(AndroidDataContext dataContext, AssetDependencies assetDeps)
       throws InterruptedException {
-    return parse(dataContext).merge(dataContext, assetDeps);
+    ParsedAndroidAssets parsedAssets = parse(dataContext);
+
+    boolean mergeAssets =
+        dataContext.getAndroidConfig().outputLibraryMergedAssets()
+            || dataContext.throwOnResourceConflict();
+
+    return mergeAssets
+        ? parsedAssets.merge(dataContext, assetDeps)
+        : MergedAndroidAssets.of(parsedAssets, null, assetDeps);
   }
 
   @Override
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
@@ -936,6 +936,14 @@ public static class Options extends FragmentOptions {
         help = "Use R.txt from the merging action, instead of from the validation action.")
     public boolean useRTxtFromMergedResources;
 
+    @Option(
+        name = "output_library_merged_assets",
+        defaultValue = "true",
+        documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+        effectTags = {OptionEffectTag.UNKNOWN},
+        help = "If disabled, does not produce merged asset.zip outputs for library targets")
+    public boolean outputLibraryMergedAssets;
+
     @Option(
         name = "legacy_main_dex_list_generator",
         // TODO(b/147692286): Update this default value to R8's GenerateMainDexList binary after
@@ -1072,6 +1080,7 @@ public FragmentOptions getHost() {
   private final boolean alwaysFilterDuplicateClassesFromAndroidTest;
   private final boolean filterLibraryJarWithProgramJar;
   private final boolean useRTxtFromMergedResources;
+  private final boolean outputLibraryMergedAssets;
   private final Label legacyMainDexListGenerator;
   private final boolean disableInstrumentationManifestMerging;
   private final boolean incompatibleUseToolchainResolution;
@@ -1135,6 +1144,7 @@ public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurati
         options.alwaysFilterDuplicateClassesFromAndroidTest;
     this.filterLibraryJarWithProgramJar = options.filterLibraryJarWithProgramJar;
     this.useRTxtFromMergedResources = options.useRTxtFromMergedResources;
+    this.outputLibraryMergedAssets = options.outputLibraryMergedAssets;
     this.legacyMainDexListGenerator = options.legacyMainDexListGenerator;
     this.disableInstrumentationManifestMerging = options.disableInstrumentationManifestMerging;
     this.incompatibleUseToolchainResolution = options.incompatibleUseToolchainResolution;
@@ -1431,6 +1441,10 @@ public boolean includeProguardLocationReferences() {
     return includeProguardLocationReferences;
   }
 
+  boolean outputLibraryMergedAssets() {
+    return outputLibraryMergedAssets;
+  }
+
   /** Returns the label provided with --legacy_main_dex_list_generator, if any. */
   // TODO(b/147692286): Move R8's main dex list tool into tool repository.
   @StarlarkConfigurationField(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidAssets.java b/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidAssets.java
@@ -28,15 +28,17 @@ static MergedAndroidAssets mergeFrom(
       throws InterruptedException {
 
     Artifact mergedAssets =
-        dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_ASSETS_ZIP);
+        dataContext.getAndroidConfig().outputLibraryMergedAssets()
+            ? dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_ASSETS_ZIP)
+            : null;
 
     BusyBoxActionBuilder builder = BusyBoxActionBuilder.create(dataContext, "MERGE_ASSETS");
     if (dataContext.throwOnResourceConflict()) {
       builder.addFlag("--throwOnAssetConflict");
     }
 
     builder
-        .addOutput("--assetsOutput", mergedAssets)
+        .maybeAddOutput("--assetsOutput", mergedAssets)
         .addInput(
             "--primaryData",
             AndroidDataConverter.PARSED_ASSET_CONVERTER.map(parsed),
diff --git a/src/test/shell/bazel/android/android_integration_test.sh b/src/test/shell/bazel/android/android_integration_test.sh
@@ -343,4 +343,61 @@ EOF
   assert_multiple_dex_files
 }
 
+function test_reduce_merged_assets() {
+  write_hello_android_files
+  setup_android_sdk_support
+  mkdir -p java/com/example/hello/assets
+  echo hello > java/com/example/hello/assets/hello.txt
+  cat > java/com/example/hello/BUILD <<'EOF'
+android_binary(
+    name = "hello",
+    manifest = "AndroidManifest.xml",
+    deps = [":hello_lib"],
+)
+android_library(
+    name = "hello_lib",
+    manifest = "AndroidManifest.xml",
+    srcs = glob(["*.java"]),
+    resource_files = glob(["res/**"]),
+    assets_dir = "assets",
+    assets = glob(["assets/**"]),
+)
+EOF
+  # The standard behavior is to output the merged assets as assets.zip
+  bazel build //java/com/example/hello:hello_lib --output_library_merged_assets || fail "build failed"
+  local tmpdir=$(mktemp -d)
+  # Expect the assets.zip to exist.
+  unzip bazel-bin/java/com/example/hello/hello_lib_files/assets.zip -d $tmpdir
+  # Expect that hello.txt contains hello.
+  assert_contains "hello" $tmpdir/assets/hello.txt
+  rm -rf $tmpdir
+}
+
+function test_dont_reduce_merged_assets() {
+  write_hello_android_files
+  setup_android_sdk_support
+  mkdir -p java/com/example/hello/assets
+  echo hello > java/com/example/hello/assets/hello.txt
+  cat > java/com/example/hello/BUILD <<'EOF'
+android_binary(
+    name = "hello",
+    manifest = "AndroidManifest.xml",
+    deps = [":hello_lib"],
+)
+android_library(
+    name = "hello_lib",
+    manifest = "AndroidManifest.xml",
+    srcs = glob(["*.java"]),
+    resource_files = glob(["res/**"]),
+    assets_dir = "assets",
+    assets = glob(["assets/**"]),
+)
+EOF
+  bazel build //java/com/example/hello:hello --nooutput_library_merged_assets || fail "build failed"
+  # Expect assets.zip to NOT exist.
+  if [[ -f bazel-bin/java/com/example/hello/hello_lib_files/assets.zip ]]; then
+    fail "assets.zip should NOT exist!"
+  fi
+}
+
 run_suite "Android integration tests"
diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidAssetMergingAction.java b/src/tools/android/java/com/google/devtools/build/android/AndroidAssetMergingAction.java
@@ -13,9 +13,9 @@
 // limitations under the License.
 package com.google.devtools.build.android;
 
-import com.android.builder.core.VariantType;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
+import com.google.devtools.build.android.AndroidDataMerger.ContentComparingChecker;
 import com.google.devtools.build.android.Converters.PathConverter;
 import com.google.devtools.build.android.Converters.SerializedAndroidDataConverter;
 import com.google.devtools.build.android.Converters.SerializedAndroidDataListConverter;
@@ -133,29 +133,46 @@ void run(Path tmp, ExecutorServiceCloser executorService) throws Exception {
 
     Preconditions.checkNotNull(options.primary);
 
-    MergedAndroidData mergedData =
-        AndroidResourceMerger.mergeDataAndWrite(
-            options.primary,
-            /* primaryManifest = */ null,
-            options.directData,
+    final ParsedAndroidData.Builder primaryBuilder = ParsedAndroidData.Builder.newBuilder();
+    final AndroidParsedDataDeserializer deserializer = AndroidParsedDataDeserializer.create();
+    options.primary.deserialize(
+        DependencyInfo.DependencyType.PRIMARY, deserializer, primaryBuilder.consumers());
+    ParsedAndroidData primaryData = primaryBuilder.build();
+
+    UnwrittenMergedAndroidData unwrittenMergedData =
+        AndroidResourceMerger.mergeData(
+            executorService,
             options.transitiveData,
-            /* resourcesOut = */ ignored,
-            mergedAssets,
-            VariantType.LIBRARY,
-            /* symbolsOut = */ null,
-            /* rclassWriter = */ null,
+            options.directData,
+            primaryData,
+            /* primaryManifest = */ null,
+            /* allowPrimaryOverrideAll = */ false,
+            deserializer,
             options.throwOnAssetConflict,
-            executorService);
+            ContentComparingChecker.create());
 
     logCompletion("Merging");
 
-    Preconditions.checkState(
-        !Files.exists(ignored),
-        "The asset merging action should not produce non-asset merge results!");
-
-    ResourcesZip.from(ignored, mergedData.getAssetDir())
-        .writeTo(options.assetsOutput, /* compress= */ true);
-    logCompletion("Create assets zip");
+    if (options.assetsOutput != null) {
+      MergedAndroidData writtenMergedData =
+          AndroidResourceMerger.writeMergedData(
+              ignored,
+              mergedAssets,
+              /* symbolsOut = */ null,
+              /* rclassWriter = */ null,
+              executorService,
+              unwrittenMergedData);
+
+      logCompletion("Writing");
+
+      Preconditions.checkState(
+          !Files.exists(ignored),
+          "The asset merging action should not produce non-asset merge results!");
+
+      ResourcesZip.from(ignored, writtenMergedData.getAssetDir())
+          .writeTo(options.assetsOutput, /* compress= */ true);
+      logCompletion("Create assets zip");
+    }
   }
 
   @Override
diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceMerger.java b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceMerger.java
@@ -97,87 +97,37 @@ public static MergedAndroidData mergeDataAndWrite(
       boolean throwOnResourceConflict) {
     try (ExecutorServiceCloser executorService = ExecutorServiceCloser.createWithFixedPoolOf(15)) {
       final ParsedAndroidData parsedPrimary = ParsedAndroidData.from(primary);
-      return mergeDataAndWrite(
-          parsedPrimary,
-          primary.getManifest(),
-          direct,
-          transitive,
+      return writeMergedData(
           resourcesOut,
           assetsOut,
-          type,
           symbolsOut,
           /* rclassWriter= */ null,
-          AndroidParsedDataDeserializer.withFilteredResources(filteredResources),
-          throwOnResourceConflict,
-          executorService);
+          executorService,
+          mergeData(
+              executorService,
+              transitive,
+              direct,
+              parsedPrimary,
+              primary.getManifest(),
+              type != VariantType.LIBRARY,
+              AndroidParsedDataDeserializer.withFilteredResources(filteredResources),
+              throwOnResourceConflict,
+              ContentComparingChecker.create()));
     } catch (IOException e) {
       throw MergingException.wrapException(e);
     }
   }
 
-  /**
-   * Merges all secondary resources with the primary resources, given that the primary resources
-   * have been separately parsed and serialized.
-   */
-  public static MergedAndroidData mergeDataAndWrite(
-      final SerializedAndroidData primary,
-      final Path primaryManifest,
-      final List<? extends SerializedAndroidData> direct,
-      final List<? extends SerializedAndroidData> transitive,
+  /** Writes out merged data. */
+  static MergedAndroidData writeMergedData(
       final Path resourcesOut,
       final Path assetsOut,
-      final VariantType type,
-      @Nullable final Path symbolsOut,
-      @Nullable final AndroidResourceClassWriter rclassWriter,
-      boolean throwOnResourceConflict,
-      ListeningExecutorService executorService) {
-    final ParsedAndroidData.Builder primaryBuilder = ParsedAndroidData.Builder.newBuilder();
-    final AndroidParsedDataDeserializer deserializer = AndroidParsedDataDeserializer.create();
-    primary.deserialize(
-        DependencyInfo.DependencyType.PRIMARY, deserializer, primaryBuilder.consumers());
-    ParsedAndroidData primaryData = primaryBuilder.build();
-    return mergeDataAndWrite(
-        primaryData,
-        primaryManifest,
-        direct,
-        transitive,
-        resourcesOut,
-        assetsOut,
-        type,
-        symbolsOut,
-        rclassWriter,
-        deserializer,
-        throwOnResourceConflict,
-        executorService);
-  }
-
-  /** Merges all secondary resources with the primary resources. */
-  private static MergedAndroidData mergeDataAndWrite(
-      final ParsedAndroidData primary,
-      final Path primaryManifest,
-      final List<? extends SerializedAndroidData> direct,
-      final List<? extends SerializedAndroidData> transitive,
-      final Path resourcesOut,
-      final Path assetsOut,
-      final VariantType type,
       @Nullable final Path symbolsOut,
       @Nullable AndroidResourceClassWriter rclassWriter,
-      AndroidParsedDataDeserializer deserializer,
-      boolean throwOnResourceConflict,
-      ListeningExecutorService executorService) {
+      ListeningExecutorService executorService,
+      UnwrittenMergedAndroidData merged) {
     Stopwatch timer = Stopwatch.createStarted();
     try {
-      UnwrittenMergedAndroidData merged =
-          mergeData(
-              executorService,
-              transitive,
-              direct,
-              primary,
-              primaryManifest,
-              type != VariantType.LIBRARY,
-              deserializer,
-              throwOnResourceConflict,
-              ContentComparingChecker.create());
       timer.reset().start();
       if (symbolsOut != null) {
         AndroidDataSerializer serializer = AndroidDataSerializer.create();
@@ -206,7 +156,7 @@ private static MergedAndroidData mergeDataAndWrite(
     }
   }
 
-  private static UnwrittenMergedAndroidData mergeData(
+  static UnwrittenMergedAndroidData mergeData(
       ListeningExecutorService executorService,
       List<? extends SerializedAndroidData> transitive,
       List<? extends SerializedAndroidData> direct,
