Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Mono.Android] avoid System.Reflection.Emit usage for common calls (d…
…otnet#6657) Context: https://github.com/xamarin/xamarin-android/wiki/Blueprint#java-type-registration Context: b7a368a Context: dotnet#4877 Context: dotnet#4927 (comment) In order for Java code to call C# code, [`JNIEnv::RegisterNatives()][0] must be invoked, providing an array of `JNINativeMethod` structures, each of which contains a function pointer to invoke, kept in `JNINativeMethod::fnPtr`. Fortunately, delegates marshal as function pointers, and there is a bunch of `generator`-emitted infrastructure and coordination with Java Callable Wrappers to eventually obtain a Delegate instance to provide `JNIEnv::RegisterNatives()`. There is one deficiency in the `generator`-emitted infrastructure: it doesn't deal with C# exceptions. However, exceptions "can't" cross the JNI boundary (see b7a368a for an example of the breakage that results when exceptions do cross the boundary!), except when we *do* want exceptions to cross the JNI boundary ("improved" IDE first chance exception experience; see dotnet#4877). This "we want to catch exceptions, except when we don't" scenario has existed since the very beginning. As "the very beginning" predates [C# 4 exception filters][1], there wasn't a way for `generator` output to "selectively `catch` exceptions". We squared this circle by using `System.Reflection.Emit`: 1. During Java Callable Wrapper registration, we lookup the "marshal method getter" as provided to the `Runtime.register()` invocation, e.g. `Android.App.Activity.GetOnCreate_Landroid_os_Bundle_Handler()`. 2. `GetOnCreate_Landroid_os_Bundle_Handler()` is `generator` output, and contains a `JNINativeWrapper.CreateDelegate()` invocation: cb_onCreate_Landroid_os_Bundle_ = JNINativeWrapper.CreateDelegate ((_JniMarshal_PPL_V) n_OnCreate_Landroid_os_Bundle_); 3. `JNINativeWrapper.CreateDelegate()` uses `System.Reflection.Emit` to create a new delegate instance which *wraps* the marshal method `Activity.n_OnCreate_Landroid_os_Bundle()` in a `try`/*filtered* `catch` block and marshals the exception to Java; `JNINativeWrapper.CreateDelegate()` effectively returns: bool _run_catch_if_debugger_not_attached (Exception e) { if (Debugger.IsAttached || !JNIEnv.PropagateExceptions) { JNIEnv.mono_unhandled_exception (e); return false; } return true; } _JniMarshal_PPL_V result = (jnienv, native__this, native_savedInstanceState) => { JNIEnv.WaitForBridgeProcessing (); try { Activity.n_OnCreate_Landroid_os_Bundle_ (jnienv, native__this, native_savedInstanceState); } catch (Exception e) when (_run_catch_if_debugger_not_attached (e)) { AndroidEnvironment.UnhandledException (e); if (Debugger.IsAttached || !JNIEnv.PropagateExceptions) throw; } }; return result; Again, this was C# 2.0 at the time, so C# 4 exception filters couldn't be used, thus the need for `System.Reflection.Emit`, so that [`ILGenerator.BeginExceptionFilterBLock()`][2] could be used (the support for which is a Mono extension). After this point, use of `System.Reflection.Emit` was part of the implicit ABI between Xamarin.Android and binding assemblies. While `generator` *could* be updated to *itself* emit the `try`/`catch` block with exception filters, that would only work for binding assemblies released *after* that `generator` fix. The `System.Reflection.Emit` wrapper *can't* be skipped without breaking semantic compatibility, *or* without allowing C# exceptions to always pass through a JNI boundary, which would be Bad™. The use of `System.Refleciton.Emit` is a Known Problem™, and something we'd *like* to remove. (Hence the [`jnimarshalmethod-gen`][3] explorations…) With that background out of the way… Let us turn our attention to the `dotnet new maui` template. The default MAUI template hits `JNINativeWrapper.CreateDelegate()` 58 times during process startup, and we were wondering if we could selectively improve these particular invocations, without needing to re-think the entire "marshal method" infrastructure. *Partially specialize* `JNINativeWrapper.CreateDelegate()` for the following delegate types: * `_JniMarshal_PP_V` * `_JniMarshal_PPI_V` * `_JniMarshal_PPL_L` * `_JniMarshal_PPL_V` * `_JniMarshal_PPL_Z` * `_JniMarshal_PPII_V` * `_JniMarshal_PPLI_V` * `_JniMarshal_PPLL_V` * `_JniMarshal_PPLL_Z` * `_JniMarshal_PPIIL_V` * `_JniMarshal_PPILL_V` * `_JniMarshal_PPLIL_Z` * `_JniMarshal_PPLLL_L` * `_JniMarshal_PPLLL_Z` * `_JniMarshal_PPIIII_V` * `_JniMarshal_PPLLLL_V` * `_JniMarshal_PPLIIII_V` * `_JniMarshal_PPZIIII_V` * `_JniMarshal_PPLIIIIIIII_V` This is done via use of a T4 template, which generates `JNINativeWrapper.CreateBuiltInDelegate()`, and `JNINativeWrapper.CreateDelegate()` is updated to call `CreateBuiltInDelegate()`: partial class JNINativeWrapper { static Delegate? CreateBuiltInDelegate (Delegate dlg, Type delegateType) { switch (delegateType.Name) { case "_JniMarshal_PP_V": return … case "_JniMarshal_PPI_V": return … … } return null; } public static Delegate CreateDelegate (Delegate dlg) { … var builtin = CreateBuiltInDelegate (dlg, dlg.GetType (); if (builtin != null) return builtin; … } } This avoids use of `System.Reflection.Emit` for the specified types. Other changes: * Update `TypeManager.GetActivateHandler()` to use `_JniMarshal_PPLLLL_V` instead of `Action<IntPtr, IntPtr, IntPtr, IntPtr, IntPtr, IntPtr>`, so the fast path can be used. * Added a log message for `adb shell septprop debug.mono.log assembly`: Falling back to System.Reflection.Emit for delegate type '{delegateType}': {dlg.Method} * I was also able to remove `mono_unhandled_exception_method` from `JNINativeWrapper` as we already has this value in `JNIEnv`. ~~ Results ~~ Testing `dotnet new maui` with version: msbuild Xamarin.Android.sln -t:InstallMaui -bl -p:MauiVersion=6.0.200-preview.13.2536 A `Release` build on a Pixel 5 device, total startup time: | Startup | Average (ms) | Std Err (ms) | Std Dev (ms) | | --------- | ------------: | ------------: | ------------: | | Before | 1106.3 | 6.919 | 21.879 | | After | 1078.8 | 5.438 | 17.197 | This might save ~35ms on average? If I time the message for one call, [such as][4]: I monodroid-timing: Runtime.register: registering type `Microsoft.Maui.MauiApplication, Microsoft.Maui, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null` I monodroid-timing: Runtime.register: end time; elapsed: 0s:17::794845 The result is: | One Call | Average (ms) | Std Err (ms) | Std Dev (ms) | | --------- | ------------: | ------------: | ------------: | | Before | 23.925 | 0.050 | 0.159 | | After | 18.723 | 0.094 | 0.298 | Saving ~5.8ms for this one call. `.apk` size difference for `dotnet new android`: % apkdiff -f before.apk after.apk Size difference in bytes ([*1] apk1 only, [*2] apk2 only): + 3,390 assemblies/assemblies.blob + 54 assemblies/assemblies.x86_64.blob - 4 assemblies/assemblies.arm64_v8a.blob - 15 assemblies/assemblies.x86.blob - 65 assemblies/assemblies.armeabi_v7a.blob Summary: + 3,360 Other entries 0.03% (of 10,526,432) + 0 Dalvik executables 0.00% (of 7,816,392) + 0 Shared libraries 0.00% (of 18,414,404) + 4,096 Package size difference 0.02% (of 21,006,128) We're looking at a ~4KB size increase for this partial specialization. [0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#RegisterNatives [1]: https://docs.microsoft.com/en-us/dotnet/csharp/fundamentals/exceptions/exception-handling#catch-blocks [2]: https://docs.microsoft.com/is-is/dotnet/api/system.reflection.emit.ilgenerator.beginexceptfilterblock?view=net-6.0 [3]: http://github.com/xamarin/Java.Interop/commit/c8f3e51a6cfd78bdce89e2429efae4495481f57b [4]: https://github.com/dotnet/maui/blob/bfba62ed796d3416c4fcaa7cfbea86dc8d5e04c2/src/Compatibility/ControlGallery/src/Android/MainApplication.cs
- Loading branch information
1 parent
ce0d212
commit 32cff43