Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Documentation/release-notes/4302.md
Original file line number Diff line number Diff line change
@@ -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.
37 changes: 30 additions & 7 deletions src/Mono.Android/Android.Runtime/AndroidRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
}

Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
}

Expand Down
67 changes: 49 additions & 18 deletions src/Mono.Android/Android.Runtime/JNIEnv.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ struct JnienvInitializeArgs {
public byte brokenExceptionTransitions;
public int packageNamingPolicy;
public byte ioExceptionType;
public int jniAddNativeMethodRegistrationAttributePresent;
}
#pragma warning restore 0649

Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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__");
Expand All @@ -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
*/
Expand Down Expand Up @@ -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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would make more sense to just leave this entirely in Java & C...?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should absolutely move all this stuff into XamarinUncaughtExceptionHandler.

Here's my conceptual problem: Thread.getDefaultUncaughtExceptionHandler() forms a "linked list", and it is the responsibility of all Thread.UncaughtExceptionHandler implementations to propagate the exception to their "parent" handler within Thread.UncaughtExceptionHandler.uncaughtException().

The problem here is twofold:

  1. UncaughtExceptionHandler instantiation is "responsible" for calling Thread.DefaultUncaughtExceptionHandler and obtaining the "parent" handler, and
  2. UncaughtExceptionHandler is only instantiated when there's an unhandled exception, so it's not "clean" as of process startup, and
  3. Propagating the unhandled exception is thus poorly defined, plus
  4. It's way confusion trying to keep these four different spots in mind and in sync (XamarinUncaughtExceptionHandler, UncaughtExceptionHandler, JNIEnv.PropagateUncaughtException(), UncaughtExceptionHandler.UncaughtException()).

It does not give me a good feeling.

Instead, let's "move" the "default uncaught exception handler linked list management" of UncaughtExceptionHandler into XamarinUncaughtExceptionHandler, and then move UncaughtExceptionHandler.UncaughtException() into JNIEnv.PropagateUncaughtException().

This should kill two places to try to keep track of things, simplifying logic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, changes applied. Working on this I wanted to make the minimum amount of changes to minimize potential of breaking something that's been in place for years, but I agree with your assessment 100%

{
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)]
Expand Down
69 changes: 0 additions & 69 deletions src/Mono.Android/Android.Runtime/UncaughtExceptionHandler.cs

This file was deleted.

4 changes: 2 additions & 2 deletions src/Mono.Android/Android.Runtime/XAPeerMembers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -73,4 +73,4 @@ static IntPtr GetThresholdClass (IJavaPeerable value)
return IntPtr.Zero;
}
}
}
}
2 changes: 1 addition & 1 deletion src/Mono.Android/Java.Interop/TypeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
1 change: 0 additions & 1 deletion src/Mono.Android/Mono.Android.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
1 change: 1 addition & 0 deletions src/Mono.Android/Test/Mono.Android-Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
5 changes: 4 additions & 1 deletion src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ public class GenerateJavaStubs : AndroidTask

public string ApplicationJavaClass { get; set; }

public bool SkipJniAddNativeMethodRegistrationAttributeScan { get; set; }

[Output]
public string [] GeneratedBinaryTypeMaps { get; set; }

Expand Down Expand Up @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()}");
Expand Down Expand Up @@ -285,6 +286,7 @@ void AddEnvironment ()
PackageNamingPolicy = pnp,
BoundExceptionType = boundExceptionType,
InstantRunEnabled = InstantRunEnabled,
JniAddNativeMethodRegistrationAttributePresent = appConfState != null ? appConfState.JniAddNativeMethodRegistrationAttributePresent : false,
};

using (var sw = MemoryStreamPool.Shared.CreateStreamWriter ()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1099,7 +1099,7 @@ public void BuildProguardEnabledProject ([Values (true, false)] bool isRelease,
FileAssert.Exists (dexFile);
var classes = new [] {
"Lmono/MonoRuntimeProvider;",
"Landroid/runtime/UncaughtExceptionHandler;",
"Landroid/runtime/JavaProxyThrowable;",
"Landroid/support/v7/widget/Toolbar;"
};
foreach (var className in classes) {
Expand Down
Loading