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.

Sign up for GitHub

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

Jump to bottom

Review reflection usage in src/Mono.Android.Export/CallbackCode.cs #5206

Closed
radekdoulik opened this issue Oct 14, 2020 · 2 comments
Closed

Review reflection usage in src/Mono.Android.Export/CallbackCode.cs #5206

radekdoulik opened this issue Oct 14, 2020 · 2 comments
Labels
Area: Linker Issues when linking assemblies.
Milestone
.NET 8

Comments

@radekdoulik
Copy link
Member

radekdoulik commented Oct 14, 2020 •
edited by jonathanpeppers
Loading

We are preserving many types unconditinally in Mono.Android.xml because of reflection used here.

It should be possible to remove some of the types from the list as illink is smarter about reflection and hopefully also update some of the code to be more linker friendly.

Also use the new dependency attributes when possible.
@radekdoulik radekdoulik added the Area: Linker Issues when linking assemblies. label Oct 14, 2020
@radekdoulik radekdoulik added this to the One .NET milestone Oct 14, 2020
@radekdoulik radekdoulik self-assigned this Oct 14, 2020
radekdoulik added a commit to radekdoulik/xamarin-android that referenced this issue Dec 22, 2020
@radekdoulik
[illink] Do not preserve whole collection types
56f7474 
Context: dotnet#5167
Context: dotnet#5206

Instead of preserving these types unconditionally, use the new dynamic
dependency attributes

apksize impact on BuildReleaseArm64False test:

    > lapkdiff -f -e dll$ before.apk after.apk
    Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
      -         477 assemblies/Java.Interop.dll
      -         603 assemblies/System.Collections.Concurrent.dll
      -       1,388 assemblies/System.Private.CoreLib.dll
      -      30,291 assemblies/Mono.Android.dll
    Summary:
      -      32,759 Assemblies -4.19% (of 781,837)
      -      32,768 Package size difference -0.43% (of 7,645,409)
@radekdoulik radekdoulik mentioned this issue Dec 22, 2020
Merged
radekdoulik added a commit to radekdoulik/xamarin-android that referenced this issue Dec 28, 2020
@radekdoulik
[illink] Do not preserve IO stream adapter/invoker
2f374a1 
Context: dotnet#5206
Context: dotnet#5167

They are only accessed through `CallbackCode` class in M.A.Export with
reflection code, which ILLink is able to detect.

This change reduces the apk size in common situation, when M.A.Export is
not used.

apk size comparison, BuildReleaseArm64False test:

    > apkdiff -f -e dll$ before.apk after.apk
    Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
      -          80 assemblies/System.Console.dll
      -         129 assemblies/Java.Interop.dll
      -       1,237 assemblies/System.Private.CoreLib.dll
      -       6,093 assemblies/Mono.Android.dll
    Summary:
      -       7,539 Assemblies -1.01% (of 749,078)
@radekdoulik radekdoulik mentioned this issue Dec 28, 2020
Merged
jonpryor pushed a commit that referenced this issue Jan 4, 2021
@radekdoulik
[One .NET] Do not preserve IO stream adapter/invoker (#5449)
051cad7 
Context: #5167
Context: #5206

The following types are accessed through the `CallbackCode` type
in `src/Mono.Android.Export` via System.Reflection:

  * `Android.Runtime.InputStreamAdapter`
  * `Android.Runtime.InputStreamInvoker`
  * `Android.Runtime.OutputStreamAdapter`
  * `Android.Runtime.OutputStreamInvoker`

Historically the use of these types via Reflection required that they
always be preserved, just in case `Mono.Android.Export.dll` was used.

.NET 6 ILLink is able to detect such reflection use.  These types no
longer need to be explicitly preserved in all circumstances.

Stop explicitly preserving these types, and rely on ILLink to
preserve them when `Mono.Android.Export.dll` is used.

This change reduces the `.apk` size in a common situation, when
`Mono.Android.Export.dll` is not used.

apk size comparison, BuildReleaseArm64False test:

	> apkdiff -f -e dll$ before.apk after.apk
	Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
	  -          80 assemblies/System.Console.dll
	  -         129 assemblies/Java.Interop.dll
	  -       1,237 assemblies/System.Private.CoreLib.dll
	  -       6,093 assemblies/Mono.Android.dll
	Summary:
	  -       7,539 Assemblies -1.01% (of 749,078)
jonpryor pushed a commit that referenced this issue Jan 7, 2021
@radekdoulik
[One .NET] Do not preserve entire collection types (#5445)
15269f6 
Context: #5167
Context: #5206

Instead of preserving the `Android.Runtime.Java*` collection types
unconditionally, use the new dynamic dependency attributes from
7083eba.

`.apk` size impact on BuildReleaseArm64False test:

	> lapkdiff -f -e dll$ before.apk after.apk
	Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
	  -         477 assemblies/Java.Interop.dll
	  -         603 assemblies/System.Collections.Concurrent.dll
	  -       1,388 assemblies/System.Private.CoreLib.dll
	  -      30,291 assemblies/Mono.Android.dll
	Summary:
	  -      32,759 Assemblies -4.19% (of 781,837)
	  -      32,768 Package size difference -0.43% (of 7,645,409)
@jonathanpeppers jonathanpeppers modified the milestones: .NET 6, .NET 7 Jun 14, 2021
@jonpryor jonpryor modified the milestones: .NET 7, .NET 8 Jul 27, 2022
@jonpryor
Copy link
Member

Pie-in-the-sky alternative: if jnimarshalmethod-gen ever becomes A Thing, then we can type-forward Java.Interop.ExportAttribute, Mono.Android to Java.Interop.ExportAttribute, Java.Interop and have build-time marshal method generation for [Export] methods.

@jonpryor
Copy link
Member

This is already fixed, e.g. 15269f6, 6f9bc6b.

I don't see any [Preserve] in src/Mono.Android.Export.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Area: Linker Issues when linking assemblies.
Projects
None yet
Milestone
.NET 8
Development

No branches or pull requests
3 participants
@jonpryor @jonathanpeppers @radekdoulik