Commit 916e5d53 authored by Peter Wen's avatar Peter Wen Committed by Commit Bot

Android: Use turbine annotation processing

Previously, turbine ran annotation processors, javac ran them, and
errorprone also ran them.

This CL reuses the files that were generated when turbine ran annotation
processors so that javac and errorprone no longer ran annotation
processors.

Saves 4.6s on Base Java Signature Change (153.6s -> 149.0s).
More importantly unlocks javac sharding (since javac now depends on
turbine).

Bug: 906803
Change-Id: Ia8d7218587244d2f5908a5555a13dff3ed781507
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2133870
Commit-Queue: Peter Wen <wnwen@chromium.org>
Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Auto-Submit: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756033}
parent 565151ba
......@@ -9,9 +9,10 @@ import logging
import multiprocessing
import optparse
import os
import shutil
import re
import shutil
import sys
import time
import zipfile
from util import build_utils
......@@ -133,6 +134,9 @@ ERRORPRONE_WARNINGS_TO_TURN_OFF = [
'UnnecessaryAnonymousClass',
# Nice to have.
'LiteProtoToString',
# Must be off since we are now passing in annotation processor generated
# code as a source jar (deduplicating work with turbine).
'RefersToDaggerCodegen',
]
# Full list of checks: https://errorprone.info/bugpatterns
......@@ -455,12 +459,14 @@ def _RunCompiler(options, javac_cmd, java_files, classpath, jar_path,
logging.debug('Build command %s', cmd)
os.makedirs(classes_dir)
os.makedirs(annotation_processor_outputs_dir)
start = time.time()
build_utils.CheckOutput(
cmd,
print_stdout=options.chromium_code,
stdout_filter=ProcessJavacOutput,
stderr_filter=ProcessJavacOutput)
logging.info('Finished build command')
end = time.time() - start
logging.info('Java compilation took %ss', end)
if save_outputs:
annotation_processor_java_files = build_utils.FindInDirectory(
......@@ -650,6 +656,12 @@ def main(argv):
if options.processors:
javac_args.extend(['-processor', ','.join(options.processors)])
else:
# This effectively disables all annotation processors, even including
# annotation processors in service provider configuration files named
# META-INF/. See the following link for reference:
# https://docs.oracle.com/en/java/javase/11/tools/javac.html
javac_args.extend(['-proc:none'])
if options.bootclasspath:
javac_args.extend(['-bootclasspath', ':'.join(options.bootclasspath)])
......
......@@ -42,9 +42,10 @@ def _OnStaleMd5(options, cmd, javac_cmd, files, classpath):
# Use AtomicOutput so that output timestamps are not updated when outputs
# are not changed.
with build_utils.AtomicOutput(options.jar_path) as f:
cmd += ['--output', f.name]
logging.info('Command: %s' % ' '.join(cmd))
with build_utils.AtomicOutput(options.jar_path) as output_jar, \
build_utils.AtomicOutput(options.generated_jar_path) as generated_jar:
cmd += ['--output', output_jar.name, '--gensrc_output', generated_jar.name]
logging.debug('Command: %s', cmd)
start = time.time()
subprocess.check_call(cmd)
end = time.time() - start
......@@ -89,6 +90,10 @@ def main(argv):
action='append',
help='key=value arguments for the annotation processors.')
parser.add_argument('--jar-path', help='Jar output path.', required=True)
parser.add_argument(
'--generated-jar-path',
required=True,
help='Output path for generated source files.')
options, unknown_args = parser.parse_known_args(argv)
options.bootclasspath = build_utils.ParseGnList(options.bootclasspath)
......@@ -150,6 +155,7 @@ def main(argv):
output_paths = [
options.jar_path,
options.generated_jar_path,
]
input_strings = cmd + options.classpath + files
......
......@@ -2780,6 +2780,14 @@ if (enable_java_templates) {
_java_srcjars += [ "$_dep_gen_dir/$_dep_name.srcjar" ]
}
# generated_jar_path is an output when use_turbine and an input otherwise.
if (!invoker.use_turbine && defined(invoker.generated_jar_path)) {
_annotation_processing = false
_java_srcjars += [ invoker.generated_jar_path ]
} else {
_annotation_processing = true
}
_javac_args = []
if (defined(invoker.javac_args)) {
_javac_args = invoker.javac_args
......@@ -2820,15 +2828,22 @@ if (enable_java_templates) {
"--jar-path=$_rebased_output_jar_path",
"--java-srcjars=$_rebased_java_srcjars",
"--classpath=@FileArg($_rebased_build_config:deps_info:javac_full_interface_classpath)",
"--processorpath=@FileArg($_rebased_build_config:javac:processor_classpath)",
"--processors=@FileArg($_rebased_build_config:javac:processor_classes)",
]
if (_annotation_processing) {
args += [
"--processorpath=@FileArg($_rebased_build_config:javac:processor_classpath)",
"--processors=@FileArg($_rebased_build_config:javac:processor_classes)",
]
}
if (invoker.use_turbine) {
_turbine_jar_path = "//third_party/turbine/turbine.jar"
inputs += [ _turbine_jar_path ]
outputs += [ invoker.generated_jar_path ]
args += [
"--turbine-jar-path",
rebase_path(_turbine_jar_path, root_build_dir),
"--generated-jar-path",
rebase_path(invoker.generated_jar_path, root_build_dir),
]
}
......@@ -3193,6 +3208,7 @@ if (enable_java_templates) {
if (_has_sources) {
_javac_jar_path = "$target_gen_dir/$_main_target_name.javac.jar"
_generated_jar_path = "$target_gen_dir/$_main_target_name.generated.jar"
}
if (_is_prebuilt) {
......@@ -3437,6 +3453,17 @@ if (enable_java_templates) {
}
}
if (_has_sources || _is_prebuilt) {
_header_target_name = "${target_name}__header"
# TODO(wnwen): Enable turbine for non-chromium code when r8 optimizes out
# bridge methods.
_enable_turbine = _has_sources && _chromium_code
if (defined(invoker.enable_turbine)) {
_enable_turbine = invoker.enable_turbine
}
}
if (_has_sources) {
if (defined(invoker.enable_errorprone)) {
_enable_errorprone = invoker.enable_errorprone
......@@ -3464,8 +3491,11 @@ if (enable_java_templates) {
chromium_code = _chromium_code
supports_android = _supports_android
requires_android = _requires_android
deps = _java_header_deps + _accumulated_deps +
[ ":$_build_config_target_name" ]
if (!defined(deps)) {
deps = []
}
deps += _java_header_deps + _accumulated_deps +
[ ":$_build_config_target_name" ]
# android_apk and junit_binary pass R.java srcjars via srcjar_deps.
if (_type == "java_library" && _requires_android) {
......@@ -3483,11 +3513,25 @@ if (enable_java_templates) {
"jar_excluded_patterns",
"never_goma",
]
if (_enable_turbine) {
compile_java_helper(_header_target_name) {
forward_variables_from(invoker, _compile_java_forward_variables)
use_turbine = true
output_jar_path = _final_ijar_path
generated_jar_path = _generated_jar_path
}
}
_analysis_public_deps = []
_compile_java_target = "${_main_target_name}__compile_java"
compile_java_helper(_compile_java_target) {
forward_variables_from(invoker, _compile_java_forward_variables)
output_jar_path = _javac_jar_path
if (_enable_turbine) {
deps = [ ":$_header_target_name" ]
generated_jar_path = _generated_jar_path
}
}
if (_enable_errorprone) {
_compile_java_errorprone_target = "${_main_target_name}__errorprone"
......@@ -3500,6 +3544,10 @@ if (enable_java_templates) {
}
javac_args += invoker.errorprone_args
}
if (_enable_turbine) {
deps = [ ":$_header_target_name" ]
generated_jar_path = _generated_jar_path
}
output_jar_path = "$target_out_dir/$target_name.errorprone.stamp"
}
_analysis_public_deps += [ ":$_compile_java_errorprone_target" ]
......@@ -3538,19 +3586,8 @@ if (enable_java_templates) {
}
} # _has_sources
if (_is_prebuilt || _has_sources) {
_header_target_name = "${target_name}__header"
_enable_turbine = _has_sources && _chromium_code
if (defined(invoker.enable_turbine)) {
_enable_turbine = invoker.enable_turbine
}
if (_enable_turbine) {
compile_java_helper(_header_target_name) {
forward_variables_from(invoker, _compile_java_forward_variables)
use_turbine = true
output_jar_path = _final_ijar_path
}
} else {
if (_has_sources || _is_prebuilt) {
if (!_enable_turbine) {
generate_interface_jar(_header_target_name) {
# Always used the unfiltered .jar to create the interface jar so that
# other targets will resolve filtered classes when depending on
......@@ -3566,7 +3603,7 @@ if (enable_java_templates) {
}
}
_public_deps += [ ":$_header_target_name" ]
} # _is_prebuilt || _has_sources
}
if (defined(_final_jar_path)) {
if (_is_system_library) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment