[Mono.Android] Reduce Reflection & reduce startup overhead (#4302)

Reduce or remove reflection use around JNI method registration, and
rework how uncaught exceptions are handled.

Java.Interop provides an alternate mechanism to allow registering
Java native methods:
[the `[JniAddNativeMethodRegistrationAttribute]` custom attribute][0]
which can be placed on methods of a type to allow "manual"
insertion into the [`JNIEnv::RegisterNatives()`][2] process.

The `Java.Interop-Tests.dll` unit test assembly (130905e) uses this
alternate native method registration mechanism..

Unfortunately, `[JniAddNativeMethodRegistration]` is looked up via
Reflection, which isn't exactly "fast".

Given that `[JniAddNativeMethodRegistration]` is only used by one
assembly *in the world*, let's optimize the common case:
[dotnet/java-interop@b33d183d][3] made the
`[JniAddNativeMethodRegistration]` lookup *optional*.  Disable the
custom attribute lookup *by default*, and allow it to be enabled by
setting the `$(_SkipJniAddNativeMethodRegistrationAttributeScan)`
MSBuild property to True.  Yes, this property is "private".  We'll
investigate more automatic and optimized ideas in the future.

Rework how Java-side unhandled exceptions are processed.  Previously,
this was handled by the `Android.Runtime.UncaughtExceptionHandler`
class, which was constructed during process startup.  A side effect
of this is that it required going through the (*normal* reflection-
based) JNI registration logic of `mono.android.Runtime.register()`,
contributing to startup for something that *ideally* would
Never Happen™.  (Nobody likes unhandled exceptions.)

Changes this so that instead of a C#-side `UncaughtExceptionHandler`
type we instead have a Java-side
`mono.android.XamarinUncaughtExceptionHandler` type which is
created and provided to
`java.lang.Thread.setDefaultUncaughtExceptionHandler()` during
startup.  `XamarinUncaughtExceptionHandler` doesn't do anything until
Java calls `XamarinUncaughtExceptionHandler.uncaughtException()`,
which in turn invokes `Runtime.propagateUncaughtException()`, which
in turn does what `UncaughtExceptionHandler` previously did: lookup
`AppDomain.DoUnhandledException()` via Reflection and invoke it, thus
raising the `AppDomain.UnhandledException` event.

In this manner all overheads associated with unhandled exceptions are
delayed until the exception happens, with minimal impact on startup.

Gains in startup time are quite nice:

  * Device name: **Pixel 3 XL**
  * Device architecture: **arm64-v8a**
  * Number of test runs: **10**
  * Test application: **Xamarin.Forms integration test**

|                 | **Native to managed**  | **Runtime init** | **Displayed** | **Notes**                      |
|-----------------|------------------------|------------------|---------------|--------------------------------|
| **master**      | 131.278                | 149.074          | 789.10        |  preload enabled; 32-bit build |
| **this commit** | 49.446                 | 66.989           | 764.30        |                                |
| **master**      | 132.315                | 147.187          | 795.60        | preload disabled; 32-bit build |
| **this commit** | 48.876                 | 63.822           | 754.30        |                                |
| **master**      | 121.544                | 137.736          | 728.20        |  preload enabled; 64-bit build |
| **this commit** | 45.350                 | 61.464           | 699.50        |                                |
| **master**      | 123.448                | 137.052          | 727.40        | preload disabled; 64-bit build |
| **this commit** | 44.765                 | 58.047           | 689.00        |                                |

  * Device name: **Pixel 3 XL**
  * Device architecture: **arm64-v8a**
  * Number of test runs: **10**
  * Test application: Xamarin.Forms "Hello World" app with one label

|                 | **Native to managed**  | **Runtime init** | **Displayed** | **Notes**                      |
|-----------------|------------------------|------------------|---------------|--------------------------------|
| **master**      | 122.090                | 142.004          | 639.00        |  preload enabled; 32-bit build |
| **this commit** | 44.370                 | 63.803           | 586.10        |                                |
| **master**      | 121.110                | 134.378          | 634.20        | preload disabled; 32-bit build |
| **this commit** | 45.085                 | 57.992           | 580.40        |                                |
| **master**      | 120.973                | 141.235          | 637.20        |  preload enabled; 64-bit build |
| **this commit** | 44.767                 | 63.846           | 578.50        |                                |
| **master**      | 120.785                | 134.588          | 627.00        | preload disabled; 64-bit build |
| **this commit** | 44.859                 | 57.590           | 575.40        |                                |

[0]: dotnet/java-interop@7d51163
[2]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#RegisterNatives
[3]: dotnet/java-interop@b33d183

Author: grendello

Documentation/release-notes/4302.md

@@ -0,0 +1,11 @@
+### App startup performance
+
+  * [GitHub PR 4302](https://github.com/xamarin/xamarin-android/pull/4302):
+    Avoid unneeded calls to `GetCustomAttribute()` during app startup for the
+    common case where an app has no types decorated with the
+    `[JniAddNativeMethodRegistration]` attribute.  Additionally, instead of
+    using a managed method to propagate uncaught exceptions from Java, use a
+    Java method that calls into the unmanaged Xamarin.Android runtime.  These
+    changes reduced the time to display the first screen of a small test
+    Xamarin.Forms app from about 730 milliseconds to about 700 milliseconds for
+    a Release configuration build on a Google Pixel 3 XL device.

src/Mono.Android/Android.Runtime/AndroidRuntime.cs

@@ -15,8 +15,18 @@ namespace Android.Runtime {
 
 	class AndroidRuntime : JniRuntime {
 
-		internal AndroidRuntime (IntPtr jnienv, IntPtr vm, bool allocNewObjectSupported, IntPtr classLoader, IntPtr classLoader_loadClass)
-			: base (new AndroidRuntimeOptions (jnienv, vm, allocNewObjectSupported, classLoader, classLoader_loadClass))
+		internal AndroidRuntime (IntPtr jnienv,
+				IntPtr vm,
+				bool allocNewObjectSupported,
+				IntPtr classLoader,
+				IntPtr classLoader_loadClass,
+				bool jniAddNativeMethodRegistrationAttributePresent)
+			: base (new AndroidRuntimeOptions (jnienv,
+					vm,
+					allocNewObjectSupported,
+					classLoader,
+					classLoader_loadClass,
+					jniAddNativeMethodRegistrationAttributePresent))
 		{
 		}
 
@@ -67,18 +77,23 @@ public override void RaisePendingException (Exception pendingException)
 	}
 
 	class AndroidRuntimeOptions : JniRuntime.CreationOptions {
-
-		public AndroidRuntimeOptions (IntPtr jnienv, IntPtr vm, bool allocNewObjectSupported, IntPtr classLoader, IntPtr classLoader_loadClass)
+		public AndroidRuntimeOptions (IntPtr jnienv,
+				IntPtr vm,
+				bool allocNewObjectSupported,
+				IntPtr classLoader,
+				IntPtr classLoader_loadClass,
+				bool jniAddNativeMethodRegistrationAttributePresent)
 		{
 			EnvironmentPointer      = jnienv;
 			ClassLoader             = new JniObjectReference (classLoader, JniObjectReferenceType.Global);
 			ClassLoader_LoadClass_id= classLoader_loadClass;
 			InvocationPointer       = vm;
 			NewObjectRequired       = !allocNewObjectSupported;
 			ObjectReferenceManager  = new AndroidObjectReferenceManager ();
-			TypeManager             = new AndroidTypeManager ();
+			TypeManager             = new AndroidTypeManager (jniAddNativeMethodRegistrationAttributePresent);
 			ValueManager            = new AndroidValueManager ();
 			UseMarshalMemberBuilder = false;
+			JniAddNativeMethodRegistrationAttributePresent = jniAddNativeMethodRegistrationAttributePresent;
 		}
 	}
 
@@ -219,6 +234,12 @@ public override void DeleteWeakGlobalReference (ref JniObjectReference value)
 	}
 
 	class AndroidTypeManager : JniRuntime.JniTypeManager {
+		bool jniAddNativeMethodRegistrationAttributePresent;
+
+		public AndroidTypeManager (bool jniAddNativeMethodRegistrationAttributePresent)
+		{
+			this.jniAddNativeMethodRegistrationAttributePresent = jniAddNativeMethodRegistrationAttributePresent;
+		}
 
 		protected override IEnumerable<Type> GetTypesForSimpleReference (string jniSimpleReference)
 		{
@@ -347,13 +368,15 @@ public override void RegisterNativeMembers (JniType nativeClass, Type type, stri
 				return;
 
 			if (string.IsNullOrEmpty (methods)) {
-				base.RegisterNativeMembers (nativeClass, type, methods);
+				if (jniAddNativeMethodRegistrationAttributePresent)
+					base.RegisterNativeMembers (nativeClass, type, methods);
 				return;
 			}
 
 			string[] members = methods.Split ('\n');
 			if (members.Length < 2) {
-				base.RegisterNativeMembers (nativeClass, type, methods);
+				if (jniAddNativeMethodRegistrationAttributePresent)
+					base.RegisterNativeMembers (nativeClass, type, methods);
 				return;
 			}
 

src/Mono.Android/Android.Runtime/JNIEnv.cs

@@ -33,6 +33,7 @@ struct JnienvInitializeArgs {
 		public byte            brokenExceptionTransitions;
 		public int             packageNamingPolicy;
 		public byte            ioExceptionType;
+		public int             jniAddNativeMethodRegistrationAttributePresent;
 	}
 #pragma warning restore 0649
 
@@ -54,7 +55,6 @@ public static partial class JNIEnv {
 		internal static int    gref_gc_threshold;
 
 		internal  static  bool  PropagateExceptions;
-		static UncaughtExceptionHandler defaultUncaughtExceptionHandler;
 
 		internal static bool IsRunningOnDesktop;
 		internal static bool LogTypemapMissStackTrace;
@@ -173,7 +173,7 @@ internal static unsafe void Initialize (JnienvInitializeArgs* args)
 			Mono.SystemDependencyProvider.Initialize ();
 
 			BoundExceptionType = (BoundExceptionType)args->ioExceptionType;
-			androidRuntime = new AndroidRuntime (args->env, args->javaVm, androidSdkVersion > 10, args->grefLoader, args->Loader_loadClass);
+			androidRuntime = new AndroidRuntime (args->env, args->javaVm, androidSdkVersion > 10, args->grefLoader, args->Loader_loadClass, args->jniAddNativeMethodRegistrationAttributePresent != 0);
 			AndroidValueManager = (AndroidValueManager) androidRuntime.ValueManager;
 
 			AllocObjectSupported = androidSdkVersion > 10;
@@ -183,12 +183,6 @@ internal static unsafe void Initialize (JnienvInitializeArgs* args)
 
 			PropagateExceptions = args->brokenExceptionTransitions == 0;
 
-			if (PropagateExceptions) {
-				defaultUncaughtExceptionHandler = new UncaughtExceptionHandler (Java.Lang.Thread.DefaultUncaughtExceptionHandler);
-				if (!IsRunningOnDesktop)
-					Java.Lang.Thread.DefaultUncaughtExceptionHandler = defaultUncaughtExceptionHandler;
-			}
-
 			JavaNativeTypeManager.PackageNamingPolicy = (PackageNamingPolicy)args->packageNamingPolicy;
 			if (IsRunningOnDesktop) {
 				string packageNamingPolicy = Environment.GetEnvironmentVariable ("__XA_PACKAGE_NAMING_POLICY__");
@@ -204,13 +198,6 @@ internal static unsafe void Initialize (JnienvInitializeArgs* args)
 
 		internal static void Exit ()
 		{
-			/* Reset uncaught exception handler so that we don't mistakenly reuse a
-			 * now-invalid handler the next time we reinitialize JNIEnv.
-			 */
-			var uncaughtExceptionHandler = Java.Lang.Thread.DefaultUncaughtExceptionHandler as UncaughtExceptionHandler;
-			if (uncaughtExceptionHandler != null && uncaughtExceptionHandler == defaultUncaughtExceptionHandler)
-				Java.Lang.Thread.DefaultUncaughtExceptionHandler = uncaughtExceptionHandler.DefaultHandler;
-
 			/* Manually dispose surfaced objects and close the current JniEnvironment to
 			 * avoid ObjectDisposedException thrown on finalizer threads after shutdown
 			 */
@@ -249,15 +236,59 @@ static void ManualJavaObjectDispose (Java.Lang.Object obj)
 			GC.SuppressFinalize (obj);
 		}
 
+		static Action<Exception> mono_unhandled_exception;
+		static Action<AppDomain, UnhandledExceptionEventArgs> AppDomain_DoUnhandledException;
+
+		static void Initialize ()
+		{
+			if (mono_unhandled_exception == null) {
+				var mono_UnhandledException = typeof (System.Diagnostics.Debugger)
+					.GetMethod ("Mono_UnhandledException", BindingFlags.NonPublic | BindingFlags.Static);
+				mono_unhandled_exception = (Action<Exception>) Delegate.CreateDelegate (typeof(Action<Exception>), mono_UnhandledException);
+			}
+
+			if (AppDomain_DoUnhandledException == null) {
+				var ad_due = typeof (AppDomain)
+					.GetMethod ("DoUnhandledException",
+					            bindingAttr:  BindingFlags.NonPublic | BindingFlags.Instance,
+					            binder:       null,
+					            types:        new []{typeof (UnhandledExceptionEventArgs)},
+					            modifiers:    null);
+				if (ad_due != null) {
+					AppDomain_DoUnhandledException  = (Action<AppDomain, UnhandledExceptionEventArgs>) Delegate.CreateDelegate (
+						typeof (Action<AppDomain, UnhandledExceptionEventArgs>), ad_due);
+				}
+			}
+		}
+
 		internal static void PropagateUncaughtException (IntPtr env, IntPtr javaThreadPtr, IntPtr javaExceptionPtr)
 		{
-			if (defaultUncaughtExceptionHandler == null)
+			if (!PropagateExceptions)
 				return;
 
-			var javaThread = JavaObject.GetObject<Java.Lang.Thread> (env, javaThreadPtr, JniHandleOwnership.DoNotTransfer);
+			try {
+				Initialize ();
+			} catch (Exception e) {
+				Android.Runtime.AndroidEnvironment.FailFast ($"Unable to initialize UncaughtExceptionHandler. Nested exception caught: {e}");
+			}
+
 			var javaException = JavaObject.GetObject<Java.Lang.Throwable> (env, javaExceptionPtr, JniHandleOwnership.DoNotTransfer);
 
-			defaultUncaughtExceptionHandler.UncaughtException (javaThread, javaException);
+			// Disabled until Linker error surfaced in https://github.com/xamarin/xamarin-android/pull/4302#issuecomment-596400025 is resolved
+			//System.Diagnostics.Debugger.Mono_UnhandledException (javaException);
+			mono_unhandled_exception (javaException);
+
+			try {
+				var jltp = javaException as JavaProxyThrowable;
+				Exception innerException = jltp?.InnerException;
+				var args  = new UnhandledExceptionEventArgs (innerException ?? javaException, isTerminating: true);
+
+				// Disabled until Linker error surfaced in https://github.com/xamarin/xamarin-android/pull/4302#issuecomment-596400025 is resolved
+				//AppDomain.CurrentDomain.DoUnhandledException (args);
+				AppDomain_DoUnhandledException (AppDomain.CurrentDomain, args);
+			} catch (Exception e) {
+				Logger.Log (LogLevel.Error, "monodroid", "Exception thrown while raising AppDomain.UnhandledException event: " + e.ToString ());
+			}
 		}
 
 		[DllImport ("__Internal", CallingConvention = CallingConvention.Cdecl)]

src/Mono.Android/Android.Runtime/UncaughtExceptionHandler.cs

@@ -7,7 +7,7 @@ namespace Android.Runtime {
 
 	public class XAPeerMembers : JniPeerMembers {
 
-		static  Dictionary<string,  JniPeerMembers>         LegacyPeerMembers = new Dictionary<string, JniPeerMembers> ();
+		static  Dictionary<string,  JniPeerMembers>         LegacyPeerMembers = new Dictionary<string, JniPeerMembers> (StringComparer.Ordinal);
 
 		public XAPeerMembers (string jniPeerTypeName, Type managedPeerType)
 			: base (jniPeerTypeName, managedPeerType)
@@ -73,4 +73,4 @@ static IntPtr GetThresholdClass (IJavaPeerable value)
 			return IntPtr.Zero;
 		}
 	}
-}
+}

src/Mono.Android/Android.Runtime/XAPeerMembers.cs

@@ -23,7 +23,7 @@ static class TypeManagerMapDictionaries
 		public static Dictionary<string, Type> JniToManaged {
 			get {
 				if (_jniToManaged == null)
-					_jniToManaged = new Dictionary<string, Type> ();
+					_jniToManaged = new Dictionary<string, Type> (StringComparer.Ordinal);
 				return _jniToManaged;
 			}
 		}

src/Mono.Android/Java.Interop/TypeManager.cs

@@ -234,7 +234,6 @@
     <Compile Include="Android.Runtime\StringDefAttribute.cs" />
     <Compile Include="Android.Runtime\TimingLogger.cs" />
     <Compile Include="Android.Runtime\TypeManager.cs" />
-    <Compile Include="Android.Runtime\UncaughtExceptionHandler.cs" />
     <Compile Include="Android.Runtime\XAPeerMembers.cs" />
     <Compile Include="Android.Runtime\XmlPullParserReader.cs" />
     <Compile Include="Android.Runtime\XmlReaderPullParser.cs" />

src/Mono.Android/Mono.Android.csproj

@@ -22,6 +22,7 @@
     <AssemblyOriginatorKeyFile>..\..\..\product.snk</AssemblyOriginatorKeyFile>
     <TargetFrameworkVersion>v10.0</TargetFrameworkVersion>
     <AndroidDexTool Condition=" '$(AndroidDexTool)' == '' ">d8</AndroidDexTool>
+    <_SkipJniAddNativeMethodRegistrationAttributeScan>True</_SkipJniAddNativeMethodRegistrationAttributeScan>
   </PropertyGroup>
   <Import Project="Mono.Android-Test.Shared.projitems" Label="Shared" Condition="Exists('Mono.Android-Test.Shared.projitems')" />
   <Import Project="..\..\..\Configuration.props" />

src/Mono.Android/Test/Mono.Android-Tests.csproj

@@ -77,6 +77,8 @@ public class GenerateJavaStubs : AndroidTask
 
 		public string ApplicationJavaClass { get; set; }
 
+		public bool SkipJniAddNativeMethodRegistrationAttributeScan { get; set; }
+
 		[Output]
 		public string [] GeneratedBinaryTypeMaps { get; set; }
 
@@ -400,9 +402,10 @@ void SaveResource (string resource, string filename, string destDir, Func<string
 		void WriteTypeMappings (List<TypeDefinition> types)
 		{
 			var tmg = new TypeMapGenerator ((string message) => Log.LogDebugMessage (message), SupportedAbis);
-			if (!tmg.Generate (types, TypemapOutputDirectory, GenerateNativeAssembly))
+			if (!tmg.Generate (SkipJniAddNativeMethodRegistrationAttributeScan, types, TypemapOutputDirectory, GenerateNativeAssembly, out ApplicationConfigTaskState appConfState))
 				throw new XamarinAndroidException (4308, Properties.Resources.XA4308);
 			GeneratedBinaryTypeMaps = tmg.GeneratedBinaryTypeMaps.ToArray ();
+			BuildEngine4.RegisterTaskObject (ApplicationConfigTaskState.RegisterTaskObjectKey, appConfState, RegisteredTaskObjectLifetime.Build, allowEarlyCollection: false);
 		}
 	}
 }

src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs

@@ -249,6 +249,7 @@ void AddEnvironment ()
 				throw new InvalidOperationException ($"Unsupported BoundExceptionType value '{BoundExceptionType}'");
 			}
 
+			var appConfState = BuildEngine4.GetRegisteredTaskObject (ApplicationConfigTaskState.RegisterTaskObjectKey, RegisteredTaskObjectLifetime.Build) as ApplicationConfigTaskState;
 			foreach (string abi in SupportedAbis) {
 				NativeAssemblerTargetProvider asmTargetProvider;
 				string baseAsmFilePath = Path.Combine (EnvironmentOutputDirectory, $"environment.{abi.ToLowerInvariant ()}");
@@ -285,6 +286,7 @@ void AddEnvironment ()
 					PackageNamingPolicy = pnp,
 					BoundExceptionType = boundExceptionType,
 					InstantRunEnabled = InstantRunEnabled,
+					JniAddNativeMethodRegistrationAttributePresent = appConfState != null ? appConfState.JniAddNativeMethodRegistrationAttributePresent : false,
 				};
 
 				using (var sw = MemoryStreamPool.Shared.CreateStreamWriter ()) {