Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[illink] Do not preserve GeneratedEnumAttribute #5450

Merged

Conversation

radekdoulik
Copy link
Member

Context: #5167

The attribute itself doesn't need to be preserved.

I didn't find any usage of the attribute instances during runtime, so
this change removes them during linking.

apk size comparison, BuildReleaseArm64False test:

> apkdiff -f -e dll$ before.apk after.apk
Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
  -          39 assemblies/Mono.Android.dll
    +         132 Resource Android.ILLink.ILLink.LinkAttributes.xml
    -             Type Android.Runtime.GeneratedEnumAttribute
Summary:
  -          39 Assemblies -0.01% (of 749,078)

Note that the attribute instances removal doesn't work yet, we need
net6 illink for that.

Context: dotnet#5167

The attribute itself doesn't need to be preserved.

I didn't find any usage of the attribute instances during runtime, so
this change removes them during linking.

apk size comparison, BuildReleaseArm64False test:

    > apkdiff -f -e dll$ before.apk after.apk
    Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
      -          39 assemblies/Mono.Android.dll
        +         132 Resource Android.ILLink.ILLink.LinkAttributes.xml
        -             Type Android.Runtime.GeneratedEnumAttribute
    Summary:
      -          39 Assemblies -0.01% (of 749,078)

Note that the attribute instances removal doesn't work yet, we need
net6 illink for that.
@radekdoulik
Copy link
Member Author

@jpobst , is it OK to remove the attribute instances during linking? I tried to dig the history and this commit says they are not used yet. Is it still the case? https://github.com/xamarin/monodroid/commit/60961175f5aeb8503d6930f727eab62503b83b65

@jonpryor
Copy link
Member

jonpryor commented Jan 4, 2021

@radekdoulik: what do you mean by "attribute instances"? I don't understand the question.

Generated code does use [GeneratedEnum]:

% grep -r '\[.*\.GeneratedEnum\]' src/Mono.Android/obj/Debug/monoandroid10/android-30 | head -10
src/Mono.Android/obj/Debug/monoandroid10/android-30/mcw/Android.Views.Accessibility.AccessibilityEvent.cs:		public static unsafe string? EventTypeToString ([global::Android.Runtime.GeneratedEnum] Android.Views.Accessibility.EventTypes eventType)
src/Mono.Android/obj/Debug/monoandroid10/android-30/mcw/Android.Views.Accessibility.AccessibilityEvent.cs:		[return:global::Android.Runtime.GeneratedEnum]
src/Mono.Android/obj/Debug/monoandroid10/android-30/mcw/Android.Views.Accessibility.AccessibilityEvent.cs:		public static unsafe Android.Views.Accessibility.AccessibilityEvent? Obtain ([global::Android.Runtime.GeneratedEnum] Android.Views.Accessibility.EventTypes eventType)
src/Mono.Android/obj/Debug/monoandroid10/android-30/mcw/Android.Views.Accessibility.AccessibilityEvent.cs:		public unsafe void SetAction ([global::Android.Runtime.GeneratedEnum] Android.AccessibilityServices.GlobalAction action)
src/Mono.Android/obj/Debug/monoandroid10/android-30/mcw/Android.Views.Accessibility.AccessibilityEvent.cs:		public unsafe void WriteToParcel (Android.OS.Parcel? parcel, [global::Android.Runtime.GeneratedEnum] Android.OS.ParcelableWriteFlags flags)
src/Mono.Android/obj/Debug/monoandroid10/android-30/mcw/Android.Text.Method.MetaKeyKeyListener.cs:		public static unsafe void ClearMetaKeyState (Android.Text.IEditable? content, [global::Android.Runtime.GeneratedEnum] Android.Views.MetaKeyStates states)
src/Mono.Android/obj/Debug/monoandroid10/android-30/mcw/Android.Text.Method.MetaKeyKeyListener.cs:		public virtual unsafe void ClearMetaKeyState (Android.Views.View? view, Android.Text.IEditable? content, [global::Android.Runtime.GeneratedEnum] Android.Views.MetaKeyStates states)
src/Mono.Android/obj/Debug/monoandroid10/android-30/mcw/Android.Text.Method.MetaKeyKeyListener.cs:		public virtual unsafe long ClearMetaKeyState ([global::Android.Runtime.GeneratedEnum] Android.Views.MetaKeyStates state, [global::Android.Runtime.GeneratedEnum] Android.Text.Method.MetaStates which)
src/Mono.Android/obj/Debug/monoandroid10/android-30/mcw/Android.Text.Method.MetaKeyKeyListener.cs:		public static unsafe int GetMetaState (Java.Lang.ICharSequence? text, [global::Android.Runtime.GeneratedEnum] Android.Text.Method.MetaStates meta)
src/Mono.Android/obj/Debug/monoandroid10/android-30/mcw/Android.Text.Method.MetaKeyKeyListener.cs:		public static int GetMetaState (string? text, [global::Android.Runtime.GeneratedEnum] Android.Text.Method.MetaStates meta)

I think it should be fine to remove the [GeneratedEnum] attribute during .apk linking.

@radekdoulik
Copy link
Member Author

@radekdoulik: what do you mean by "attribute instances"? I don't understand the question.

I mean the instances in the assemblies like:

  .method public hidebysig newslot virtual final
          instance void  Require(valuetype Org.XmlPull.V1.XmlPullParserNode 'type',
                                 string namespace,
                                 string name) cil managed
  {
    .param [1]
    .custom instance void Android.Runtime.GeneratedEnumAttribute::.ctor() = ( 01 00 00 00 )
    // Code size       161 (0xa1)
    .maxstack  4
    .locals init (native int V_0,
             native int V_1,
             valuetype Android.Runtime.JValue* V_2,
             bool V_3)
...

And whether they are used during runtime, which I think are not. The above mentioned commit message says they are not used yet, so I wanted to be sure we didn't start using them later. I didn't find any place, so hopefully that's the case.

@jonpryor
Copy link
Member

jonpryor commented Jan 7, 2021

[GeneratedEnumAttribute] is not used at run-time. It is used at binding time.

It should be safe to remove them during linking. They're not needed for JCW creation.

@jonpryor jonpryor merged commit 55dbc33 into dotnet:master Jan 7, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants