Use fast-path compiler for android_binary R.classes

Hook up the RClassGeneratorAction to blaze code.

This is currently only used for android_binary
targets to build the final R classes of the
binary + all of its libraries.

Behind a flag and off by default (flip on later).

--
MOS_MIGRATED_REVID=126935165

5 files changed, 58 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
index 8a332a8296..d01f525be4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
@@ -345,7 +345,8 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
         androidSemantics,
         resourceApk,
         ruleContext.getConfiguration().isCodeCoverageEnabled(),
-        true /* collectJavaCompilationArgs */);
+        true /* collectJavaCompilationArgs */,
+        true /* isBinary */);
     ruleContext.assertNoErrors();
 
     Artifact deployJar = createDeployJar(ruleContext, javaSemantics, androidCommon, resourceClasses,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
index 784d48dec3..7b807c5dc2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
@@ -368,15 +368,20 @@ public class AndroidCommon {
 
   private void compileResources(
       JavaSemantics javaSemantics,
+      ResourceApk resourceApk,
       Artifact resourcesJar,
       JavaCompilationArtifacts.Builder artifactsBuilder,
       JavaTargetAttributes.Builder attributes,
       NestedSetBuilder<Artifact> filesBuilder,
-      ImmutableList.Builder<Artifact> jarsProducedForRuntime) throws InterruptedException {
-    compileResourceJar(javaSemantics, resourcesJar);
+      ImmutableList.Builder<Artifact> jarsProducedForRuntime,
+      boolean useRClassGenerator) throws InterruptedException {
+    compileResourceJar(javaSemantics, resourceApk, resourcesJar, useRClassGenerator);
     // Add the compiled resource jar to the classpath of the main compilation.
     attributes.addDirectJars(NestedSetBuilder.create(Order.STABLE_ORDER, resourceClassJar));
     // Add the compiled resource jar to the classpath of consuming targets.
+    // We don't actually use the ijar. That is almost the same as the resource class jar
+    // except for <clinit>, but it takes time to build and waiting for that to build would
+    // just delay building the rest of the library.
     artifactsBuilder.addCompileTimeJar(resourceClassJar);
     // Combined resource constants needs to come even before our own classes that may contain
     // local resource constants.
@@ -387,7 +392,9 @@ public class AndroidCommon {
     filesBuilder.add(resourceClassJar);
   }
 
-  private void compileResourceJar(JavaSemantics javaSemantics, Artifact resourcesJar)
+  private void compileResourceJar(
+      JavaSemantics javaSemantics, ResourceApk resourceApk, Artifact resourcesJar,
+      boolean useRClassGenerator)
       throws InterruptedException {
     resourceSourceJar = ruleContext.getImplicitOutputArtifact(
         AndroidRuleClasses.ANDROID_RESOURCES_SOURCE_JAR);
@@ -399,15 +406,23 @@ public class AndroidCommon {
         .addSourceJar(resourcesJar);
     JavaCompilationHelper javacHelper = new JavaCompilationHelper(
         ruleContext, javaSemantics, getJavacOpts(), javacAttributes);
-
-    Artifact outputDepsProto =
-        javacHelper.createOutputDepsProtoArtifact(resourceClassJar, javaArtifactsBuilder);
-    javacHelper.createCompileActionWithInstrumentation(
-        resourceClassJar,
-        null /* manifestProtoOutput */,
-        null /* genSourceJar */,
-        outputDepsProto,
-        javaArtifactsBuilder);
+    if (useRClassGenerator) {
+      RClassGeneratorActionBuilder actionBuilder =
+          new RClassGeneratorActionBuilder(ruleContext)
+              .withPrimary(resourceApk.getPrimaryResource())
+              .withDependencies(resourceApk.getResourceDependencies())
+              .setClassJarOut(resourceClassJar);
+      actionBuilder.build();
+    } else {
+      Artifact outputDepsProto =
+          javacHelper.createOutputDepsProtoArtifact(resourceClassJar, javaArtifactsBuilder);
+      javacHelper.createCompileActionWithInstrumentation(
+          resourceClassJar,
+          null /* manifestProtoOutput */,
+          null /* genSourceJar */,
+          outputDepsProto,
+          javaArtifactsBuilder);
+    }
     javacHelper.createSourceJarAction(resourceSourceJar, null);
     resourceIJar = javacHelper.createCompileTimeJarAction(resourceClassJar, javaArtifactsBuilder);
   }
@@ -472,7 +487,8 @@ public class AndroidCommon {
   public JavaTargetAttributes init(
       JavaSemantics javaSemantics, AndroidSemantics androidSemantics,
       ResourceApk resourceApk,
-      boolean addCoverageSupport, boolean collectJavaCompilationArgs) throws InterruptedException {
+      boolean addCoverageSupport, boolean collectJavaCompilationArgs, boolean isBinary)
+      throws InterruptedException {
 
     classJar = ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_LIBRARY_CLASS_JAR);
     idlHelper = new AndroidIdlHelper(ruleContext, classJar);
@@ -492,8 +508,14 @@ public class AndroidCommon {
     Artifact resourcesJar = resourceApk.getResourceJavaSrcJar();
     if (resourcesJar != null) {
       filesBuilder.add(resourcesJar);
-      compileResources(javaSemantics, resourcesJar, artifactsBuilder, attributes, filesBuilder,
-          jarsProducedForRuntime);
+      // Use a fast-path R class generator for android_binary with local resources, where there is
+      // a bottleneck. For legacy resources, the srcjar and R class compiler don't match up
+      // (the legacy srcjar requires the createJarJar step below).
+      boolean useRClassGenerator =
+          getAndroidConfig(ruleContext).useRClassGenerator()
+          && isBinary && !resourceApk.isLegacy();
+      compileResources(javaSemantics, resourceApk, resourcesJar, artifactsBuilder, attributes,
+          filesBuilder, jarsProducedForRuntime, useRClassGenerator);
       if (resourceApk.isLegacy()) {
         // Repackages the R.java for each dependency package and places the resultant jars before
         // the dependency libraries to ensure that the generated resource ids are correct.
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 d8f7cbb724..bcb7792a7d 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
@@ -338,6 +338,15 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
             + "transition to the Android manifest merger from the legacy merger.")
     public AndroidManifestMerger manifestMerger;
 
+    // Do not use on the command line.
+    // The idea is that once this option works, we'll flip the default value in a config file, then
+    // once it is proven that it works, remove it from Bazel and said config file.
+    @Option(name = "experimental_use_rclass_generator",
+        defaultValue = "false",
+        category = "undocumented",
+        help = "Use the specialized R class generator to build the final app and lib R classes.")
+    public boolean useRClassGenerator;
+
     @Override
     public void addAllLabels(Multimap<String, Label> labelMap) {
       if (androidCrosstoolTop != null) {
@@ -408,6 +417,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
   private final boolean allowAndroidLibraryDepsWithoutSrcs;
   private final boolean useAndroidResourceShrinking;
   private final boolean useProguardPreviousObfuscationMap;
+  private final boolean useRClassGenerator;
   private final AndroidManifestMerger manifestMerger;
 
   AndroidConfiguration(Options options, Label androidSdk) {
@@ -432,6 +442,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     this.allowAndroidLibraryDepsWithoutSrcs = options.allowAndroidLibraryDepsWithoutSrcs;
     this.useAndroidResourceShrinking = options.useAndroidResourceShrinking;
     this.useProguardPreviousObfuscationMap = options.useProguardPreviousObfuscationMap;
+    this.useRClassGenerator = options.useRClassGenerator;
     this.manifestMerger = options.manifestMerger;
   }
 
@@ -509,6 +520,10 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     return useProguardPreviousObfuscationMap;
   }
 
+  public boolean useRClassGenerator() {
+    return useRClassGenerator;
+  }
+
   public AndroidManifestMerger getManifestMerger() {
     return manifestMerger;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
index 964db8ae54..33c423e5db 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
@@ -116,7 +116,8 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
         androidSemantics,
         resourceApk,
         false /* addCoverageSupport */,
-        true /* collectJavaCompilationArgs */);
+        true /* collectJavaCompilationArgs */,
+        false /* isBinary */);
     if (javaTargetAttributes == null) {
       return null;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
index d779ce04b4..197c1552fd 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
@@ -404,6 +404,8 @@ public final class AndroidRuleClasses {
               env.getToolsLabel(DEFAULT_AAR_GENERATOR)))
           .add(attr("$android_manifest_merger", LABEL).cfg(HOST).exec().value(
               env.getToolsLabel(DEFAULT_MANIFEST_MERGER)))
+          .add(attr("$android_rclass_generator", LABEL).cfg(HOST).exec().value(
+              env.getToolsLabel(DEFAULT_RCLASS_GENERATOR)))
           .add(attr("$android_resources_processor", LABEL).cfg(HOST).exec().value(
               env.getToolsLabel(DEFAULT_RESOURCES_PROCESSOR)))
           .add(attr("$android_resource_shrinker", LABEL).cfg(HOST).exec().value(