Android: Use R8 for dex splitting.

R8 has now added support (b/122902374) for this, fixing a few
outstanding bugs related to class merging. This also makes build rules
a bit simpler.

Still need to switch Trichrome synchronized proguard over to using
R8 for dexsplitting, then we can remove dexsplitter support entirely.

This CL updates R8 with another patch, but does not change the R8
base version used.

Binary-Size: R8 patch now keeping extra methods.
Bug: 1032609
Change-Id: I79f88736c20d96b718eb129cdbd093fe1cd297b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2014309
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Sam Maier <smaier@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735315}

DEPS

@@ -1400,7 +1400,7 @@ deps = {
       'packages': [
           {
               'package': 'chromium/third_party/r8',
-              'version': '-oXGY8FjY2ZuIBHoGAByn8N6Vn2b0wB2QO8Ct_169XoC',
+              'version': 'vDpIPy78EWpdAzbQ6cYYJMQaFQghOjmCtF0NcaoO0IIC',
           },
       ],
       'condition': 'checkout_android',

build/android/gyp/proguard.py

@@ -5,6 +5,7 @@
 # found in the LICENSE file.
 
 import argparse
+import itertools
 import os
 import re
 import shutil
@@ -111,8 +112,7 @@ def _ParseOptions():
   group.add_argument('--r8-path', help='Path to the R8.jar to use.')
   parser.add_argument(
       '--input-paths', required=True, help='GN-list of .jar files to optimize.')
-  parser.add_argument(
-      '--output-path', required=True, help='Path to the generated .jar file.')
+  parser.add_argument('--output-path', help='Path to the generated .jar file.')
   parser.add_argument(
       '--proguard-configs',
       action='append',
@@ -160,9 +160,34 @@ def _ParseOptions():
       '--disable-checkdiscard',
       action='store_true',
       help='Disable -checkdiscard directives')
+  parser.add_argument(
+      '--feature-jars',
+      action='append',
+      help='GN list of path to jars which comprise the corresponding feature.')
+  parser.add_argument(
+      '--dex-dest',
+      action='append',
+      dest='dex_dests',
+      help='Destination for dex file of the corresponding feature.')
+  parser.add_argument(
+      '--feature-name',
+      action='append',
+      dest='feature_names',
+      help='The name of the feature module.')
+  parser.add_argument(
+      '--stamp',
+      help='File to touch upon success. Mutually exclusive with --output-path')
 
   options = parser.parse_args(args)
 
+  if options.feature_names:
+    if options.output_path:
+      parser.error('Feature splits cannot specify an output in GN.')
+    if not options.stamp:
+      parser.error('Feature splits require a stamp file as output.')
+  elif not options.output_path:
+    parser.error('Output path required when feature splits aren\'t used')
+
   if options.main_dex_rules_path and not options.r8_path:
     parser.error('R8 must be enabled to pass main dex rules.')
 
@@ -178,6 +203,17 @@ def _ParseOptions():
   options.extra_mapping_output_paths = build_utils.ParseGnList(
       options.extra_mapping_output_paths)
 
+  if options.feature_names:
+    if 'base' not in options.feature_names:
+      parser.error('"base" feature required when feature arguments are used.')
+    if len(options.feature_names) != len(options.feature_jars) or len(
+        options.feature_names) != len(options.dex_dests):
+      parser.error('Invalid feature argument lengths.')
+
+    options.feature_jars = [
+        build_utils.ParseGnList(x) for x in options.feature_jars
+    ]
+
   return options
 
 
@@ -199,6 +235,36 @@ https://chromium.googlesource.com/chromium/src/+/HEAD/chrome/android/java/README
       f.write(msg)
 
 
+class _DexPathContext(object):
+  def __init__(self, name, output_path, input_jars, work_dir):
+    self.name = name
+    self.input_paths = input_jars
+    self._final_output_path = output_path
+    self._temporary_dir = os.path.join(work_dir, name)
+
+  def OutputPath(self):
+    if not os.path.exists(self._temporary_dir):
+      os.mkdir(self._temporary_dir)
+    return self._temporary_dir
+
+  def Commit(self):
+    found_files = build_utils.FindInDirectory(self._temporary_dir)
+    if self._final_output_path.endswith('.dex'):
+      if len(found_files) != 1:
+        raise Exception('Expected exactly 1 dex file output, found: {}'.format(
+            len(found_files)))
+      shutil.move(found_files[0], self._final_output_path)
+    else:
+      if len(found_files) == 0:
+        raise Exception('Missing dex outputs in {}'.format(self._temporary_dir))
+      # Add to .jar using Python rather than having R8 output to a .zip directly
+      # in order to disable compression of the .jar, saving ~500ms.
+      tmp_jar_output = self._temporary_dir + '.jar'
+      build_utils.DoZip(
+          found_files, tmp_jar_output, base_dir=self._temporary_dir)
+      shutil.move(tmp_jar_output, self._final_output_path)
+
+
 def _OptimizeWithR8(options,
                     config_paths,
                     libraries,
@@ -219,6 +285,20 @@ def _OptimizeWithR8(options,
     tmp_output = os.path.join(tmp_dir, 'r8out')
     os.mkdir(tmp_output)
 
+    feature_contexts = []
+    if options.feature_names:
+      for name, dest_dex, input_paths in itertools.izip(
+          options.feature_names, options.dex_dests, options.feature_jars):
+        feature_context = _DexPathContext(name, dest_dex, input_paths,
+                                          tmp_output)
+        if name == 'base':
+          base_dex_context = feature_context
+        else:
+          feature_contexts.append(feature_context)
+    else:
+      base_dex_context = _DexPathContext('base', options.output_path,
+                                         options.input_paths, tmp_output)
+
     cmd = [
         build_utils.JAVA_PATH,
         '-jar',
@@ -226,7 +306,7 @@ def _OptimizeWithR8(options,
         '--no-desugaring',
         '--no-data-resources',
         '--output',
-        tmp_output,
+        base_dex_context.OutputPath(),
         '--pg-map-output',
         tmp_mapping_path,
     ]
@@ -244,7 +324,19 @@ def _OptimizeWithR8(options,
       for main_dex_rule in options.main_dex_rules_path:
         cmd += ['--main-dex-rules', main_dex_rule]
 
-    cmd += options.input_paths
+    input_code_paths = set()
+    for feature in feature_contexts:
+      for feature_input_jar in feature.input_paths:
+        if feature_input_jar not in base_dex_context.input_paths:
+          input_code_paths.add(feature_input_jar)
+          cmd += [
+              '--feature-jar', feature_input_jar + ':' + feature.OutputPath()
+          ]
+
+    input_code_paths.update(base_dex_context.input_paths)
+    assert input_code_paths == set(options.input_paths), (
+        'Sum of feature input jars not equal to expected runtime classpath.')
+    cmd += base_dex_context.input_paths
 
     env = os.environ.copy()
     stderr_filter = lambda l: re.sub(r'.*_JAVA_OPTIONS.*\n?', '', l)
@@ -261,17 +353,10 @@ def _OptimizeWithR8(options,
           'android/docs/java_optimization.md#Debugging-common-failures\n'))
       raise ProguardProcessError(err, debugging_link)
 
-    found_files = build_utils.FindInDirectory(tmp_output)
-    if not options.output_path.endswith('.dex'):
-      # Add to .jar using Python rather than having R8 output to a .zip directly
-      # in order to disable compression of the .jar, saving ~500ms.
-      tmp_jar_output = tmp_output + '.jar'
-      build_utils.DoZip(found_files, tmp_jar_output, base_dir=tmp_output)
-      shutil.move(tmp_jar_output, options.output_path)
-    else:
-      if len(found_files) > 1:
-        raise Exception('Too many files created: {}'.format(found_files))
-      shutil.move(found_files[0], options.output_path)
+    base_dex_context.Commit()
+    if options.feature_names:
+      for feature in feature_contexts:
+        feature.Commit()
 
     with open(options.mapping_output, 'w') as out_file, \
         open(tmp_mapping_path) as in_file:
@@ -279,6 +364,8 @@ def _OptimizeWithR8(options,
       # some of our tooling so remove those (specifically: apkanalyzer).
       out_file.writelines(l for l in in_file if not l.startswith('#'))
 
+  return input_code_paths
+
 
 def _OptimizeWithProguard(options,
                           config_paths,
@@ -459,23 +546,29 @@ def main():
                              options.output_config,
                              options.proguard_expectations_failure_file)
 
+  inputs = options.proguard_configs + libraries
   if options.r8_path:
-    _OptimizeWithR8(options, proguard_configs, libraries, dynamic_config_data,
-                    print_stdout)
+    inputs += _OptimizeWithR8(options, proguard_configs, libraries,
+                              dynamic_config_data, print_stdout)
   else:
     _OptimizeWithProguard(options, proguard_configs, libraries,
                           dynamic_config_data, print_stdout)
+    inputs += options.input_paths
 
   # After ProGuard / R8 has run:
   for output in options.extra_mapping_output_paths:
     shutil.copy(options.mapping_output, output)
 
-  inputs = options.proguard_configs + options.input_paths + libraries
   if options.apply_mapping:
     inputs.append(options.apply_mapping)
 
+  output_path = options.output_path
+  if options.stamp:
+    build_utils.Touch(options.stamp)
+    output_path = options.stamp
+
   build_utils.WriteDepfile(
-      options.depfile, options.output_path, inputs=inputs, add_pydeps=False)
+      options.depfile, output_path, inputs=inputs, add_pydeps=False)
 
 
 if __name__ == '__main__':

build/config/android/internal_rules.gni

@@ -969,16 +969,10 @@ if (enable_java_templates) {
         _mapping_path = invoker.proguard_mapping_path
       }
       depfile = "${target_gen_dir}/${target_name}.d"
-      outputs = [
-        _output_path,
-        _mapping_path,
-      ]
       _rebased_build_config = rebase_path(invoker.build_config, root_build_dir)
       args = [
         "--depfile",
         rebase_path(depfile, root_build_dir),
-        "--output-path",
-        rebase_path(_output_path, root_build_dir),
         "--mapping-output",
         rebase_path(_mapping_path, root_build_dir),
         "--classpath",
@@ -987,6 +981,35 @@ if (enable_java_templates) {
         "@FileArg($_rebased_build_config:android:sdk_jars)",
       ]
 
+      if (defined(invoker.modules)) {
+        foreach(_feature_module, invoker.modules) {
+          _rebased_module_build_config =
+              rebase_path(_feature_module.build_config, root_build_dir)
+          args += [
+            "--feature-name",
+            _feature_module.name,
+            "--dex-dest=@FileArg($_rebased_module_build_config:final_dex:path)",
+          ]
+          if (!defined(invoker.feature_jars_args)) {
+            args += [ "--feature-jars=@FileArg($_rebased_module_build_config:deps_info:java_runtime_classpath)" ]
+          }
+          deps += [ _feature_module.build_config_target ]
+        }
+        _stamp = "${target_gen_dir}/${target_name}.stamp"
+        outputs = [ _stamp ]
+        args += [
+          "--stamp",
+          rebase_path(_stamp, root_build_dir),
+        ]
+      } else {
+        args += [
+          "--output-path",
+          rebase_path(_output_path, root_build_dir),
+        ]
+        outputs = [ _output_path ]
+      }
+
+      outputs += [ _mapping_path ]
       if (defined(invoker.disable_r8_outlining) &&
           invoker.disable_r8_outlining) {
         args += [ "--disable-outlining" ]
@@ -1106,8 +1129,6 @@ if (enable_java_templates) {
   #   apply_mapping: The path to the ProGuard mapping file to apply.
   #   disable_incremental: Disable incremental dexing.
   template("dex") {
-    assert(defined(invoker.output))
-
     _proguard_enabled =
         defined(invoker.proguard_enabled) && invoker.proguard_enabled
     _proguarding_with_r8 =
@@ -1122,7 +1143,8 @@ if (enable_java_templates) {
         not_needed(invoker, [ "negative_main_dex_globs" ])
       }
     }
-
+    assert(defined(invoker.output) ||
+           (_proguarding_with_r8 && defined(invoker.modules)))
     assert(!_proguard_enabled || !(defined(invoker.input_dex_filearg) ||
                                        defined(invoker.input_classes_filearg) ||
                                        defined(invoker.input_class_jars)),
@@ -1137,7 +1159,13 @@ if (enable_java_templates) {
 
     if (_proguard_enabled) {
       if (_proguarding_with_r8) {
+        if (defined(invoker.output)) {
           _proguard_output_path = invoker.output
+        } else {
+          # This will never actually be created, but other outputs use this as
+          # a base bath.
+          _proguard_output_path = "$target_gen_dir/$target_name.jar"
+        }
         _proguard_target_name = target_name
         _proguard_config_output_path = "$_proguard_output_path.proguard_flags"
       } else {
@@ -1153,6 +1181,7 @@ if (enable_java_templates) {
                                  "disable_r8_outlining",
                                  "deps",
                                  "failed_proguard_expectation_file",
+                                 "modules",
                                  "proguard_expectations_file",
                                  "proguard_jar_path",
                                  "proguard_mapping_path",

build/config/android/rules.gni

@@ -4462,12 +4462,9 @@ if (enable_java_templates) {
       _uses_static_library_synchronized_proguard =
           defined(invoker.static_library_synchronized_proguard) &&
           invoker.static_library_synchronized_proguard
-      if (_uses_static_library_synchronized_proguard) {
-        _sync_dex_target_dep = "${invoker.static_library_provider}__dexsplitter"
-      } else {
-        _sync_dex_target = "${_target_name}__sync_dex"
-        _sync_dex_target_dep = ":$_sync_dex_target"
-      }
+
+      # TODO(crbug.com/1032609): Remove dexsplitter from Trichrome Proguard.
+      _dex_target = "${_target_name}__dex"
       _proguard_mapping_path = "${_bundle_path}.mapping"
     }
 
@@ -4581,16 +4578,12 @@ if (enable_java_templates) {
       _proguard_mapping_path = "${_bundle_path}.mapping"
     }
 
-    _unsplit_dex_zip =
-        "${target_gen_dir}/${_target_name}/${_target_name}__unsplit_dex.zip"
-
     write_build_config(_build_config_target) {
       type = "android_app_bundle"
       possible_config_deps = _module_targets + [ proguard_android_sdk_dep_ ]
       build_config = _build_config
       proguard_enabled = _proguard_enabled
       module_build_configs = _module_build_configs
-      final_dex_path = _unsplit_dex_zip
 
       if (_proguard_enabled) {
         proguard_mapping_path = _proguard_mapping_path
@@ -4615,7 +4608,7 @@ if (enable_java_templates) {
               get_label_info(":$_target_name", "dir") +
               "/java/$_target_name.proguard_flags.expected"
         }
-        dex(_sync_dex_target) {
+        dex(_dex_target) {
           enable_multidex = _enable_multidex
           proguard_enabled = true
           proguard_mapping_path = _proguard_mapping_path
@@ -4632,19 +4625,8 @@ if (enable_java_templates) {
           }
 
           deps = _module_java_targets + [ ":$_build_config_target" ]
-          output = _unsplit_dex_zip
-        }
+          modules = _modules
         }
-
-      _dexsplitter_target = "${_target_name}__dexsplitter"
-      dexsplitter(_dexsplitter_target) {
-        input_dex_zip = _unsplit_dex_zip
-        proguard_mapping = _proguard_mapping_path
-        all_modules = _modules
-        deps = [
-                 ":$_build_config_target",
-                 _sync_dex_target_dep,
-               ] + _module_java_targets
       }
     }
 
@@ -4659,7 +4641,7 @@ if (enable_java_templates) {
       if (!_proguard_enabled) {
         _dex_target_for_module = "${_module_target}__final_dex"
       } else {
-        _dex_target_for_module = ":$_dexsplitter_target"
+        _dex_target_for_module = ":$_dex_target"
       }
 
       # Generate one module .zip file per bundle module.

third_party/r8/README.chromium

@@ -18,6 +18,7 @@ Turning off outlining because it caused issues when synchronized proguarding
 Trichrome due to illegal dex references (crbug.com/956839). Will only be used
 for android_apk() targets that set `disable_r8_outlining = true`):
 
-Increase callGraphCycleEliminatorMaxDepthThreshold (b/143685705).
+Adding command line support for having R8 generate feature module .dex files
+(crbug.com/1032609).
 
 Patch is located at //third_party/r8/local_modifications.diff.

third_party/r8/local_modifications.diff

+diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java
+index 8c63d06f5..aee9ba39b 100644
+--- a/src/main/java/com/android/tools/r8/R8Command.java
++++ b/src/main/java/com/android/tools/r8/R8Command.java
+@@ -3,6 +3,7 @@
+ // BSD-style license that can be found in the LICENSE file.
+ package com.android.tools.r8;
+ 
++import com.android.tools.r8.DexIndexedConsumer.DirectoryConsumer;
+ import com.android.tools.r8.ProgramResource.Kind;
+ import com.android.tools.r8.errors.DexFileOverflowDiagnostic;
+ import com.android.tools.r8.experimental.graphinfo.GraphConsumer;
+@@ -268,6 +269,15 @@ public final class R8Command extends BaseCompilerCommand {
+       return self();
+     }
+ 
++    public Builder addFeatureJar(Path inputJarPath, Path outputPath) {
++      addFeatureSplit(splitBuilder ->
++          splitBuilder
++            .addProgramResourceProvider(ArchiveProgramResourceProvider.fromArchive(inputJarPath))
++            .setProgramConsumer(new DirectoryConsumer(outputPath))
++            .build());
++      return self();
++    }
++
+     /**
+      * Set a consumer for receiving the proguard seeds information.
+      *
+diff --git a/src/main/java/com/android/tools/r8/R8CommandParser.java b/src/main/java/com/android/tools/r8/R8CommandParser.java
+index fd4f570ea..66a2e9506 100644
+--- a/src/main/java/com/android/tools/r8/R8CommandParser.java
++++ b/src/main/java/com/android/tools/r8/R8CommandParser.java
+@@ -25,7 +25,8 @@ public class R8CommandParser extends BaseCompilerCommandParser {
+           "--main-dex-list",
+           "--main-dex-list-output",
+           "--pg-conf",
+-          "--pg-map-output");
++          "--pg-map-output",
++          "--feature-jar");
+ 
+   public static void main(String[] args) throws CompilationFailedException {
+     R8Command command = parse(args, Origin.root()).build();
+@@ -74,8 +75,11 @@ public class R8CommandParser extends BaseCompilerCommandParser {
+               "  --main-dex-list <file>   # List of classes to place in the primary dex file.",
+               "  --main-dex-list-output <file>  ",
+               "                           # Output the full main-dex list in <file>.",
++              "  --feature-jar <file>:<file>  ",
++              "                           # Feature jar <input-file>:<output-dir> files.",
+               "  --version                # Print the version of r8.",
+               "  --help                   # Print this message."));
++
+   /**
+    * Parse the R8 command-line.
+    *
+@@ -208,6 +212,11 @@ public class R8CommandParser extends BaseCompilerCommandParser {
+         builder.setProguardMapOutputPath(Paths.get(nextArg));
+       } else if (arg.equals("--no-data-resources")) {
+         state.includeDataResources = false;
++      } else if (arg.equals("--feature-jar")) {
++        String[] argParts = nextArg.split(":");
++        String featureJarInputPath = argParts[0];
++        String featureJarOutputPath = argParts[1];
++        builder.addFeatureJar(Paths.get(featureJarInputPath), Paths.get(featureJarOutputPath));
+       } else {
+         if (arg.startsWith("--")) {
+           builder.error(new StringDiagnostic("Unknown option: " + arg, argsOrigin));
 diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
 index 595ac28da..e96f06364 100644
 --- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java