[CoreCLR] Remove unused marshal methods code #10180
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
df5f231 to
73200bd
Compare
73200bd to
c101949
Compare
There was a problem hiding this comment.
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; } = "";
jonathanpeppers
approved these changes
Jun 12, 2025
| 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.
As long as none of the NativeAOT tests get here, this looks good. 👍
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 n 8000 ames, 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)