Skip to content

Commit cec9eb4

Browse files
radekdoulikjonpryor
authored andcommitted
[linker] Improve *Invoker types linking (#3525)
Fixes: #3454 Context: #3525 (comment) Improves fix of #3263 The fix in commit 4d8c28f increased the apk size more than needed. It marked all the `*Invoker` types, because the `MarkJavaObjects` substep iterates over all `Java.Lang.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. > Interfaces on a type will no longer be marked simply because the > type is marked. Instead, marking of interfaces on types is deferred > until that interface type is marked for some other reason. Before the new optimization, the linker preserved these interfaces (like `ITextWatcher`) when marking the types based on them (like `AbsListView` or `ITextWatcherImplementor` in this case). That also resulted in keeping the invoker interfaces in the assembly. The above mentioned optimization changed that. Now they were linked out, because the interfaces implementation are only accessed (during app runtime) after the invoker type is loaded by `Type.GetType(string,bool)` in the native members registration process. The linker cannot know about that without XA help during the linking. The result of `ITextWatcher` being linked away is that the `ITeextWatcherInvoker` type was linked away too. It is now fixed by overriding the `MarkStep.ShouldMarkInterfaceImplementation()` method, which returns true for registered interfaces. These interfaces are special for XA in a way, that it can be, in some cases, reached only from Java side and thus linker is unable to track the interface usage. It is similar to how linker handles the COM interfaces. The crash in Topeka sample: System.TypeLoadException: Could not load type 'Android.Text.ITextWatcherInvoker' from assembly 'Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'. at (wrapper managed-to-native) System.RuntimeTypeHandle.internal_from_name(string,System.Threading.StackCrawlMark&,System.Reflection.Assembly,bool,bool,bool) at System.RuntimeTypeHandle.GetTypeByName (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, System.Threading.StackCrawlMark& stackMark, System.Boolean loadTypeFromPartialName) at System.RuntimeType.GetType (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, System.Threading.StackCrawlMark& stackMark) at System.Type.GetType (System.String typeName, System.Boolean throwOnError) at Android.Runtime.AndroidTypeManager.RegisterNativeMembers (Java.Interop.JniType jniType, System.Type type, System.String methods) at Android.Runtime.JNIEnv.RegisterJniNatives (System.IntPtr typeName_ptr, System.Int32 typeName_len, System.IntPtr jniClass, System.IntPtr methods_ptr, System.Int32 methods_len) at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_call_static_object_method_a(intptr,intptr&,intptr,intptr,Java.Interop.JniArgumentValue*) at Java.Interop.JniEnvironment+StaticMethods.CallStaticObjectMethod (Java.Interop.JniObjectReference type, Java.Interop.JniMethodInfo method, Java.Interop.JniArgumentValue* args) at Android.Runtime.JNIEnv.CallStaticObjectMethod (System.IntPtr jclass, System.IntPtr jmethod, Android.Runtime.JValue* parms) at Android.Runtime.JNIEnv.CallStaticObjectMethod (System.IntPtr jclass, System.IntPtr jmethod, Android.Runtime.JValue[] parms) at Android.Runtime.JNIEnv.FindClass (System.String classname) at Android.Runtime.JNIEnv.AllocObject (System.String jniClassName) at Android.Runtime.JNIEnv.StartCreateInstance (System.String jniClassName, System.String jniCtorSignature, Android.Runtime.JValue* constructorParameters) at Android.Runtime.JNIEnv.StartCreateInstance (System.String jniClassName, System.String jniCtorSignature, Android.Runtime.JValue[] constructorParameters) 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) at Android.Widget.TextView.add_TextChanged (System.EventHandler`1[TEventArgs] value) at Topeka.Fragments.SignInFragment.InitContentViews (Android.Views.View view) at Topeka.Fragments.SignInFragment.OnViewCreated (Android.Views.View view, Android.OS.Bundle savedInstanceState) 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) 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): | Version | Size in bytes | |----------------|--------------:| | Before @4d8c28 | 21434941 | | After @4d8c28 | 28242405 | | This fix | 21638678 | So the previous fix increased the apk size by around 6.5Mbytes, this fix adds less than 199kbytes instead.
1 parent ebd65c9 commit cec9eb4

File tree

3 files changed

+31
-6
lines changed

3 files changed

+31
-6
lines changed

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Extensions.cs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,14 +120,14 @@ private static bool IsRegisterAttribute (CustomAttribute attribute)
120120
return true;
121121
}
122122

123-
public static bool TryGetRegisterAttribute (this MethodDefinition method, out CustomAttribute register)
123+
static bool TryGetRegisterAttribute (ICustomAttributeProvider provider, out CustomAttribute register)
124124
{
125125
register = null;
126126

127-
if (!method.HasCustomAttributes)
127+
if (!provider.HasCustomAttributes)
128128
return false;
129129

130-
foreach (CustomAttribute attribute in method.CustomAttributes) {
130+
foreach (CustomAttribute attribute in provider.CustomAttributes) {
131131
if (!IsRegisterAttribute (attribute))
132132
continue;
133133

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

141+
public static bool TryGetRegisterAdapter (this TypeDefinition td, out string adapter)
142+
{
143+
CustomAttribute register;
144+
adapter = null;
145+
146+
if (!TryGetRegisterAttribute (td, out register))
147+
return false;
148+
149+
if (register.ConstructorArguments.Count != 3)
150+
return false;
151+
152+
adapter = (string) register.ConstructorArguments [2].Value;
153+
154+
if (string.IsNullOrEmpty (adapter))
155+
return false;
156+
157+
return true;
158+
}
159+
141160
public static bool TryGetRegisterMember (this MethodDefinition md, out string method)
142161
{
143162
return TryGetRegisterMember (md, out method, out _, out _);
@@ -150,7 +169,7 @@ public static bool TryGetRegisterMember (this MethodDefinition md, out string me
150169
nativeMethod = null;
151170
signature = null;
152171

153-
if (!md.TryGetRegisterAttribute (out register))
172+
if (!TryGetRegisterAttribute (md, out register))
154173
return false;
155174

156175
if (register.ConstructorArguments.Count != 3)

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,6 @@ void PreserveInvoker (TypeDefinition type)
192192
if (invoker == null)
193193
return;
194194

195-
Annotations.Mark (invoker);
196-
197195
PreserveIntPtrConstructor (invoker);
198196
PreserveInterfaceMethods (type, invoker);
199197
}

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MonoDroidMarkStep.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,14 @@ protected override void DoAdditionalMethodProcessing (MethodDefinition method)
436436
PreserveRegisteredMethod (method.DeclaringType, member);
437437
}
438438

439+
protected override bool ShouldMarkInterfaceImplementation (TypeDefinition type, InterfaceImplementation iface, TypeDefinition resolvedInterfaceType)
440+
{
441+
if (base.ShouldMarkInterfaceImplementation (type, iface, resolvedInterfaceType))
442+
return true;
443+
444+
return resolvedInterfaceType.TryGetRegisterAdapter (out _);
445+
}
446+
439447
protected override void DoAdditionalTypeProcessing (TypeDefinition type)
440448
{
441449
// If we are preserving a Mono.Android interface,

0 commit comments

Comments
 (0)