Android: Move dex merging to incremental install step

* Fully disables dex merging for library targets, speeding up builds.
* Introduces sharding into incremental installs, dramatically speeding
  up launch times.
  * E.g. instead of ~250 dex files to load, shards into ~25 files.

Bug: 937005, 1016846
Change-Id: I9a206145be196af61e55c4a25dcce3fbceb7190c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1874046
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708295}

build/android/gyp/create_bundle_wrapper_script.pydeps

@@ -87,6 +87,7 @@
 ../../gn_helpers.py
 ../adb_command_line.py
 ../apk_operations.py
+../convert_dex_profile.py
 ../devil_chromium.py
 ../incremental_install/__init__.py
 ../incremental_install/installer.py
@@ -101,7 +102,9 @@
 ../pylib/utils/time_profile.py
 bundletool.py
 create_bundle_wrapper_script.py
+dex.py
 util/__init__.py
 util/build_utils.py
 util/md5_check.py
 util/resource_utils.py
+util/zipalign.py

build/android/gyp/dex.py

@@ -44,10 +44,6 @@ def _ParseArgs(args):
   parser.add_argument(
       '--incremental-dir',
       help='Path of directory to put intermediate dex files.')
-  parser.add_argument(
-      '--merge-incrementals',
-      action='store_true',
-      help='Combine all per-class .dex files into a single classes.dex')
   parser.add_argument(
       '--main-dex-list-path',
       help='File containing a list of the classes to include in the main dex.')
@@ -272,22 +268,22 @@ def _PerformDexlayout(tmp_dir, tmp_dex_output, options):
   return final_output
 
 
-def _CreateFinalDex(options, d8_inputs, tmp_dir, dex_cmd):
+def _CreateFinalDex(d8_inputs, output, tmp_dir, dex_cmd, options=None):
   tmp_dex_output = os.path.join(tmp_dir, 'tmp_dex_output.zip')
-  if (options.merge_incrementals or options.output.endswith('.dex')
+  if (output.endswith('.dex')
       or not all(f.endswith('.dex') for f in d8_inputs)):
-    if options.multi_dex and options.main_dex_list_path:
+    if options and options.main_dex_list_path:
       # Provides a list of classes that should be included in the main dex file.
       dex_cmd = dex_cmd + ['--main-dex-list', options.main_dex_list_path]
 
     tmp_dex_dir = os.path.join(tmp_dir, 'tmp_dex_dir')
     os.mkdir(tmp_dex_dir)
     _RunD8(dex_cmd, d8_inputs, tmp_dex_dir)
-    logging.info('Performed dex merging')
+    logging.debug('Performed dex merging')
 
     dex_files = [os.path.join(tmp_dex_dir, f) for f in os.listdir(tmp_dex_dir)]
 
-    if options.output.endswith('.dex'):
+    if output.endswith('.dex'):
       if len(dex_files) > 1:
         raise Exception('%d files created, expected 1' % len(dex_files))
       tmp_dex_output = dex_files[0]
@@ -296,13 +292,13 @@ def _CreateFinalDex(options, d8_inputs, tmp_dir, dex_cmd):
   else:
     # Skip dexmerger. Just put all incrementals into the .jar individually.
     _ZipAligned(sorted(d8_inputs), tmp_dex_output)
-    logging.info('Quick-zipped %d files', len(d8_inputs))
+    logging.debug('Quick-zipped %d files', len(d8_inputs))
 
-  if options.dexlayout_profile:
+  if options and options.dexlayout_profile:
     tmp_dex_output = _PerformDexlayout(tmp_dir, tmp_dex_output, options)
 
   # The dex file is complete and can be moved out of tmp_dir.
-  shutil.move(tmp_dex_output, options.output)
+  shutil.move(tmp_dex_output, output)
 
 
 def _IntermediateDexFilePathsFromInputJars(class_inputs, incremental_dir):
@@ -354,18 +350,18 @@ def _CreateIntermediateDexFiles(changes, options, tmp_dir, dex_cmd):
     changes = None
   class_files = _ExtractClassFiles(changes, tmp_extract_dir,
                                    options.class_inputs)
-  logging.info('Extracted class files: %d', len(class_files))
+  logging.debug('Extracted class files: %d', len(class_files))
 
   # If the only change is deleting a file, class_files will be empty.
   if class_files:
     # Dex necessary classes into intermediate dex files.
     dex_cmd = dex_cmd + ['--intermediate', '--file-per-class']
     _RunD8(dex_cmd, class_files, options.incremental_dir)
-    logging.info('Dexed class files.')
+    logging.debug('Dexed class files.')
 
 
 def _OnStaleMd5(changes, options, final_dex_inputs, dex_cmd):
-  logging.info('_OnStaleMd5')
+  logging.debug('_OnStaleMd5')
   with build_utils.TempDir() as tmp_dir:
     if options.incremental_dir:
       # Create directory for all intermediate dex files.
@@ -373,11 +369,23 @@ def _OnStaleMd5(changes, options, final_dex_inputs, dex_cmd):
         os.makedirs(options.incremental_dir)
 
       _DeleteStaleIncrementalDexFiles(options.incremental_dir, final_dex_inputs)
-      logging.info('Stale files deleted')
+      logging.debug('Stale files deleted')
       _CreateIntermediateDexFiles(changes, options, tmp_dir, dex_cmd)
 
-    _CreateFinalDex(options, final_dex_inputs, tmp_dir, dex_cmd)
-  logging.info('Dex finished for: %s', options.output)
+    _CreateFinalDex(
+        final_dex_inputs, options.output, tmp_dir, dex_cmd, options=options)
+  logging.debug('Dex finished for: %s', options.output)
+
+
+def MergeDexForIncrementalInstall(r8_jar_path, src_paths, dest_dex_jar):
+  dex_cmd = [
+      build_utils.JAVA_PATH,
+      '-jar',
+      r8_jar_path,
+      'd8',
+  ]
+  with build_utils.TempDir() as tmp_dir:
+    _CreateFinalDex(src_paths, dest_dex_jar, tmp_dir, dex_cmd)
 
 
 def main(args):

build/android/incremental_install/README.md

@@ -47,7 +47,9 @@ Slower Initial Runs:
  * The first time you run an incremental .apk, the `DexOpt` needs to run on all
    .dex files. This step is normally done during `adb install`, but is done on
    start-up for incremental apks.
-   * DexOpt results are cached, so subsequent runs are much faster
+   * DexOpt results are cached, so subsequent runs are faster.
+   * The slowdown varies significantly based on the Android version. Android O+
+     has almost no visible slow-down.
 
 Caveats:
  * Isolated processes (on L+) are incompatible with incremental install. As a

build/android/incremental_install/installer.py

@@ -7,6 +7,8 @@
 """Install *_incremental.apk targets as well as their dependent files."""
 
 import argparse
+import collections
+import functools
 import glob
 import json
 import logging
@@ -14,7 +16,6 @@ import os
 import posixpath
 import shutil
 import sys
-import zipfile
 
 sys.path.append(
     os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir)))
@@ -28,24 +29,20 @@ from pylib.utils import time_profile
 
 prev_sys_path = list(sys.path)
 sys.path.insert(0, os.path.join(os.path.dirname(__file__), os.pardir, 'gyp'))
+import dex
 from util import build_utils
 sys.path = prev_sys_path
 
 
+_R8_PATH = os.path.join(build_utils.DIR_SOURCE_ROOT, 'third_party', 'r8', 'lib',
+                        'r8.jar')
+
+
 def _DeviceCachePath(device):
   file_name = 'device_cache_%s.json' % device.adb.GetDeviceSerial()
   return os.path.join(constants.GetOutDirectory(), file_name)
 
 
-def _TransformDexPaths(paths):
-  """Given paths like ["/a/b/c", "/a/c/d"], returns ["b.c", "c.d"]."""
-  if len(paths) == 1:
-    return [os.path.basename(paths[0])]
-
-  prefix_len = len(os.path.commonprefix(paths))
-  return [p[prefix_len:].replace(os.sep, '.') for p in paths]
-
-
 def _Execute(concurrently, *funcs):
   """Calls all functions in |funcs| concurrently or in sequence."""
   timer = time_profile.TimeProfile()
@@ -63,10 +60,63 @@ def _GetDeviceIncrementalDir(package):
   return '/data/local/tmp/incremental-app-%s' % package
 
 
-def _HasClasses(jar_path):
-  """Returns whether the given jar contains classes.dex."""
-  with zipfile.ZipFile(jar_path) as jar:
-    return 'classes.dex' in jar.namelist()
+def _IsStale(src_paths, dest):
+  """Returns if |dest| is older than any of |src_paths|, or missing."""
+  if not os.path.exists(dest):
+    return True
+  dest_time = os.path.getmtime(dest)
+  for path in src_paths:
+    if os.path.getmtime(path) > dest_time:
+      return True
+  return False
+
+
+def _AllocateDexShards(dex_files):
+  """Divides input dex files into buckets."""
+  # Goals:
+  # * Make shards small enough that they are fast to merge.
+  # * Minimize the number of shards so they load quickly on device.
+  # * Partition files into shards such that a change in one file results in only
+  #   one shard having to be re-created.
+  shards = collections.defaultdict(list)
+  # As of Oct 2019, 10 shards results in a min/max size of 582K/2.6M.
+  NUM_CORE_SHARDS = 10
+  # As of Oct 2019, 17 dex files are larger than 1M.
+  SHARD_THRESHOLD = 2**20
+  for src_path in dex_files:
+    if os.path.getsize(src_path) >= SHARD_THRESHOLD:
+      # Use the path as the name rather than an incrementing number to ensure
+      # that it shards to the same name every time.
+      name = os.path.relpath(src_path, constants.GetOutDirectory()).replace(
+          os.sep, '.')
+      shards[name].append(src_path)
+    else:
+      name = 'shard{}.dex.jar'.format(hash(src_path) % NUM_CORE_SHARDS)
+      shards[name].append(src_path)
+  logging.info('Sharding %d dex files into %d buckets', len(dex_files),
+               len(shards))
+  return shards
+
+
+def _CreateDexFiles(shards, dex_staging_dir, use_concurrency):
+  """Creates dex files within |dex_staging_dir| defined by |shards|."""
+  tasks = []
+  for name, src_paths in shards.iteritems():
+    dest_path = os.path.join(dex_staging_dir, name)
+    if _IsStale(src_paths, dest_path):
+      tasks.append(
+          functools.partial(dex.MergeDexForIncrementalInstall, _R8_PATH,
+                            src_paths, dest_path))
+
+  # TODO(agrieve): It would be more performant to write a custom d8.jar
+  #     wrapper in java that would process these in bulk, rather than spinning
+  #     up a new process for each one.
+  _Execute(use_concurrency, *tasks)
+
+  # Remove any stale shards.
+  for name in os.listdir(dex_staging_dir):
+    if name not in shards:
+      os.unlink(os.path.join(dex_staging_dir, name))
 
 
 def Uninstall(device, package, enable_device_cache=False):
@@ -108,6 +158,7 @@ def Install(device, install_json, apk=None, enable_device_cache=False,
   main_timer = time_profile.TimeProfile()
   install_timer = time_profile.TimeProfile()
   push_native_timer = time_profile.TimeProfile()
+  merge_dex_timer = time_profile.TimeProfile()
   push_dex_timer = time_profile.TimeProfile()
 
   def fix_path(p):
@@ -122,6 +173,10 @@ def Install(device, install_json, apk=None, enable_device_cache=False,
 
   apk_package = apk.GetPackageName()
   device_incremental_dir = _GetDeviceIncrementalDir(apk_package)
+  dex_staging_dir = os.path.join(constants.GetOutDirectory(),
+                                 'incremental-install',
+                                 install_dict['apk_path'])
+  device_dex_dir = posixpath.join(device_incremental_dir, 'dex')
 
   # Install .apk(s) if any of them have changed.
   def do_install():
@@ -144,6 +199,8 @@ def Install(device, install_json, apk=None, enable_device_cache=False,
 
   # Push .so and .dex files to the device (if they have changed).
   def do_push_files():
+
+    def do_push_native():
       push_native_timer.Start()
       if native_libs:
         with build_utils.TempDir() as temp_dir:
@@ -156,23 +213,22 @@ def Install(device, install_json, apk=None, enable_device_cache=False,
                                   delete_device_stale=True)
       push_native_timer.Stop(log=False)
 
+    def do_merge_dex():
+      merge_dex_timer.Start()
+      shards = _AllocateDexShards(dex_files)
+      build_utils.MakeDirectory(dex_staging_dir)
+      _CreateDexFiles(shards, dex_staging_dir, use_concurrency)
+      merge_dex_timer.Stop(log=False)
+
+    def do_push_dex():
       push_dex_timer.Start()
-    if dex_files:
-      # Put all .dex files to be pushed into a temporary directory so that we
-      # can use delete_device_stale=True.
-      with build_utils.TempDir() as temp_dir:
-        device_dex_dir = posixpath.join(device_incremental_dir, 'dex')
-        # Ensure no two files have the same name.
-        transformed_names = _TransformDexPaths(dex_files)
-        for src_path, dest_name in zip(dex_files, transformed_names):
-          # Binary targets with no extra classes create .dex.jar without a
-          # classes.dex (which Android chokes on).
-          if _HasClasses(src_path):
-            shutil.copy(src_path, os.path.join(temp_dir, dest_name))
-        device.PushChangedFiles([(temp_dir, device_dex_dir)],
+      device.PushChangedFiles([(dex_staging_dir, device_dex_dir)],
                               delete_device_stale=True)
       push_dex_timer.Stop(log=False)
 
+    _Execute(use_concurrency, do_push_native, do_merge_dex)
+    do_push_dex()
+
   def check_device_configured():
     target_sdk_version = int(apk.GetTargetSdkVersion())
     # Beta Q builds apply whitelist to targetSdk=28 as well.
@@ -240,10 +296,10 @@ To restore back to default:
   finalize_timer = _Execute(use_concurrency, release_installer_lock, save_cache)
 
   logging.info(
-      'Install of %s took %s seconds '
-      '(setup=%s, install=%s, libs=%s, dex=%s, finalize=%s)',
-      os.path.basename(apk.path), main_timer.GetDelta(), setup_timer.GetDelta(),
-      install_timer.GetDelta(), push_native_timer.GetDelta(),
+      'Install of %s took %s seconds (setup=%s, install=%s, lib_push=%s, '
+      'dex_merge=%s dex_push=%s, finalize=%s)', os.path.basename(apk.path),
+      main_timer.GetDelta(), setup_timer.GetDelta(), install_timer.GetDelta(),
+      push_native_timer.GetDelta(), merge_dex_timer.GetDelta(),
       push_dex_timer.GetDelta(), finalize_timer.GetDelta())
   if show_proguard_warning:
     logging.warning('Target had proguard enabled, but incremental install uses '

build/android/test_runner.pydeps

@@ -127,10 +127,13 @@
 ../gn_helpers.py
 ../util/lib/common/chrome_test_server_spawner.py
 ../util/lib/common/unittest_util.py
+convert_dex_profile.py
 devil_chromium.py
+gyp/dex.py
 gyp/util/__init__.py
 gyp/util/build_utils.py
 gyp/util/md5_check.py
+gyp/util/zipalign.py
 incremental_install/__init__.py
 incremental_install/installer.py
 pylib/__init__.py

build/config/android/internal_rules.gni

@@ -1382,11 +1382,6 @@ if (enable_java_templates) {
             "--incremental-dir",
             rebase_path("$target_out_dir/$target_name", root_build_dir),
           ]
-          if (is_java_debug) {
-            # The performance of incremental install is unbearable if each
-            # lib.dex.jar file has multiple classes.dex files in it.
-            args += [ "--merge-incrementals" ]
-          }
         }
 
         if (_enable_multidex) {