Android: Speed up dexing step of builds by re-using unchanged classes

This is a finish up of intern change:
https://chromium-review.googlesource.com/c/chromium/src/+/1738632

Changing one file in chrome_java before/after:
17.9s vs 3.6s for just the dex step.

Bug: 937874
Change-Id: I1aefc01446d99eedd79094e9429d7d69c6d05631
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1825471
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Reviewed-by: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701149}

build/android/gyp/dex.py

@@ -27,7 +27,23 @@ def _ParseArgs(args):
 
   build_utils.AddDepfileOption(parser)
   parser.add_argument('--output', required=True, help='Dex output path.')
-  parser.add_argument('--input-list', help='GN-list of additional input paths.')
+  parser.add_argument(
+      '--class-inputs',
+      action='append',
+      help='GN-list of .jars with .class files.')
+  parser.add_argument(
+      '--class-inputs-filearg',
+      action='append',
+      help='GN-list of .jars with .class files (added to depfile).')
+  parser.add_argument(
+      '--dex-inputs', action='append', help='GN-list of .jars with .dex files.')
+  parser.add_argument(
+      '--dex-inputs-filearg',
+      action='append',
+      help='GN-list of .jars with .dex files (added to depfile).')
+  parser.add_argument(
+      '--incremental-dir',
+      help='Path of directory to put intermediate dex files.')
   parser.add_argument(
       '--main-dex-list-path',
       help='File containing a list of the classes to include in the main dex.')
@@ -44,7 +60,6 @@ def _ParseArgs(args):
       'main dex and keeps all line number information, and then some.')
   parser.add_argument(
       '--min-api', help='Minimum Android API level compatibility.')
-  parser.add_argument('inputs', nargs='*', help='Input .jar files.')
 
   group = parser.add_argument_group('Dexlayout')
   group.add_argument(
@@ -79,8 +94,12 @@ def _ParseArgs(args):
   if options.main_dex_list_path and not options.multi_dex:
     parser.error('--main-dex-list-path is unused if multidex is not enabled')
 
-  if options.input_list:
-    options.inputs += build_utils.ParseGnList(options.input_list)
+  options.class_inputs = build_utils.ParseGnList(options.class_inputs)
+  options.class_inputs_filearg = build_utils.ParseGnList(
+      options.class_inputs_filearg)
+  options.dex_inputs = build_utils.ParseGnList(options.dex_inputs)
+  options.dex_inputs_filearg = build_utils.ParseGnList(
+      options.dex_inputs_filearg)
 
   return options
 
@@ -249,48 +268,142 @@ def _PerformDexlayout(tmp_dir, tmp_dex_output, options):
   return final_output
 
 
-def _PerformDexing(options):
-  dex_cmd = ['java', '-jar', options.r8_jar_path, 'd8', '--no-desugaring']
+def _CreateFinalDex(options, d8_inputs, tmp_dir, dex_cmd):
   if options.multi_dex and options.main_dex_list_path:
-    dex_cmd += ['--main-dex-list', options.main_dex_list_path]
-  if options.release:
-    dex_cmd += ['--release']
-  if options.min_api:
-    dex_cmd += ['--min-api', options.min_api]
+    # 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]
 
-  with build_utils.TempDir() as tmp_dir:
-    tmp_dex_dir = os.path.join(tmp_dir, 'tmp_dex_dir')
-    os.mkdir(tmp_dex_dir)
-    _RunD8(dex_cmd, options.inputs, tmp_dex_dir)
-    dex_files = [os.path.join(tmp_dex_dir, f) for f in os.listdir(tmp_dex_dir)]
-
-    if not options.output.endswith('.dex'):
-      tmp_dex_output = os.path.join(tmp_dir, 'tmp_dex_output.zip')
-      _ZipAligned(sorted(dex_files), tmp_dex_output)
+  tmp_dex_dir = os.path.join(tmp_dir, 'tmp_dex_dir')
+  os.mkdir(tmp_dex_dir)
+  _RunD8(dex_cmd, d8_inputs, tmp_dex_dir)
+
+  dex_files = [os.path.join(tmp_dex_dir, f) for f in os.listdir(tmp_dex_dir)]
+
+  if not options.output.endswith('.dex'):
+    tmp_dex_output = os.path.join(tmp_dir, 'tmp_dex_output.zip')
+    _ZipAligned(sorted(dex_files), tmp_dex_output)
+  else:
+    # Output to a .dex file.
+    if len(dex_files) > 1:
+      raise Exception('%d files created, expected 1' % len(dex_files))
+    tmp_dex_output = dex_files[0]
+
+  if 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)
+
+
+def _IntermediateDexFilePathsFromInputJars(class_inputs, incremental_dir):
+  """Returns a list of all intermediate dex file paths."""
+  dex_files = []
+  for jar in class_inputs:
+    with zipfile.ZipFile(jar, 'r') as z:
+      for subpath in z.namelist():
+        if subpath.endswith('.class'):
+          subpath = subpath[:-5] + 'dex'
+          dex_files.append(os.path.join(incremental_dir, subpath))
+  return dex_files
+
+
+def _DeleteStaleIncrementalDexFiles(dex_dir, dex_files):
+  """Deletes intermediate .dex files that are no longer needed."""
+  all_files = build_utils.FindInDirectory(dex_dir)
+  desired_files = set(dex_files)
+  for path in all_files:
+    if path not in desired_files:
+      os.unlink(path)
+
+
+def _ExtractClassFiles(changes, tmp_dir, class_inputs):
+  classes_list = []
+  for jar in class_inputs:
+    if changes:
+      changed_class_list = set(changes.IterChangedSubpaths(jar))
+      predicate = lambda x: x in changed_class_list and x.endswith('.class')
     else:
-      # Output to a .dex file.
-      if len(dex_files) > 1:
-        raise Exception('%d files created, expected 1' % len(dex_files))
-      tmp_dex_output = dex_files[0]
+      predicate = lambda x: x.endswith('.class')
+
+    classes_list.extend(
+        build_utils.ExtractAll(jar, path=tmp_dir, predicate=predicate))
+  return classes_list
 
-    if 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)
+def _CreateIntermediateDexFiles(changes, options, tmp_dir, dex_cmd):
+  # Create temporary directory for classes to be extracted to.
+  tmp_extract_dir = os.path.join(tmp_dir, 'tmp_extract_dir')
+  os.mkdir(tmp_extract_dir)
+
+  # Check whether changes were to a non-jar file, requiring full re-dex.
+  # E.g. r8.jar updated.
+  rebuild_all = changes.HasStringChanges() or not all(
+      p.endswith('.jar') for p in changes.IterChangedPaths())
+
+  if rebuild_all:
+    changes = None
+  class_files = _ExtractClassFiles(changes, tmp_extract_dir,
+                                   options.class_inputs)
+
+  # 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)
+
+
+def _OnStaleMd5(changes, options, final_dex_inputs, dex_cmd):
+  with build_utils.TempDir() as tmp_dir:
+    if options.incremental_dir:
+      # Create directory for all intermediate dex files.
+      if not os.path.exists(options.incremental_dir):
+        os.makedirs(options.incremental_dir)
+
+      _DeleteStaleIncrementalDexFiles(options.incremental_dir, final_dex_inputs)
+      _CreateIntermediateDexFiles(changes, options, tmp_dir, dex_cmd)
+
+    _CreateFinalDex(options, final_dex_inputs, tmp_dir, dex_cmd)
 
 
 def main(args):
+  logging.basicConfig(
+      level=logging.INFO,
+      format='%(levelname).1s %(relativeCreated)6d %(message)s')
   options = _ParseArgs(args)
 
-  input_paths = list(options.inputs)
+  options.class_inputs += options.class_inputs_filearg
+  options.dex_inputs += options.dex_inputs_filearg
+
+  input_paths = options.class_inputs + options.dex_inputs
   if options.multi_dex and options.main_dex_list_path:
     input_paths.append(options.main_dex_list_path)
+  input_paths.append(options.r8_jar_path)
 
-  _PerformDexing(options)
+  output_paths = [options.output]
+
+  if options.incremental_dir:
+    final_dex_inputs = _IntermediateDexFilePathsFromInputJars(
+        options.class_inputs, options.incremental_dir)
+    output_paths += final_dex_inputs
+  else:
+    final_dex_inputs = list(options.class_inputs)
+  final_dex_inputs += options.dex_inputs
+
+  dex_cmd = ['java', '-jar', options.r8_jar_path, 'd8', '--no-desugaring']
+  if options.release:
+    dex_cmd += ['--release']
+  if options.min_api:
+    dex_cmd += ['--min-api', options.min_api]
 
-  build_utils.WriteDepfile(
-      options.depfile, options.output, input_paths, add_pydeps=False)
+  build_utils.CallAndWriteDepfileIfStale(
+      lambda changes: _OnStaleMd5(changes, options, final_dex_inputs, dex_cmd),
+      options,
+      depfile_deps=options.class_inputs_filearg + options.dex_inputs_filearg,
+      output_paths=output_paths,
+      input_paths=input_paths,
+      input_strings=dex_cmd + [bool(options.incremental_dir)],
+      pass_changes=True,
+      add_pydeps=False)
 
 
 if __name__ == '__main__':

build/android/gyp/main_dex_list.py

@@ -28,26 +28,25 @@ def main(args):
                            'main dex.')
   parser.add_argument('--main-dex-list-path', required=True,
                       help='The main dex list file to generate.')
-  parser.add_argument('--inputs',
-                      help='JARs for which a main dex list should be '
-                           'generated.')
+  parser.add_argument(
+      '--class-inputs',
+      action='append',
+      help='GN-list of .jars with .class files.')
+  parser.add_argument(
+      '--class-inputs-filearg',
+      action='append',
+      help='GN-list of .jars with .class files (added to depfile).')
   parser.add_argument(
       '--r8-path', required=True, help='Path to the r8 executable.')
   parser.add_argument('--negative-main-dex-globs',
       help='GN-list of globs of .class names (e.g. org/chromium/foo/Bar.class) '
            'that will fail the build if they match files in the main dex.')
 
-  parser.add_argument('paths', nargs='*', default=[],
-                      help='JARs for which a main dex list should be '
-                           'generated.')
-
   args = parser.parse_args(build_utils.ExpandFileArgs(args))
 
-  depfile_deps = []
-  if args.inputs:
-    args.inputs = build_utils.ParseGnList(args.inputs)
-    depfile_deps = args.inputs
-    args.paths.extend(args.inputs)
+  args.class_inputs = build_utils.ParseGnList(args.class_inputs)
+  args.class_inputs_filearg = build_utils.ParseGnList(args.class_inputs_filearg)
+  args.class_inputs += args.class_inputs_filearg
 
   if args.negative_main_dex_globs:
     args.negative_main_dex_globs = build_utils.ParseGnList(
@@ -82,7 +81,7 @@ def main(args):
     '--disable-annotation-resolution-workaround',
   ]
 
-  input_paths = list(args.paths)
+  input_paths = list(args.class_inputs)
   input_paths += [
     args.shrinked_android_path,
     args.dx_path,
@@ -106,8 +105,8 @@ def main(args):
   ]
 
   def _LineLengthHelperForOnStaleMd5():
-    _OnStaleMd5(proguard_cmd, proguard_flags, main_dex_list_cmd, args.paths,
-                args.main_dex_list_path)
+    _OnStaleMd5(proguard_cmd, proguard_flags, main_dex_list_cmd,
+                args.class_inputs, args.main_dex_list_path)
 
   build_utils.CallAndWriteDepfileIfStale(
       _LineLengthHelperForOnStaleMd5,
@@ -115,7 +114,7 @@ def main(args):
       input_paths=input_paths,
       input_strings=input_strings,
       output_paths=output_paths,
-      depfile_deps=depfile_deps,
+      depfile_deps=args.class_inputs_filearg,
       add_pydeps=False)
 
   return 0

build/android/gyp/util/build_utils.py

@@ -70,7 +70,7 @@ def Touch(path, fail_if_missing=False):
     os.utime(path, None)
 
 
-def FindInDirectory(directory, filename_filter):
+def FindInDirectory(directory, filename_filter='*'):
   files = []
   for root, _dirnames, filenames in os.walk(directory):
     matched_files = fnmatch.filter(filenames, filename_filter)

build/android/gyp/util/md5_check.py

@@ -106,19 +106,20 @@ class Changes(object):
 
   def HasChanges(self):
     """Returns whether any changes exist."""
-    return (self.force or
-            not self.old_metadata or
-            self.old_metadata.StringsMd5() != self.new_metadata.StringsMd5() or
-            self.old_metadata.FilesMd5() != self.new_metadata.FilesMd5())
+    return (self.HasStringChanges()
+            or self.old_metadata.FilesMd5() != self.new_metadata.FilesMd5())
+
+  def HasStringChanges(self):
+    """Returns whether string metadata changed."""
+    return (self.force or not self.old_metadata
+            or self.old_metadata.StringsMd5() != self.new_metadata.StringsMd5())
 
   def AddedOrModifiedOnly(self):
     """Returns whether the only changes were from added or modified (sub)files.
 
     No missing outputs, no removed paths/subpaths.
     """
-    if (self.force or
-        not self.old_metadata or
-        self.old_metadata.StringsMd5() != self.new_metadata.StringsMd5()):
+    if self.HasStringChanges():
       return False
     if any(self.IterRemovedPaths()):
       return False

build/config/android/config.gni

@@ -230,17 +230,13 @@ if (is_android || is_chromeos) {
       android_sdk_platform_version = android_sdk_version
     }
 
-    # Speed up dexing using dx --incremental.
-    enable_incremental_dx = is_java_debug
+    # Reduce build time by using d8 incremental build.
+    enable_incremental_d8 = true
 
     # Use hashed symbol names to reduce JNI symbol overhead.
     use_hashed_jni_names = !is_java_debug
   }
 
-  # This should not be used for release builds since dx --incremental is known
-  # to not produce byte-for-byte identical output.
-  assert(!(enable_incremental_dx && !is_java_debug))
-
   # Path to where selected build variables are written to.
   android_build_vars = "$root_build_dir/build_vars.txt"
 

build/config/android/internal_rules.gni

@@ -1133,8 +1133,11 @@ if (enable_java_templates) {
       }
     }
 
-    assert(!(defined(invoker.input_jars) && _proguard_enabled),
-           "input_jars can't be specified when proguarding a dex.")
+    assert(!_proguard_enabled || !(defined(invoker.input_dex_filearg) ||
+                                       defined(invoker.input_dex_jars) ||
+                                       defined(invoker.input_classes_filearg) ||
+                                       defined(invoker.input_class_jars)),
+           "Cannot explicitly set inputs when proguarding a dex.")
 
     assert(!(defined(invoker.apply_mapping) && !_proguard_enabled),
            "apply_mapping can only be specified if proguard is enabled.")
@@ -1143,13 +1146,6 @@ if (enable_java_templates) {
       _main_dex_rules = "//build/android/main_dex_classes.flags"
     }
 
-    if (!_proguarding_with_r8) {
-      _dexing_jars = []
-      if (defined(invoker.input_jars)) {
-        _dexing_jars += invoker.input_jars
-      }
-    }
-
     if (_proguard_enabled) {
       if (_proguarding_with_r8) {
         _proguard_output_path = invoker.output
@@ -1158,7 +1154,6 @@ if (enable_java_templates) {
       } else {
         _proguard_output_path = invoker.output + ".proguard.jar"
         _proguard_target_name = "${target_name}__proguard"
-        _dexing_jars += [ _proguard_output_path ]
       }
 
       proguard(_proguard_target_name) {
@@ -1234,6 +1229,18 @@ if (enable_java_templates) {
     }
 
     if (!_proguarding_with_r8) {
+      _input_class_jars = []
+      if (defined(invoker.input_class_jars)) {
+        _input_class_jars = invoker.input_class_jars
+      }
+      if (_proguard_enabled) {
+        _input_class_jars += [ _proguard_output_path ]
+      }
+      if (_input_class_jars != []) {
+        _rebased_input_class_jars =
+            rebase_path(_input_class_jars, root_build_dir)
+      }
+
       if (_enable_main_dex_list) {
         _main_dex_list_path = invoker.output + ".main_dex_list"
         _main_dex_list_target_name = "${target_name}__main_dex_list"
@@ -1254,11 +1261,11 @@ if (enable_java_templates) {
           _dx = "$android_sdk_build_tools/lib/dx.jar"
           _r8 = "//third_party/r8/lib/r8.jar"
           inputs = [
-            _main_dex_rules,
-            _dx,
-            _r8,
-            _shrinked_android,
-          ]
+                     _main_dex_rules,
+                     _dx,
+                     _r8,
+                     _shrinked_android,
+                   ] + _input_class_jars
 
           outputs = [
             _main_dex_list_path,
@@ -1298,14 +1305,17 @@ if (enable_java_templates) {
             ]
           }
 
-          if (defined(invoker.input_jar_classpath)) {
+          if (defined(invoker.input_classes_filearg)) {
             inputs += [ invoker.build_config ]
-            args += [ "--inputs=@FileArg(${invoker.input_jar_classpath})" ]
-          } else {
-            inputs += _dexing_jars
-            if (_dexing_jars != []) {
-              args += rebase_path(_dexing_jars, root_build_dir)
-            }
+            args +=
+                [ "--class-inputs-filearg=${invoker.input_classes_filearg}" ]
+          }
+          if (defined(invoker.main_dex_list_input_classes_filearg)) {
+            inputs += [ invoker.build_config ]
+            args += [ "--class-inputs-filearg=${invoker.main_dex_list_input_classes_filearg}" ]
+          }
+          if (_input_class_jars != []) {
+            args += [ "--class-inputs=${_rebased_input_class_jars}" ]
           }
         }
       }
@@ -1332,6 +1342,12 @@ if (enable_java_templates) {
 
         if (_proguard_enabled) {
           deps += [ ":${_proguard_target_name}" ]
+        } else if (enable_incremental_d8) {
+          # Don't use incremental dexing for ProGuarded inputs as a precaution.
+          args += [
+            "--incremental-dir",
+            rebase_path("$target_out_dir/$target_name", root_build_dir),
+          ]
         }
 
         if (_enable_multidex) {
@@ -1346,14 +1362,23 @@ if (enable_java_templates) {
           }
         }
 
-        if (defined(invoker.input_dex_classpath)) {
+        if (defined(invoker.input_dex_filearg)) {
           inputs += [ invoker.build_config ]
-          args += [ "--input-list=@FileArg(${invoker.input_dex_classpath})" ]
+          args += [ "--dex-inputs-filearg=${invoker.input_dex_filearg}" ]
         }
-
-        inputs += _dexing_jars
-        if (_dexing_jars != []) {
-          args += rebase_path(_dexing_jars, root_build_dir)
+        if (defined(invoker.input_dex_jars) && invoker.input_dex_jars != []) {
+          inputs += invoker.input_dex_jars
+          _rebased_input_dex_jars =
+              rebase_path(invoker.input_dex_jars, root_build_dir)
+          args += [ "--dex-inputs=${_rebased_input_dex_jars}" ]
+        }
+        if (defined(invoker.input_classes_filearg)) {
+          inputs += [ invoker.build_config ]
+          args += [ "--class-inputs-filearg=${invoker.input_classes_filearg}" ]
+        }
+        if (_input_class_jars != []) {
+          inputs += _input_class_jars
+          args += [ "--class-inputs=${_rebased_input_class_jars}" ]
         }
 
         if (defined(invoker.dexlayout_profile)) {
@@ -3623,7 +3648,7 @@ if (enable_java_templates) {
 
         if (defined(_dex_path)) {
           dex("${target_name}__dex") {
-            input_jars = [ _final_jar_path ]
+            input_class_jars = [ _final_jar_path ]
             output = _dex_path
             deps = [
               ":$_process_prebuilt_target_name",

build/config/android/rules.gni

@@ -2909,12 +2909,10 @@ if (enable_java_templates) {
           deps += _deps + [ ":$_compile_resources_target" ]
           proguard_mapping_path = _proguard_mapping_path
         } else {
-          input_jars = [ _lib_dex_path ]
-          input_dex_classpath =
-              "${_rebased_build_config}:final_dex:dependency_dex_files"
+          input_dex_jars = [ _lib_dex_path ]
+          input_dex_filearg = "@FileArg(${_rebased_build_config}:final_dex:dependency_dex_files)"
           if (_enable_main_dex_list) {
-            input_jar_classpath =
-                "${_rebased_build_config}:deps_info:java_runtime_classpath"
+            main_dex_list_input_classes_filearg = "@FileArg(${_rebased_build_config}:deps_info:java_runtime_classpath)"
           }
         }