From 49c3876ba565ee57484f1fd12d945e8eea283e17 Mon Sep 17 00:00:00 2001 From: Sam Maier Date: Wed, 1 Feb 2023 23:11:26 +0000 Subject: [PATCH] JNI: make JNI Multiplexing work (behind flag) Now, the gn arg "allow_jni_multiplexing" turns on a functional, if not ideally optimized, jni multiplexing implementation. There is still some work that is needed to improve the multiplexing before we enable globally. Bug: 929682 Change-Id: Ia4275bc32fd2bbca58fb3c8ff4a712b0883446a8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3872586 Reviewed-by: Nate Fischer Commit-Queue: Sam Maier Reviewed-by: Andrew Grieve Auto-Submit: Sam Maier Commit-Queue: Nate Fischer Cr-Commit-Position: refs/heads/main@{#1100141} --- android_webview/BUILD.gn | 16 ++++++++++ android_webview/lib/BUILD.gn | 9 +++++- android_webview/lib/webview_entry_point.cc | 3 ++ android_webview/system_webview_apk_tmpl.gni | 3 ++ base/android/jni_generator/BUILD.gn | 8 +++-- .../jni_registration_generator.py | 2 ++ .../jni_generator/sample_entry_point.cc | 2 +- .../gyp/generate_linker_version_script.py | 8 ++++- build/config/android/config.gni | 6 +--- .../config/android/linker_version_script.gni | 4 +++ build/config/android/rules.gni | 31 ++++++++++-------- chrome/android/BUILD.gn | 32 ++++++++++++++++++- .../android/chrome_common_shared_library.gni | 6 +++- chrome/android/chrome_public_apk_tmpl.gni | 4 +++ .../modules/chrome_feature_module_tmpl.gni | 1 + .../modules/test_dummy/internal/BUILD.gn | 3 ++ .../browser/android/monochrome_entry_point.cc | 3 ++ chrome/browser/android/vr/BUILD.gn | 6 ++++ 18 files changed, 122 insertions(+), 25 deletions(-) diff --git a/android_webview/BUILD.gn b/android_webview/BUILD.gn index 6874afe51bd19e..b2a29ba7c37f8d 100644 --- a/android_webview/BUILD.gn +++ b/android_webview/BUILD.gn @@ -90,7 +90,9 @@ standalone_system_webview_apk_tmpl("system_webview_no_weblayer_apk") { [ "//third_party/androidx:androidx_recyclerview_recyclerview_java" ] } +_main_webview_bundle_target = ":system_webview_bundle" if (android_64bit_target_cpu && skip_secondary_abi_for_cq) { + _main_webview_bundle_target = ":system_webview_64_bundle" group("system_webview_bundle") { deps = [ ":system_webview_64_bundle" ] } @@ -389,6 +391,13 @@ if (android_64bit_target_cpu && !skip_secondary_abi_for_cq) { } } +if (allow_jni_multiplexing && current_toolchain == default_toolchain) { + generate_jni_registration("webview_jni_registration") { + targets = [ _main_webview_bundle_target ] + enable_jni_multiplexing = true + } +} + # The shared library used by standalone WebView. template("libwebviewchromium_tmpl") { shared_library(target_name) { @@ -399,6 +408,9 @@ template("libwebviewchromium_tmpl") { "//android_webview/nonembedded", "//third_party/blink/public:blink", ] + if (allow_jni_multiplexing) { + deps += [ ":webview_jni_registration($default_toolchain)" ] + } configs -= [ "//build/config/android:hide_all_but_jni_onload" ] configs += [ "//build/config/android:hide_all_but_jni", @@ -430,6 +442,10 @@ template("webview_alternate_library") { "//android_webview/lib:webview_entry_point", "//android_webview/nonembedded", ] + if (allow_jni_multiplexing) { + deps += [ ":webview_jni_registration($default_toolchain)" ] + enable_jni_multiplexing = true + } is_webview = true } } diff --git a/android_webview/lib/BUILD.gn b/android_webview/lib/BUILD.gn index 82e18fe87538b8..680531d1328d98 100644 --- a/android_webview/lib/BUILD.gn +++ b/android_webview/lib/BUILD.gn @@ -2,6 +2,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import("//build/config/android/config.gni") import("//components/gwp_asan/buildflags/buildflags.gni") import("//components/spellcheck/spellcheck_build_features.gni") import("//weblayer/variables.gni") @@ -68,9 +69,15 @@ source_set("webview_entry_point") { "//base", ] sources = [ "webview_entry_point.cc" ] + defines = [] + + if (allow_jni_multiplexing) { + defines += [ "JNI_REGISTRATION_REQUIRED" ] + deps += [ "//android_webview:webview_jni_registration($default_toolchain)" ] + } if (webview_includes_weblayer) { - defines = [ "WEBVIEW_INCLUDES_WEBLAYER" ] + defines += [ "WEBVIEW_INCLUDES_WEBLAYER" ] deps += [ "//weblayer:weblayer_lib_webview" ] } } diff --git a/android_webview/lib/webview_entry_point.cc b/android_webview/lib/webview_entry_point.cc index c6e1ac2efd7d20..221fbcec131b5a 100644 --- a/android_webview/lib/webview_entry_point.cc +++ b/android_webview/lib/webview_entry_point.cc @@ -7,6 +7,9 @@ #include "base/android/jni_android.h" #include "base/android/library_loader/library_loader_hooks.h" +#if defined(JNI_REGISTRATION_REQUIRED) +#include "android_webview/webview_jni_registration_generated.h" +#endif #if defined(WEBVIEW_INCLUDES_WEBLAYER) #include "weblayer/app/jni_onload.h" #include "weblayer/browser/web_view_compatibility_helper_impl.h" diff --git a/android_webview/system_webview_apk_tmpl.gni b/android_webview/system_webview_apk_tmpl.gni index d79146c388461c..26ad7a1245925c 100644 --- a/android_webview/system_webview_apk_tmpl.gni +++ b/android_webview/system_webview_apk_tmpl.gni @@ -101,6 +101,9 @@ template("system_webview_apk_or_module_tmpl") { if (!_omit_dex) { product_config_java_packages = [ invoker.webview_product_config_java_package ] + if (allow_jni_multiplexing) { + enable_jni_multiplexing = true + } } if (_webview_includes_weblayer) { diff --git a/base/android/jni_generator/BUILD.gn b/base/android/jni_generator/BUILD.gn index 1b9fba1ece28e3..7145fb448a8e2f 100644 --- a/base/android/jni_generator/BUILD.gn +++ b/base/android/jni_generator/BUILD.gn @@ -48,12 +48,17 @@ source_set("jni_sample_native_side") { ] } +generate_jni_registration("jni_registration") { + targets = [ ":jni_sample_java" ] + manual_jni_registration = true +} + shared_library("jni_sample_lib") { sources = [ "sample_entry_point.cc" ] deps = [ + ":jni_registration", ":jni_sample_native_side", - ":sample_jni_apk__final_jni", # For registration_header "//base", ] } @@ -63,7 +68,6 @@ android_apk("sample_jni_apk") { android_manifest = "AndroidManifest.xml" deps = [ ":jni_sample_java" ] shared_libraries = [ ":jni_sample_lib" ] - manual_jni_registration = true } # Serves to test that generated bindings compile properly. diff --git a/base/android/jni_generator/jni_registration_generator.py b/base/android/jni_generator/jni_registration_generator.py index 8f9f21856709f4..f5563ed7b3b4b6 100755 --- a/base/android/jni_generator/jni_registration_generator.py +++ b/base/android/jni_generator/jni_registration_generator.py @@ -706,6 +706,8 @@ def _AddCases(self): params = _GetParamsListForMultiplex(signature[1], with_types=False) values = { 'SWITCH_NUM': native.switch_num, + # We are forced to call the generated stub instead of the impl because + # the impl is not guaranteed to have a globally unique name. 'STUB_NAME': self.helper.GetStubName(native), 'PARAMS': params, } diff --git a/base/android/jni_generator/sample_entry_point.cc b/base/android/jni_generator/sample_entry_point.cc index 91fbdf0c03c977..5a7eb48b711a97 100644 --- a/base/android/jni_generator/sample_entry_point.cc +++ b/base/android/jni_generator/sample_entry_point.cc @@ -3,7 +3,7 @@ // found in the LICENSE file. #include "base/android/jni_android.h" -#include "base/android/jni_generator/sample_jni_apk__final_jni_generated.h" +#include "base/android/jni_generator/jni_registration_generated.h" #include "base/android/jni_utils.h" // This is called by the VM when the shared library is first loaded. diff --git a/build/android/gyp/generate_linker_version_script.py b/build/android/gyp/generate_linker_version_script.py index dd5d604a3cd864..28e58095cf1aeb 100755 --- a/build/android/gyp/generate_linker_version_script.py +++ b/build/android/gyp/generate_linker_version_script.py @@ -35,6 +35,10 @@ def main(): '--export-java-symbols', action='store_true', help='Export Java_* JNI methods') + parser.add_argument( + '--jni-multiplexing', + action='store_true', + help='Export only the JNI methods generated by multiplexing') parser.add_argument('--export-fortesting-java-symbols', action='store_true', help='Export Java_*_ForTesting JNI methods') @@ -61,7 +65,9 @@ def main(): symbol_list = ['CrashpadHandlerMain', 'JNI_OnLoad'] if options.export_java_symbols: - if options.export_fortesting_java_symbols: + if options.jni_multiplexing: + symbol_list.append('Java_*_resolve_1for_*') + elif options.export_fortesting_java_symbols: symbol_list.append('Java_*') else: # The linker uses unix shell globbing patterns, not regex. So, we have to diff --git a/build/config/android/config.gni b/build/config/android/config.gni index 903ccec4ea6e15..880357f7e3de9e 100644 --- a/build/config/android/config.gni +++ b/build/config/android/config.gni @@ -291,7 +291,7 @@ if (is_android || is_chromeos) { use_hashed_jni_names = !is_java_debug # Enables JNI multiplexing to reduce JNI native methods overhead. - enable_jni_multiplexing = false + allow_jni_multiplexing = false # Enables trace event injection on Android views with bytecode rewriting. # This adds an additional step on android_app_bundle_module targets that @@ -300,10 +300,6 @@ if (is_android || is_chromeos) { !is_java_debug && android_channel != "stable" } - if (enable_jni_multiplexing) { - use_hashed_jni_names = false - } - # Host stuff ----------------------------------------------------------------- # Defines the name the Android build gives to the current host CPU diff --git a/build/config/android/linker_version_script.gni b/build/config/android/linker_version_script.gni index 5d14f7e169af64..7dab72227bf511 100644 --- a/build/config/android/linker_version_script.gni +++ b/build/config/android/linker_version_script.gni @@ -2,6 +2,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import("//build/config/android/config.gni") import("//build/config/python.gni") # Generate a custom linker version script that can later be used with @@ -27,6 +28,9 @@ template("generate_linker_version_script") { if (defined(invoker.testonly) && invoker.testonly) { args += [ "--export-fortesting-java-symbols" ] } + if (allow_jni_multiplexing) { + args += [ "--jni-multiplexing" ] + } } if (defined(invoker.export_feature_registrations) && diff --git a/build/config/android/rules.gni b/build/config/android/rules.gni index b1e49661be473a..d969956fd1afee 100644 --- a/build/config/android/rules.gni +++ b/build/config/android/rules.gni @@ -222,7 +222,9 @@ if (enable_java_templates) { if (!is_robolectric && use_hashed_jni_names) { args += [ "--use_proxy_hash" ] } - if (!is_robolectric && enable_jni_multiplexing) { + + if (!is_robolectric && defined(invoker.enable_jni_multiplexing) && + invoker.enable_jni_multiplexing) { args += [ "--enable_jni_multiplexing" ] } if (defined(invoker.namespace)) { @@ -450,16 +452,20 @@ if (enable_java_templates && is_android) { _manual_jni_registration = defined(invoker.manual_jni_registration) && invoker.manual_jni_registration + _enable_jni_multiplexing = defined(invoker.enable_jni_multiplexing) && + invoker.enable_jni_multiplexing + if (_manual_jni_registration) { + args += [ "--manual-jni-registration" ] + } + if (_enable_jni_multiplexing) { + args += [ "--enable-jni-multiplexing" ] + } - if (_manual_jni_registration || enable_jni_multiplexing) { + if ((!defined(invoker.prevent_header_output) || + !invoker.prevent_header_output) && + (_manual_jni_registration || _enable_jni_multiplexing)) { assert(current_toolchain == default_toolchain, "We do not need >1 toolchain copies of the same header.") - if (_manual_jni_registration) { - args += [ "--manual-jni-registration" ] - } - if (enable_jni_multiplexing) { - args += [ "--enable-jni-multiplexing" ] - } _subdir = rebase_path(target_gen_dir, root_gen_dir) _jni_header_output = @@ -2389,8 +2395,6 @@ if (enable_java_templates && is_android) { # generated is solely controlled by this flag. Otherwise, the default behavior # is NativeLibraries.java will only be generated for the base module/apk when # its `shared_libraries` is not empty. - # manual_jni_registration: If true, causes the ${target_name}__final_jni target - # to additionally output a header file for use with manual JNI registration. # jni_file_exclusions: List of source path to exclude from the # final_jni step. # aapt_locale_allowlist: If set, all locales not in this list will be @@ -3028,8 +3032,8 @@ if (enable_java_templates && is_android) { generate_jni_registration("${_template_name}__final_jni") { forward_variables_from(invoker, [ + "enable_jni_multiplexing", "enable_native_mocks", - "manual_jni_registration", "require_native_mocks", ]) if (defined(invoker.bundle_target)) { @@ -3040,6 +3044,7 @@ if (enable_java_templates && is_android) { if (defined(invoker.jni_file_exclusions)) { file_exclusions = invoker.jni_file_exclusions } + prevent_header_output = true } _srcjar_deps += [ ":${_template_name}__final_jni" ] } else { @@ -3561,6 +3566,7 @@ if (enable_java_templates && is_android) { "data_deps", "deps", "enable_lint", + "enable_jni_multiplexing", "enable_multidex", "enable_native_mocks", "enable_proguard_checks", @@ -3586,7 +3592,6 @@ if (enable_java_templates && is_android) { "lint_suppressions_file", "loadable_modules", "manifest_package", - "manual_jni_registration", "max_sdk_version", "mergeable_android_manifests", "product_config_java_packages", @@ -3698,6 +3703,7 @@ if (enable_java_templates && is_android) { "data", "data_deps", "deps", + "enable_jni_multiplexing", "enable_multidex", "expected_android_manifest", "expected_android_manifest_base", @@ -3717,7 +3723,6 @@ if (enable_java_templates && is_android) { "product_config_java_packages", "main_component_library", "manifest_package", - "manual_jni_registration", "max_sdk_version", "min_sdk_version", "mergeable_android_manifests", diff --git a/chrome/android/BUILD.gn b/chrome/android/BUILD.gn index 7430efcf9ede94..5375340a8ea20e 100644 --- a/chrome/android/BUILD.gn +++ b/chrome/android/BUILD.gn @@ -2128,6 +2128,9 @@ if (current_toolchain == default_toolchain) { targets = [ ":chrome_public_base_module_java" ] manual_jni_registration = true file_exclusions = chrome_jni_file_exclusions + if (allow_jni_multiplexing) { + enable_jni_multiplexing = true + } } # The test apks do not use chromium linker, but using manual JNI registration @@ -2137,6 +2140,20 @@ if (current_toolchain == default_toolchain) { targets = [ ":chrome_public_base_module_java_for_test" ] manual_jni_registration = true file_exclusions = chrome_jni_file_exclusions + if (allow_jni_multiplexing) { + enable_jni_multiplexing = true + } + } + + if (allow_jni_multiplexing) { + generate_jni_registration("monochrome_jni_registration") { + targets = [ + ":monochrome_java", + ":chrome_all_java", + ] + file_exclusions = chrome_jni_file_exclusions + enable_jni_multiplexing = true + } } # This template instantiates targets responsible for generating pak @@ -4006,6 +4023,9 @@ template("libchrome_impl") { chrome_common_shared_library(target_name) { sources = [ "../browser/android/chrome_entry_point.cc" ] deps = [ ":chrome_jni_registration($default_toolchain)" ] + if (allow_jni_multiplexing) { + enable_jni_multiplexing = true + } if (defined(invoker.deps)) { deps += invoker.deps } @@ -4063,6 +4083,9 @@ chrome_common_shared_library("libchromefortest") { if (enable_vr) { deps += [ "//chrome/browser/android/vr:test_support" ] } + if (allow_jni_multiplexing) { + enable_jni_multiplexing = true + } # Make this a partitioned library, since some partitioned code is linked in # (otherwise, the library will warn at build time that it contains multiple @@ -4085,11 +4108,18 @@ template("libmonochrome_apk_or_bundle_tmpl") { deps += invoker.deps } + defines = [] if (webview_includes_weblayer) { - defines = [ "WEBVIEW_INCLUDES_WEBLAYER" ] + defines += [ "WEBVIEW_INCLUDES_WEBLAYER" ] deps += [ "//weblayer:weblayer_lib" ] } + if (allow_jni_multiplexing) { + defines += [ "JNI_REGISTRATION_REQUIRED" ] + deps += [ ":monochrome_jni_registration($default_toolchain)" ] + enable_jni_multiplexing = true + } + if (enable_vr) { deps += [ "//chrome/browser/android/vr:module_factory" ] } diff --git a/chrome/android/chrome_common_shared_library.gni b/chrome/android/chrome_common_shared_library.gni index f23200549c9b57..e3b03679515b35 100644 --- a/chrome/android/chrome_common_shared_library.gni +++ b/chrome/android/chrome_common_shared_library.gni @@ -120,7 +120,8 @@ template("chrome_common_shared_library") { # Handle VR JNI registration and dependencies. if (!_is_webview && enable_vr) { - if (_export_java_symbols) { + if (_export_java_symbols && (!defined(invoker.enable_jni_multiplexing) || + !invoker.enable_jni_multiplexing)) { # NOTE: While this file is named *_monochrome.cc, it just contains an # empty vr::RegisterJni() function that returns true. sources += [ "../browser/android/vr/register_jni_monochrome.cc" ] @@ -137,6 +138,9 @@ template("chrome_common_shared_library") { } } + # We check this variable last, so sometimes it isn't used to decide which registration to use. + not_needed([ "enable_jni_multiplexing" ]) + if (_generate_partitions) { partitions = [] foreach(_module_desc, _module_descs) { diff --git a/chrome/android/chrome_public_apk_tmpl.gni b/chrome/android/chrome_public_apk_tmpl.gni index c84f89fafae593..c403955b19239a 100644 --- a/chrome/android/chrome_public_apk_tmpl.gni +++ b/chrome/android/chrome_public_apk_tmpl.gni @@ -278,6 +278,10 @@ template("chrome_public_common_apk_or_module_tmpl") { custom_assertion_handler = crash_reporting_assertion_handler } + if (allow_jni_multiplexing) { + enable_jni_multiplexing = true + } + if (!defined(version_name)) { version_name = chrome_version_name } diff --git a/chrome/android/modules/chrome_feature_module_tmpl.gni b/chrome/android/modules/chrome_feature_module_tmpl.gni index 3f7cd69b77916a..ac61a2a788ca8e 100644 --- a/chrome/android/modules/chrome_feature_module_tmpl.gni +++ b/chrome/android/modules/chrome_feature_module_tmpl.gni @@ -91,6 +91,7 @@ template("chrome_feature_module") { "add_view_trace_events", "base_module_target", "custom_assertion_handler", + "enable_jni_multiplexing", "expected_android_manifest", "expected_android_manifest_base", "manifest_package", diff --git a/chrome/android/modules/test_dummy/internal/BUILD.gn b/chrome/android/modules/test_dummy/internal/BUILD.gn index 351f0258772dbe..617c54b5c4935b 100644 --- a/chrome/android/modules/test_dummy/internal/BUILD.gn +++ b/chrome/android/modules/test_dummy/internal/BUILD.gn @@ -42,5 +42,8 @@ if (current_toolchain == default_toolchain) { targets = [ "//chrome/browser/test_dummy/internal:java" ] manual_jni_registration = true namespace = "test_dummy" + if (allow_jni_multiplexing) { + enable_jni_multiplexing = true + } } } diff --git a/chrome/browser/android/monochrome_entry_point.cc b/chrome/browser/android/monochrome_entry_point.cc index 139658d3b58897..72d85f599682f3 100644 --- a/chrome/browser/android/monochrome_entry_point.cc +++ b/chrome/browser/android/monochrome_entry_point.cc @@ -9,6 +9,9 @@ #include "base/functional/bind.h" #include "chrome/app/android/chrome_jni_onload.h" +#if defined(JNI_REGISTRATION_REQUIRED) +#include "chrome/android/monochrome_jni_registration_generated.h" +#endif #if defined(WEBVIEW_INCLUDES_WEBLAYER) #include "weblayer/app/jni_onload.h" #endif diff --git a/chrome/browser/android/vr/BUILD.gn b/chrome/browser/android/vr/BUILD.gn index 3498789f90590c..8e37c64a1d71f4 100644 --- a/chrome/browser/android/vr/BUILD.gn +++ b/chrome/browser/android/vr/BUILD.gn @@ -166,6 +166,9 @@ if (current_toolchain == default_toolchain) { [ "//chrome/android:chrome_modern_public_bundle__vr_bundle_module" ] manual_jni_registration = true namespace = "vr" + if (allow_jni_multiplexing) { + enable_jni_multiplexing = true + } } generate_jni_registration("jni_registration_for_testing") { @@ -174,6 +177,9 @@ if (current_toolchain == default_toolchain) { manual_jni_registration = true namespace = "vr" testonly = true + if (allow_jni_multiplexing) { + enable_jni_multiplexing = true + } } }