Skip to content

Commit

Permalink
[linker] Improve *Invoker types linking
Browse files Browse the repository at this point in the history
Improves fix of dotnet#3263

The
dotnet@4d8c28f
fix increased the apk size more than needed. It marked all the
*Invoker types, beause MarkJavaObjects substep iterates over all
Java.Object derived types, before all the linking.

Turned out that the original regression came from the linker
itself. The new linker optimization was introduced in
dotnet/linker@13ea158
and so the ITextWatcher interface was not marked anymore - as it is
only accessed by reflection and instantiated during the runtime. And
thus the ITeextWatcherInvoker type was linked away too.

It is now fixed by overriding the `ShouldMarkInterfaceImplementation`
method in `MarkStep`, which returns true for registered interfaces.

The crash in Topeka sample:

    I MonoDroid: System.TypeLoadException: Could not load type 'Android.Text.ITextWatcherInvoker' from assembly 'Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.
    I MonoDroid:   at (wrapper managed-to-native) System.RuntimeTypeHandle.internal_from_name(string,System.Threading.StackCrawlMark&,System.Reflection.Assembly,bool,bool,bool)
    I MonoDroid:   at System.RuntimeTypeHandle.GetTypeByName (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, System.Threading.StackCrawlMark& stackMark, System.Boolean loadTypeFromPartialName) [0x0008d] in <ab232a84cff64754aaec9d455f50a794>:0
    I MonoDroid:   at System.RuntimeType.GetType (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, System.Threading.StackCrawlMark& stackMark) [0x0000e] in <ab232a84cff64754aaec9d455f50a794>:0
    I MonoDroid:   at System.Type.GetType (System.String typeName, System.Boolean throwOnError) [0x00002] in <ab232a84cff64754aaec9d455f50a794>:0
    I MonoDroid:   at Android.Runtime.AndroidTypeManager.RegisterNativeMembers (Java.Interop.JniType jniType, System.Type type, System.String methods) [0x00131] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.RegisterJniNatives (System.IntPtr typeName_ptr, System.Int32 typeName_len, System.IntPtr jniClass, System.IntPtr methods_ptr, System.Int32 methods_len) [0x00128] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_call_static_object_method_a(intptr,intptr&,intptr,intptr,Java.Interop.JniArgumentValue*)
    I MonoDroid:   at Java.Interop.JniEnvironment+StaticMethods.CallStaticObjectMethod (Java.Interop.JniObjectReference type, Java.Interop.JniMethodInfo method, Java.Interop.JniArgumentValue* args) [0x00073] in <fbb1482d0c6b44009a68f9f7e8c6389a>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.CallStaticObjectMethod (System.IntPtr jclass, System.IntPtr jmethod, Android.Runtime.JValue* parms) [0x0000f] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.CallStaticObjectMethod (System.IntPtr jclass, System.IntPtr jmethod, Android.Runtime.JValue[] parms) [0x0001a] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.FindClass (System.String classname) [0x0003f] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.AllocObject (System.String jniClassName) [0x00001] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.StartCreateInstance (System.String jniClassName, System.String jniCtorSignature, Android.Runtime.JValue* constructorParameters) [0x0000a] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.StartCreateInstance (System.String jniClassName, System.String jniCtorSignature, Android.Runtime.JValue[] constructorParameters) [0x00019] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Text.TextWatcherImplementor..ctor (System.Object inst, System.EventHandler`1[TEventArgs] changed_handler, System.EventHandler`1[TEventArgs] before_handler, System.EventHandler`1[TEventArgs] after_handler) [0x00010] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Widget.TextView.add_TextChanged (System.EventHandler`1[TEventArgs] value) [0x0001f] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Topeka.Fragments.SignInFragment.InitContentViews (Android.Views.View view) [0x00011] in <201b9318435b446d821a19dfd1fa7bfb>:0
    I MonoDroid:   at Topeka.Fragments.SignInFragment.OnViewCreated (Android.Views.View view, Android.OS.Bundle savedInstanceState) [0x0003e] in <201b9318435b446d821a19dfd1fa7bfb>:0
    I MonoDroid:   at Android.App.Fragment.n_OnViewCreated_Landroid_view_View_Landroid_os_Bundle_ (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_view, System.IntPtr native_savedInstanceState) [0x0001a] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at (wrapper dynamic-method) Android.Runtime.DynamicMethodNameCounter.9(intptr,intptr,intptr,intptr)

The comparison of apk sizes with original and this fix (Topeka sample,
which was also used as repro):

Before @4d8c28    21434941 bytes
After  @4d8c28    28242405 bytes
This fix          21638678 bytes

So the previous fix increased the apk size by around 6.5Mbytes, this
fix adds less than 199kbytes instead.
  • Loading branch information
radekdoulik committed Aug 20, 2019
1 parent bfaedff commit 441858a
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,14 @@ private static bool IsRegisterAttribute (CustomAttribute attribute)
return true;
}

public static bool TryGetRegisterAttribute (this MethodDefinition method, out CustomAttribute register)
static bool TryGetRegisterAttribute (ICustomAttributeProvider provider, out CustomAttribute register)
{
register = null;

if (!method.HasCustomAttributes)
if (!provider.HasCustomAttributes)
return false;

foreach (CustomAttribute attribute in method.CustomAttributes) {
foreach (CustomAttribute attribute in provider.CustomAttributes) {
if (!IsRegisterAttribute (attribute))
continue;

Expand All @@ -138,6 +138,25 @@ public static bool TryGetRegisterAttribute (this MethodDefinition method, out Cu
return false;
}

public static bool TryGetRegisterAdapter (this TypeDefinition td, out string adapter)
{
CustomAttribute register;
adapter = null;

if (!TryGetRegisterAttribute (td, out register))
return false;

if (register.ConstructorArguments.Count != 3)
return false;

adapter = (string) register.ConstructorArguments [2].Value;

if (string.IsNullOrEmpty (adapter))
return false;

return true;
}

public static bool TryGetRegisterMember (this MethodDefinition md, out string method)
{
return TryGetRegisterMember (md, out method, out _, out _);
Expand All @@ -150,7 +169,7 @@ public static bool TryGetRegisterMember (this MethodDefinition md, out string me
nativeMethod = null;
signature = null;

if (!md.TryGetRegisterAttribute (out register))
if (!TryGetRegisterAttribute (md, out register))
return false;

if (register.ConstructorArguments.Count != 3)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,6 @@ void PreserveInvoker (TypeDefinition type)
if (invoker == null)
return;

Annotations.Mark (invoker);

PreserveIntPtrConstructor (invoker);
PreserveInterfaceMethods (type, invoker);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,14 @@ protected override void DoAdditionalMethodProcessing (MethodDefinition method)
PreserveRegisteredMethod (method.DeclaringType, member);
}

protected override bool ShouldMarkInterfaceImplementation (TypeDefinition type, InterfaceImplementation iface, TypeDefinition resolvedInterfaceType)
{
if (base.ShouldMarkInterfaceImplementation (type, iface, resolvedInterfaceType))
return true;

return resolvedInterfaceType.TryGetRegisterAdapter (out _);
}

protected override void DoAdditionalTypeProcessing (TypeDefinition type)
{
// If we are preserving a Mono.Android interface,
Expand Down

0 comments on commit 441858a

Please sign in to comment.