Reland "Add -noverify to build commands to improve runtime (local builds only)"

This reverts commit 17e2a917.

Reason for revert: Ensure buildtools.py is a no-op on bots.

Original change's description:
> Revert "Add -noverify to build commands to improve runtime (local builds only)"
>
> This reverts commit 132e1deb.
>
> Reason for revert: Android ASAN bot compilation failures: https://ci.chromium.org/p/chromium/builders/ci/Android%20ASAN%20%28dbg%29/26489
>
> Original change's description:
> > Add -noverify to build commands to improve runtime (local builds only)
> >
> > The flag is guarded by treat_warnings_as_errors=false so that it
> > will apply only to local developer builds.
> >
> > Results from //tools/android/build_speed/benchmark.py base_java_sig:
> > With change:
> > base_java_sig: 68.0s avg (68.8s, 67.2s)
> > base_java_sig: 68.4s avg (68.6s, 68.2s)
> >
> > Without change:
> > base_java_sig: 69.4s avg (69.3s, 69.5s)
> > base_java_sig: 69.7s avg (69.7s, 69.7s)
> >
> > time out/Debug/bin/run_base_junit_tests:
> > With Change:
> > real 0m22.741s
> > real 0m23.543s
> >
> > Without change:
> > real 0m23.176s
> > real 0m24.328s
> >
> > Bug: 1117222
> > Change-Id: I4bfd742165468bf9538e6fa0dc8f35daa9e0f23f
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2313299
> > Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> > Auto-Submit: Andrew Grieve <agrieve@chromium.org>
> > Reviewed-by: Peter Wen <wnwen@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#799285}
>
> TBR=wnwen@chromium.org,agrieve@chromium.org
>
> Change-Id: I82f411e5dab89c361a30c33e9103bd8561f19765
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1117222
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362493
> Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
> Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#799471}

Bug: 1117222
Change-Id: Ib43bfe72869def7da43b237360df0e155149a963
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2363975Reviewed-by: Peter Wen <wnwen@chromium.org>
Commit-Queue: Peter Wen <wnwen@chromium.org>
Auto-Submit: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800233}

build/android/gyp/apkbuilder.py

@@ -119,6 +119,9 @@ def _ParseArgs(args):
       '--library-renames',
       action='append',
       help='The list of library files that we prepend crazy. to their names.')
+  parser.add_argument('--warnings-as-errors',
+                      action='store_true',
+                      help='Treat all warnings as errors.')
   diff_utils.AddCommandLineFlags(parser)
   options = parser.parse_args(args)
   options.assets = build_utils.ParseGnList(options.assets)
@@ -536,9 +539,15 @@ def main(args):
 
     if options.format == 'apk':
       zipalign_path = None if fast_align else options.zipalign_path
-      finalize_apk.FinalizeApk(options.apksigner_jar, zipalign_path, f.name,
-                               f.name, options.key_path, options.key_passwd,
-                               options.key_name, int(options.min_sdk_version))
+      finalize_apk.FinalizeApk(options.apksigner_jar,
+                               zipalign_path,
+                               f.name,
+                               f.name,
+                               options.key_path,
+                               options.key_passwd,
+                               options.key_name,
+                               int(options.min_sdk_version),
+                               warnings_as_errors=options.warnings_as_errors)
     logging.debug('Moving file into place')
 
     if options.depfile:

build/android/gyp/bundletool.py

@@ -24,11 +24,16 @@ BUNDLETOOL_VERSION = '0.13.3'
 BUNDLETOOL_JAR_PATH = os.path.join(
     BUNDLETOOL_DIR, 'bundletool-all-%s.jar' % BUNDLETOOL_VERSION)
 
-def RunBundleTool(args):
-  args = [build_utils.JAVA_PATH, '-jar', BUNDLETOOL_JAR_PATH] + args
-  logging.debug(' '.join(args))
+
+def RunBundleTool(args, warnings_as_errors=()):
+  # Use () instead of None because command-line flags are None by default.
+  verify = warnings_as_errors == () or warnings_as_errors
+  cmd = build_utils.JavaCmd(verify)
+  cmd += ['-jar', BUNDLETOOL_JAR_PATH]
+  cmd += args
+  logging.debug(' '.join(cmd))
   return build_utils.CheckOutput(
-      args,
+      cmd,
       print_stderr=True,
       fail_on_output=False,
       stderr_filter=build_utils.FilterReflectiveAccessJavaWarnings)

build/android/gyp/compile_java.py

@@ -395,8 +395,7 @@ def _OnStaleMd5(options, javac_cmd, javac_args, java_files):
       raise Exception('--enable-kythe-annotations requires '
                       'KYTHE_ROOT_DIRECTORY and KYTHE_OUTPUT_DIRECTORY '
                       'environment variables to be set.')
-    javac_extractor_cmd = [
-        build_utils.JAVA_PATH,
+    javac_extractor_cmd = build_utils.JavaCmd() + [
         '-jar',
         _JAVAC_EXTRACTOR,
     ]

build/android/gyp/create_app_bundle.py

@@ -98,6 +98,9 @@ def _ParseArgs(args):
       'listed there _and_ in --base-module-rtxt-path will '
       'be kept in the base bundle module, even if language'
       ' splitting is enabled.')
+  parser.add_argument('--warnings-as-errors',
+                      action='store_true',
+                      help='Treat all warnings as errors.')
 
   parser.add_argument('--keystore-path', help='Keystore path')
   parser.add_argument('--keystore-password', help='Keystore password')
@@ -418,17 +421,21 @@ def main(args):
     with open(tmp_bundle_config, 'w') as f:
       f.write(bundle_config)
 
-    cmd_args = [
-        build_utils.JAVA_PATH, '-jar', bundletool.BUNDLETOOL_JAR_PATH,
-        'build-bundle', '--modules=' + ','.join(module_zips),
-        '--output=' + tmp_unsigned_bundle, '--config=' + tmp_bundle_config
+    cmd_args = build_utils.JavaCmd(options.warnings_as_errors) + [
+        '-jar',
+        bundletool.BUNDLETOOL_JAR_PATH,
+        'build-bundle',
+        '--modules=' + ','.join(module_zips),
+        '--output=' + tmp_unsigned_bundle,
+        '--config=' + tmp_bundle_config,
     ]
 
     build_utils.CheckOutput(
         cmd_args,
         print_stdout=True,
         print_stderr=True,
-        stderr_filter=build_utils.FilterReflectiveAccessJavaWarnings)
+        stderr_filter=build_utils.FilterReflectiveAccessJavaWarnings,
+        fail_on_output=options.warnings_as_errors)
 
     if options.keystore_path:
       # NOTE: As stated by the public documentation, apksigner cannot be used
@@ -443,7 +450,9 @@ def main(args):
           tmp_unsigned_bundle,
           options.key_name,
       ]
-      build_utils.CheckOutput(signing_cmd_args, print_stderr=True)
+      build_utils.CheckOutput(signing_cmd_args,
+                              print_stderr=True,
+                              fail_on_output=options.warnings_as_errors)
 
     shutil.move(tmp_bundle, options.out_bundle)
 

build/android/gyp/create_java_binary_script.py

@@ -91,7 +91,8 @@ def main(argv):
 
   run_dir = os.path.dirname(options.output)
   classpath = [os.path.relpath(p, run_dir) for p in classpath]
-  java_path = os.path.relpath(build_utils.JAVA_PATH, run_dir)
+  java_path = os.path.relpath(
+      os.path.join(build_utils.JAVA_HOME, 'bin', 'java'), run_dir)
 
   with build_utils.AtomicOutput(options.output) as script:
     script.write(

build/android/gyp/desugar.py

@@ -25,13 +25,15 @@ def main():
                       help='Classpath.')
   parser.add_argument('--bootclasspath', required=True,
                       help='Path to javac bootclasspath interface jar.')
+  parser.add_argument('--warnings-as-errors',
+                      action='store_true',
+                      help='Treat all warnings as errors.')
   options = parser.parse_args(args)
 
   options.bootclasspath = build_utils.ParseGnList(options.bootclasspath)
   options.classpath = build_utils.ParseGnList(options.classpath)
 
-  cmd = [
-      build_utils.JAVA_PATH,
+  cmd = build_utils.JavaCmd(options.warnings_as_errors) + [
       '-jar',
       options.desugar_jar,
       '--input',
@@ -50,7 +52,8 @@ def main():
   build_utils.CheckOutput(
       cmd,
       print_stdout=False,
-      stderr_filter=build_utils.FilterReflectiveAccessJavaWarnings)
+      stderr_filter=build_utils.FilterReflectiveAccessJavaWarnings,
+      fail_on_output=options.warnings_as_errors)
 
   if options.depfile:
     build_utils.WriteDepfile(options.depfile,

build/android/gyp/dex.py

@@ -472,9 +472,7 @@ def _OnStaleMd5(changes, options, final_dex_inputs, dex_cmd):
 
 
 def MergeDexForIncrementalInstall(r8_jar_path, src_paths, dest_dex_jar):
-  dex_cmd = [
-      build_utils.JAVA_PATH,
-      '-Xmx1G',
+  dex_cmd = build_utils.JavaCmd(verify=False) + [
       '-cp',
       r8_jar_path,
       'com.android.tools.r8.D8',
@@ -509,9 +507,7 @@ def main(args):
     track_subpaths_allowlist = None
   final_dex_inputs += options.dex_inputs
 
-  dex_cmd = [
-      build_utils.JAVA_PATH,
-      '-Xmx1G',
+  dex_cmd = build_utils.JavaCmd(options.warnings_as_errors) + [
       '-cp',
       options.r8_jar_path,
       'com.android.tools.r8.D8',

build/android/gyp/dex_jdk_libs.py

@@ -39,9 +39,7 @@ def DexJdkLibJar(r8_path, min_api, desugar_jdk_libs_json, desugar_jdk_libs_jar,
                  warnings_as_errors):
   # TODO(agrieve): Spews a lot of stderr about missing classes.
   with build_utils.TempDir() as tmp_dir:
-    cmd = [
-        build_utils.JAVA_PATH,
-        '-Xmx1G',
+    cmd = build_utils.JavaCmd(warnings_as_errors) + [
         '-cp',
         r8_path,
         'com.android.tools.r8.L8',

build/android/gyp/dexsplitter.py

@@ -48,8 +48,7 @@ def _ParseOptions(args):
 
 
 def _RunDexsplitter(options, output_dir):
-  cmd = [
-      build_utils.JAVA_PATH,
+  cmd = build_utils.JavaCmd() + [
       '-cp',
       options.r8_path,
       'com.android.tools.r8.dexsplitter.DexSplitter',

build/android/gyp/finalize_apk.py

@@ -12,24 +12,32 @@ import tempfile
 from util import build_utils
 
 
-def FinalizeApk(apksigner_path, zipalign_path, unsigned_apk_path,
-                final_apk_path, key_path, key_passwd, key_name,
-                min_sdk_version):
+def FinalizeApk(apksigner_path,
+                zipalign_path,
+                unsigned_apk_path,
+                final_apk_path,
+                key_path,
+                key_passwd,
+                key_name,
+                min_sdk_version,
+                warnings_as_errors=False):
   # Use a tempfile so that Ctrl-C does not leave the file with a fresh mtime
   # and a corrupted state.
   with tempfile.NamedTemporaryFile() as staging_file:
     if zipalign_path:
       # v2 signing requires that zipalign happen first.
       logging.debug('Running zipalign')
-      subprocess.check_output([
+      zipalign_cmd = [
           zipalign_path, '-p', '-f', '4', unsigned_apk_path, staging_file.name
-      ])
+      ]
+      build_utils.CheckOutput(zipalign_cmd,
+                              print_stdout=True,
+                              fail_on_output=warnings_as_errors)
       signer_input_path = staging_file.name
     else:
       signer_input_path = unsigned_apk_path
 
-    sign_cmd = [
-        build_utils.JAVA_PATH,
+    sign_cmd = build_utils.JavaCmd(warnings_as_errors) + [
         '-jar',
         apksigner_path,
         'sign',
@@ -58,6 +66,8 @@ def FinalizeApk(apksigner_path, zipalign_path, unsigned_apk_path,
       sign_cmd += ['--min-sdk-version', '1']
 
     logging.debug('Signing apk')
-    subprocess.check_call(sign_cmd)
+    build_utils.CheckOutput(sign_cmd,
+                            print_stdout=True,
+                            fail_on_output=warnings_as_errors)
     shutil.move(staging_file.name, final_apk_path)
     staging_file.delete = False

build/android/gyp/jacoco_instr.py

@@ -199,8 +199,8 @@ def _RunInstrumentCommand(parser):
     source_files.extend(build_utils.ReadSourcesList(args.java_sources_file))
 
   with build_utils.TempDir() as temp_dir:
-    instrument_cmd = [
-        build_utils.JAVA_PATH, '-jar', args.jacococli_jar, 'instrument'
+    instrument_cmd = build_utils.JavaCmd() + [
+        '-jar', args.jacococli_jar, 'instrument'
     ]
 
     if not args.files_to_instrument:

build/android/gyp/main_dex_list.py

@@ -39,6 +39,9 @@ def _ParseArgs():
   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('--warnings-as-errors',
+                      action='store_true',
+                      help='Treat all warnings as errors.')
 
   args = parser.parse_args(build_utils.ExpandFileArgs(sys.argv[1:]))
 
@@ -54,9 +57,7 @@ def _ParseArgs():
 
 def main():
   args = _ParseArgs()
-  proguard_cmd = [
-      build_utils.JAVA_PATH,
-      '-Xmx1G',
+  proguard_cmd = build_utils.JavaCmd(args.warnings_as_errors) + [
       '-cp',
       args.r8_path,
       'com.android.tools.r8.R8',
@@ -96,7 +97,9 @@ def main():
         proguard_cmd += ['--pg-conf', proguard_flags_file.name]
         for injar in args.class_inputs:
           proguard_cmd.append(injar)
-        build_utils.CheckOutput(proguard_cmd, print_stderr=False)
+        build_utils.CheckOutput(proguard_cmd,
+                                print_stderr=False,
+                                fail_on_output=args.warnings_as_errors)
 
       # Record the classes kept by ProGuard. Not used by the build, but useful
       # for debugging what classes are kept by ProGuard vs. MainDexListBuilder.
@@ -107,8 +110,7 @@ def main():
 
       # Step 2: Expand inclusion list to all classes referenced by the .class
       # files of kept classes (non-recursive).
-      main_dex_list_cmd = [
-          build_utils.JAVA_PATH,
+      main_dex_list_cmd = build_utils.JavaCmd() + [
           '-cp',
           args.dx_path,
           'com.android.multidex.MainDexListBuilder',
@@ -119,7 +121,8 @@ def main():
           temp_jar.name,
           ':'.join(args.class_inputs)
       ]
-      main_dex_list = build_utils.CheckOutput(main_dex_list_cmd)
+      main_dex_list = build_utils.CheckOutput(
+          main_dex_list_cmd, fail_on_output=args.warnings_as_errors)
 
   except build_utils.CalledProcessError as e:
     if 'output jar is empty' in e.output:

build/android/gyp/merge_manifest.py

@@ -85,13 +85,15 @@ def main(argv):
   parser.add_argument(
       '--manifest-package',
       help='Package name of the merged AndroidManifest.xml.')
+  parser.add_argument('--warnings-as-errors',
+                      action='store_true',
+                      help='Treat all warnings as errors.')
   args = parser.parse_args(argv)
 
   classpath = _BuildManifestMergerClasspath(args.android_sdk_cmdline_tools)
 
   with build_utils.AtomicOutput(args.output) as output:
-    cmd = [
-        build_utils.JAVA_PATH,
+    cmd = build_utils.JavaCmd(args.warnings_as_errors) + [
         '-cp',
         classpath,
         _MANIFEST_MERGER_MAIN_CLASS,
@@ -123,11 +125,13 @@ def main(argv):
           '--property',
           'PACKAGE=' + package,
       ]
-      build_utils.CheckOutput(cmd,
-        # https://issuetracker.google.com/issues/63514300:
-        # The merger doesn't set a nonzero exit code for failures.
-        fail_func=lambda returncode, stderr: returncode != 0 or
-          build_utils.IsTimeStale(output.name, [root_manifest] + extras))
+      build_utils.CheckOutput(
+          cmd,
+          # https://issuetracker.google.com/issues/63514300:
+          # The merger doesn't set a nonzero exit code for failures.
+          fail_func=lambda returncode, stderr: returncode != 0 or build_utils.
+          IsTimeStale(output.name, [root_manifest] + extras),
+          fail_on_output=args.warnings_as_errors)
 
     # Check for correct output.
     _, manifest, _ = manifest_utils.ParseManifest(output.name)

build/android/gyp/proguard.py

@@ -251,14 +251,12 @@ def _OptimizeWithR8(options,
       base_dex_context = _DexPathContext('base', options.output_path,
                                          options.input_paths, tmp_output)
 
-    cmd = [
-        build_utils.JAVA_PATH,
+    cmd = build_utils.JavaCmd(options.warnings_as_errors) + [
         '-Dcom.android.tools.r8.allowTestProguardOptions=1',
     ]
     if options.disable_outlining:
-      cmd += [' -Dcom.android.tools.r8.disableOutlining=1']
+      cmd += ['-Dcom.android.tools.r8.disableOutlining=1']
     cmd += [
-        '-Xmx1G',
         '-cp',
         options.r8_path,
         'com.android.tools.r8.R8',

build/android/gyp/turbine.py

@@ -8,7 +8,6 @@ import argparse
 import logging
 import os
 import shutil
-import subprocess
 import sys
 import time
 
@@ -47,7 +46,9 @@ def _OnStaleMd5(options, cmd, javac_cmd, files, classpath):
     cmd += ['--output', output_jar.name, '--gensrc_output', generated_jar.name]
     logging.debug('Command: %s', cmd)
     start = time.time()
-    subprocess.check_call(cmd)
+    build_utils.CheckOutput(cmd,
+                            print_stdout=True,
+                            fail_on_output=options.warnings_as_errors)
     end = time.time() - start
     logging.info('Header compilation took %ss', end)
 
@@ -94,6 +95,9 @@ def main(argv):
       '--generated-jar-path',
       required=True,
       help='Output path for generated source files.')
+  parser.add_argument('--warnings-as-errors',
+                      action='store_true',
+                      help='Treat all warnings as errors.')
   options, unknown_args = parser.parse_known_args(argv)
 
   options.bootclasspath = build_utils.ParseGnList(options.bootclasspath)
@@ -108,9 +112,8 @@ def main(argv):
     if arg.startswith('@'):
       files.extend(build_utils.ReadSourcesList(arg[1:]))
 
-  cmd = [
-      build_utils.JAVA_PATH, '-classpath', options.turbine_jar_path,
-      'com.google.turbine.main.Main'
+  cmd = build_utils.JavaCmd(options.warnings_as_errors) + [
+      '-classpath', options.turbine_jar_path, 'com.google.turbine.main.Main'
   ]
   javac_cmd = []
 

build/android/gyp/util/build_utils.py

@@ -33,18 +33,32 @@ DIR_SOURCE_ROOT = os.path.relpath(
             os.path.dirname(__file__), os.pardir, os.pardir, os.pardir,
             os.pardir)))
 JAVA_HOME = os.path.join(DIR_SOURCE_ROOT, 'third_party', 'jdk', 'current')
-JAVA_PATH = os.path.join(JAVA_HOME, 'bin', 'java')
 JAVAC_PATH = os.path.join(JAVA_HOME, 'bin', 'javac')
 JAVAP_PATH = os.path.join(JAVA_HOME, 'bin', 'javap')
 RT_JAR_PATH = os.path.join(DIR_SOURCE_ROOT, 'third_party', 'jdk', 'extras',
                            'java_8', 'jre', 'lib', 'rt.jar')
 
+# TODO(agrieve): Remove once safe to do so.
+JAVA_PATH = os.path.join(JAVA_HOME, 'bin', 'java')
+
 try:
   string_types = basestring
 except NameError:
   string_types = (str, bytes)
 
 
+def JavaCmd(verify=True):
+  ret = [os.path.join(JAVA_HOME, 'bin', 'java')]
+  # Limit heap to 1GB of RAM.
+  ret += ['-Xmx1G']
+
+  # Disable bytecode verification for local builds gives a ~2% speed-up.
+  if not verify:
+    ret += ['-noverify']
+
+  return ret
+
+
 @contextlib.contextmanager
 def TempDir(**kwargs):
   dirname = tempfile.mkdtemp(**kwargs)

build/config/android/internal_rules.gni

@@ -1367,9 +1367,11 @@ if (enable_java_templates) {
           "--classpath",
           rebase_path("//third_party/jacoco/lib/jacocoagent.jar",
                       root_build_dir),
-          "--noverify",
         ]
       }
+      if (use_jacoco_coverage || !treat_warnings_as_errors) {
+        args += [ "--noverify" ]
+      }
       if (defined(invoker.wrapper_script_args)) {
         args += [ "--" ] + invoker.wrapper_script_args
       }
@@ -1608,6 +1610,9 @@ if (enable_java_templates) {
           if (_input_class_jars != []) {
             args += [ "--class-inputs=${_rebased_input_class_jars}" ]
           }
+          if (treat_warnings_as_errors) {
+            args += [ "--warnings-as-errors" ]
+          }
         }
       }
 
@@ -1996,6 +2001,9 @@ if (enable_java_templates) {
             "--depfile",
             rebase_path(depfile, root_build_dir),
           ]
+          if (treat_warnings_as_errors) {
+            args += [ "--warnings-as-errors" ]
+          }
         }
 
         _deps = []
@@ -2137,6 +2145,10 @@ if (enable_java_templates) {
       if (defined(invoker.max_sdk_version)) {
         args += [ "--max-sdk-version=${invoker.max_sdk_version}" ]
       }
+
+      if (treat_warnings_as_errors) {
+        args += [ "--warnings-as-errors" ]
+      }
     }
   }
 
@@ -2955,6 +2967,9 @@ if (enable_java_templates) {
     if (_secondary_native_lib_placeholders != []) {
       _args += [ "--secondary-native-lib-placeholders=$_secondary_native_lib_placeholders" ]
     }
+    if (treat_warnings_as_errors) {
+      _args += [ "--warnings-as-errors" ]
+    }
 
     if (defined(invoker.expected_libs_and_assets)) {
       _expectations_target =
@@ -4183,6 +4198,10 @@ template("create_android_app_bundle_module") {
     ]
   }
 
+  if (treat_warnings_as_errors) {
+    _args += [ "--warnings-as-errors" ]
+  }
+
   if (defined(invoker.expected_libs_and_assets)) {
     _expectations_target = "${invoker.top_target_name}_validate_libs_and_assets"
     action_with_pydeps(_expectations_target) {

build/config/android/rules.gni

@@ -4880,6 +4880,9 @@ if (enable_java_templates) {
           invoker.compress_shared_libraries) {
         args += [ "--compress-shared-libraries" ]
       }
+      if (treat_warnings_as_errors) {
+        args += [ "--warnings-as-errors" ]
+      }
 
       if (_enable_language_splits) {
         args += [