-
Notifications
You must be signed in to change notification settings - Fork 564
[CoreCLR] Remove unused marshal methods code #10180
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
[CoreCLR] Remove unused marshal methods code #10180
Conversation
df5f231 to
73200bd
Compare
73200bd to
c101949
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes unused CoreCLR-specific marshal methods and related data from the native runtime to reduce binary size and startup relocations, and refactors the MSBuild tasks to cleanly separate MonoVM vs. CoreCLR code generation.
- Eliminate CoreCLR marshal-method tables and stubs from native sources
- Split
MarshalMethodsNativeAssemblyGeneratorinto MonoVM and CoreCLR subclasses - Update the MSBuild task to dispatch based on an
AndroidRuntimeparameter
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/common/include/runtime-base/timing-internal.hh | Simplify timing log label from "; elapsed exp:" to "; elapsed:" |
| src/native/clr/xamarin-app-stub/application_dso_stub.cc | Remove #if RELEASE marshal-methods data structures |
| src/native/clr/include/xamarin-app.hh | Drop marshal-methods declarations under CoreCLR |
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets | Add AndroidRuntime attribute to GenerateNativeMarshalMethodSources |
| src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGeneratorMonoVM.cs | Introduce MonoVM-specific generator subclass |
| src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGeneratorCoreCLR.cs | Introduce CoreCLR-specific generator subclass |
| src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs | Refactor base generator to abstract and move CoreCLR/MonoVM logic |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeMarshalMethodSources.cs | Dispatch generator creation via AndroidRuntime switch |
Comments suppressed due to low confidence (4)
src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeMarshalMethodSources.cs:69
- The switch cases reference
Tasks.AndroidRuntime.MonoVMandTasks.AndroidRuntime.CoreCLR, butTasksisn’t an existing alias here. Use theAndroidRuntimeenum directly (e.g.AndroidRuntime.MonoVM) or add a properusingso the enum resolves correctly.
MarshalMethodsNativeAssemblyGenerator marshalMethodsAsmGen = androidRuntime switch {
src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs:135
- List fields should be initialized with a
new List<uint>()instead of[], which is valid only for array literals. Otherwise this will not compile.
public List<uint> Keys32 = [];
src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs:140
- Similarly, initialize
Keys64usingnew List<ulong>()instead of[]to avoid compile errors.
public List<ulong> Keys64 = [];
src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeMarshalMethodSources.cs:33
- [nitpick] Indent this property to match the other task properties for consistent formatting.
public string AndroidRuntime { get; set; } = "";
| MarshalMethodsNativeAssemblyGenerator marshalMethodsAsmGen = androidRuntime switch { | ||
| Tasks.AndroidRuntime.MonoVM => MakeMonoGenerator (), | ||
| Tasks.AndroidRuntime.CoreCLR => MakeCoreCLRGenerator (), | ||
| _ => throw new NotSupportedException ($"Internal error: unsupported runtime type '{androidRuntime}'") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as none of the NativeAOT tests get here, this looks good. 👍
Our build process targetting CoreCLR generated marshal methods code that is
only used by our MonoVM runtime. The native code included marshal methods and
class names, as well as assembly image cache which is not supported on CoreCLR.
All of the above data were stored as pointers in the resulting
libxamarin-app.so,each pointer being a relocation that has to be fixed up by the system dynamic linker
at application load time. The more relocations exist across the
.sofile, the moreof it the linker has to actually read into memory (instead of just mmapping it and
lazily loading whenever/if the code/data in that section is used).
This PR removes all of that code from the CoreCLR version of
libxamarin-app.so,together with a number of native structs supporting the data.
Testing the changes on a
dotnet new maui -scapplication, we can see the followingsavings:
libxamarin-app.sodrops from 903 to 394 entrieslibxamarin-app.sosize drops from:Testing the above MAUI app performance on Pixel 8, we can also see a tiny startup
time decrease by 0.71%. On slower devices and with bigger applications the startup
performance improvement will be bigger (the slower the device's storage, the bigger
time savings)