Revert "Android: Use R8 for dex splitting."

This reverts commit b283bd25.

Reason for revert: Probable culprit for the intermittent compile failures on android_arm64-builder-perf. See crbug.com/1045866.

Original change's description:
> 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}

TBR=agrieve@chromium.org,estevenson@chromium.org,smaier@chromium.org

Change-Id: I917a73a43e38a4583d9fcedf43d8a29fffbdf914
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1032609, 1045866
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2020964Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735345}

DEPS

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

build/android/gyp/proguard.py

@@ -5,7 +5,6 @@
 # found in the LICENSE file.
 
 import argparse
-import itertools
 import os
 import re
 import shutil
@@ -112,7 +111,8 @@ 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', help='Path to the generated .jar file.')
+  parser.add_argument(
+      '--output-path', required=True, help='Path to the generated .jar file.')
   parser.add_argument(
       '--proguard-configs',
       action='append',
@@ -160,34 +160,9 @@ 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.')
 
@@ -203,17 +178,6 @@ 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
 
 
@@ -235,36 +199,6 @@ 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,
@@ -285,20 +219,6 @@ 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',
@@ -306,7 +226,7 @@ def _OptimizeWithR8(options,
         '--no-desugaring',
         '--no-data-resources',
         '--output',
-        base_dex_context.OutputPath(),
+        tmp_output,
         '--pg-map-output',
         tmp_mapping_path,
     ]
@@ -324,19 +244,7 @@ def _OptimizeWithR8(options,
       for main_dex_rule in options.main_dex_rules_path:
         cmd += ['--main-dex-rules', main_dex_rule]
 
-    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
+    cmd += options.input_paths
 
     env = os.environ.copy()
     stderr_filter = lambda l: re.sub(r'.*_JAVA_OPTIONS.*\n?', '', l)
@@ -353,10 +261,17 @@ def _OptimizeWithR8(options,
           'android/docs/java_optimization.md#Debugging-common-failures\n'))
       raise ProguardProcessError(err, debugging_link)
 
-    base_dex_context.Commit()
-    if options.feature_names:
-      for feature in feature_contexts:
-        feature.Commit()
+    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)
 
     with open(options.mapping_output, 'w') as out_file, \
         open(tmp_mapping_path) as in_file:
@@ -364,8 +279,6 @@ 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,
@@ -546,29 +459,23 @@ def main():
                              options.output_config,
                              options.proguard_expectations_failure_file)
 
-  inputs = options.proguard_configs + libraries
   if options.r8_path:
-    inputs += _OptimizeWithR8(options, proguard_configs, libraries,
-                              dynamic_config_data, print_stdout)
+    _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, output_path, inputs=inputs, add_pydeps=False)
+      options.depfile, options.output_path, inputs=inputs, add_pydeps=False)
 
 
 if __name__ == '__main__':

build/config/android/internal_rules.gni

@@ -969,10 +969,16 @@ 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",
@@ -981,35 +987,6 @@ 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" ]
@@ -1129,6 +1106,8 @@ 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 =
@@ -1143,8 +1122,7 @@ 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)),
@@ -1159,13 +1137,7 @@ 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_output_path = invoker.output
         _proguard_target_name = target_name
         _proguard_config_output_path = "$_proguard_output_path.proguard_flags"
       } else {
@@ -1181,7 +1153,6 @@ 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,9 +4462,12 @@ if (enable_java_templates) {
       _uses_static_library_synchronized_proguard =
           defined(invoker.static_library_synchronized_proguard) &&
           invoker.static_library_synchronized_proguard
-
-      # TODO(crbug.com/1032609): Remove dexsplitter from Trichrome Proguard.
-      _dex_target = "${_target_name}__dex"
+      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"
+      }
       _proguard_mapping_path = "${_bundle_path}.mapping"
     }
 
@@ -4578,12 +4581,16 @@ 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
@@ -4608,7 +4615,7 @@ if (enable_java_templates) {
               get_label_info(":$_target_name", "dir") +
               "/java/$_target_name.proguard_flags.expected"
         }
-        dex(_dex_target) {
+        dex(_sync_dex_target) {
           enable_multidex = _enable_multidex
           proguard_enabled = true
           proguard_mapping_path = _proguard_mapping_path
@@ -4625,9 +4632,20 @@ if (enable_java_templates) {
           }
 
           deps = _module_java_targets + [ ":$_build_config_target" ]
-          modules = _modules
+          output = _unsplit_dex_zip
         }
       }
+
+      _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
+      }
     }
 
     _all_create_module_targets = []
@@ -4641,7 +4659,7 @@ if (enable_java_templates) {
       if (!_proguard_enabled) {
         _dex_target_for_module = "${_module_target}__final_dex"
       } else {
-        _dex_target_for_module = ":$_dex_target"
+        _dex_target_for_module = ":$_dexsplitter_target"
       }
 
       # Generate one module .zip file per bundle module.

third_party/r8/README.chromium

@@ -18,7 +18,6 @@ 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`):
 
-Adding command line support for having R8 generate feature module .dex files
-(crbug.com/1032609).
+Increase callGraphCycleEliminatorMaxDepthThreshold (b/143685705).
 
 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