Remove managed NativeAOT type map in favor of the trimmable type map#11643
Remove managed NativeAOT type map in favor of the trimmable type map#11643simonrozsival wants to merge 2 commits into
Conversation
Use the trimmable type map for NativeAOT builds and remove the managed NativeAOT type map runtime and ILLink generator path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Keep NativeAOT ILLink-to-ILC plumbing unchanged, make NativeAOT runtime fail when the trimmable typemap switch is disabled, and remove only the managed typemap device test case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
changed the title
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy “managed” NativeAOT typemap pipeline and standardizes NativeAOT on the trimmable typemap, updating build validation, runtime behavior, and tests accordingly.
Changes:
- Make NativeAOT default to
_AndroidTypeMapImplementation=trimmableand reject other typemap implementations for NativeAOT builds. - Remove the managed NativeAOT typemap implementation (runtime types + ILLink
TypeMappingStep) and associated build plumbing. - Update tests/comments to drop managed typemap coverage and align wording with the new typemap behavior.
Show a summary per file
| File | Description |
|---|---|
| tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs | Removes “managed” typemap test cases and updates ignore text for MonoVM dotnet run. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/ObjectTest.cs | Updates TODO wording to reflect current typemap behavior issues on CoreCLR. |
| tests/Mono.Android-Tests/Java.Interop-Tests/Java.Interop-Tests.NET.csproj | Updates comments describing disabled tests to refer to typemap behavior (not “managed typemaps”). |
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets | Removes “managed” from typemap validation; enforces NativeAOT=trimmable; tightens typemap target imports. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs | Updates NativeAOT test to validate presence of generated trimmable typemap assemblies instead of managed typemap IL. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateTypeMappings.cs | Removes non-llvm-ir branching and clarifies NativeAOT skips (trimmable typemap path). |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets | Adjusts documentation comment for trimmable typemap description. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.LlvmIr.targets | Removes passing of typemap implementation to GenerateTypeMappings and deletes ILLink custom step wiring for managed typemap generation. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.RuntimeConfig.targets | Removes the runtimeconfig switch for RuntimeFeature.ManagedTypeMap. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.NativeAOT.targets | Changes NativeAOT default typemap implementation from managed to trimmable. |
| src/Mono.Android/Mono.Android.csproj | Stops compiling managed typemap runtime sources. |
| src/Mono.Android/Microsoft.Android.Runtime/RuntimeFeature.cs | Removes RuntimeFeature.ManagedTypeMap feature switch. |
| src/Mono.Android/Android.Runtime/JNIEnvInit.cs | Removes ManagedTypeManager selection; aligns NativeAOT behavior with trimmable typemap type manager. |
| src/Mono.Android/Microsoft.Android.Runtime/ManagedTypeMapping.cs | Deleted: legacy managed typemap mapping implementation. |
| src/Mono.Android/Microsoft.Android.Runtime/ManagedTypeManager.cs | Deleted: legacy managed typemap JniTypeManager implementation. |
| src/Microsoft.Android.Sdk.ILLink/TypeMappingStep.cs | Deleted: legacy ILLink typemap generation step for NativeAOT managed typemap. |
| src/Microsoft.Android.Sdk.ILLink/Microsoft.Android.Sdk.ILLink.csproj | Removes Java.Interop typemap-related imports/sources no longer needed after deleting TypeMappingStep. |
| src/Microsoft.Android.Runtime.NativeAOT/Java.Interop/JreRuntime.cs | Adds fail-fast exception when RuntimeFeature.TrimmableTypeMap is disabled under NativeAOT. |
Copilot's findings
- Files reviewed: 18/18 changed files
- Comments generated: 1
| foreach (AndroidRuntime runtime in Enum.GetValues (typeof (AndroidRuntime))) { | ||
| AddTestData (true, "llvm-ir", runtime); | ||
| AddTestData (false, "llvm-ir", runtime); | ||
| AddTestData (true, "managed", runtime); | ||
| // NOTE: TypeMappingStep is not yet setup for Debug mode | ||
| //AddTestData (false, "managed", runtime); | ||
| } |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ LGTM — Clean removal, well-structured
Summary: This PR removes the managed NativeAOT typemap (ManagedTypeManager/ManagedTypeMapping/TypeMappingStep) in favor of the trimmable typemap. The change is well-executed — defaults are updated, validation guards are added at both the MSBuild and runtime levels, and dead code paths are cleaned up consistently across the codebase.
What I verified
- ✅ No remaining references to
ManagedTypeManager,ManagedTypeMapping, orTypeMappingStepinsrc/ - ✅
_AndroidTypeMapImplementationdefaults are correct:trimmablefor NativeAOT (inNativeAOT.targets),llvm-irfor others (inDefaultProperties.targets) - ✅ MSBuild validation rejects
managedvalue and enforcestrimmablefor NativeAOT - ✅ Runtime fail-fast in
JreRuntime.CreateJreVM()for any bypass of build-time validation - ✅
Xamarin.Android.Common.targetsimport conditions updated correctly (== 'llvm-ir'instead of!= 'trimmable') — safe because defaults always populate the property - ✅ ILLink project correctly drops
Crc64/TypeNameMappings/NamingCustomAttributesreferences only used byTypeMappingStep
Suggestions (all 💡)
- Duplicate
!RuntimeFeature.TrimmableTypeMapguard inJreRuntime(unreachable inCreateDefaultTypeManager) - Defensive
IsNativeAotRuntimebranch inJNIEnvInit.CreateTypeManageris dead code — consider a clarifying comment - TODOs in test files should reference tracking issues
- Test coverage reduced significantly for NativeAOT typemap content — follow-up should be tracked
CI
CI is still in progress (some dotnet-android jobs cancelled/restarted, Windows build running). No failures visible yet. Xamarin.Android-PR has not triggered yet.
Note
NativeAOT.targets still has several Condition=" '$(_AndroidTypeMapImplementation)' != 'trimmable' " branches (lines 62, 140, 158, 184, 419) that are now dead code for NativeAOT since it always uses trimmable. These could be cleaned up in a follow-up.
Generated by Android PR Reviewer for issue #11643 · ● 35.2M
| if (!RuntimeFeature.TrimmableTypeMap) { | ||
| throw new NotSupportedException ("NativeAOT requires the trimmable typemap."); |
There was a problem hiding this comment.
🤖 💡 Code organization — This guard is unreachable. CreateJreVM (line 59–61) already throws NotSupportedException before builder.TypeManager ??= CreateDefaultTypeManager() on line 64 can execute. The duplicate check is harmless but adds dead code. Consider removing it and just keeping the unconditional return new TrimmableTypeMapTypeManager().
Rule: Remove unused code (Postmortem #58)
| var typemapDir = Path.Combine (intermediate, "typemap"); | ||
| FileAssert.Exists (Path.Combine (typemapDir, "_Microsoft.Android.TypeMaps.dll")); | ||
| FileAssert.Exists (Path.Combine (typemapDir, "_Mono.Android.TypeMap.dll")); |
There was a problem hiding this comment.
🤖 💡 Testing — The old test verified the typemap content (specific JNI name→managed type mappings, duplicate handling, Invoker precedence). The new test only checks that the typemap DLLs exist on disk. This is a significant reduction in test coverage. The PR description mentions "leaving trimmable test expansion to a separate PR" — is there a tracking issue for that follow-up?
Rule: Bug fixes need regression tests
| if (RuntimeFeature.IsNativeAotRuntime) { | ||
| return new TrimmableTypeMapTypeManager (); |
There was a problem hiding this comment.
🤖 💡 Code organization — Since NativeAOT now requires the trimmable typemap (enforced by both MSBuild validation and JreRuntime.CreateJreVM), and NativeAOT uses JreRuntime rather than JNIEnvInit.CreateTypeManager, this IsNativeAotRuntime branch is effectively dead code. It can't be reached by NativeAOT (which goes through JreRuntime), and for MonoVM/CoreCLR IsNativeAotRuntime is always false.
Not blocking — keeping it as a defensive fallback is reasonable, but a // Defensive: NativeAOT uses JreRuntime, not this path comment would clarify intent.
Rule: Remove stale comments (Postmortem #59)
| // TODO: fix on CoreCLR once the typemap behavior is fixed. Currently fails with | ||
| // java/lang/Object is typemap'd to Java.InteropTests.JavaLangRemappingTestObject, not Java.Lang.Object, Mono.Android! | ||
| [Test] | ||
| public void java_lang_Object_Is_Java_Lang_Object () |
There was a problem hiding this comment.
🤖 💡 Documentation — Same here: "TODO: fix on CoreCLR once the typemap behavior is fixed" would benefit from linking to a tracking issue. This comment is unchanged code, but the PR touches it — good opportunity to add the issue reference.
Rule: Track TODOs as issues (Postmortem #60)
| <!-- Some tests included in the files below fail on CoreCLR. This is PROBABLY caused by typemap behavior. | ||
| Until then, these tests can remain disabled. --> | ||
| <!-- TODO: put these back when the typemap behavior is fixed. --> |
There was a problem hiding this comment.
🤖 💡 Documentation — The comment says "Until then, these tests can remain disabled" but there's no issue reference tracking this. The TODO on line 47 also doesn't link to an issue. Consider filing an issue and referencing it here so this doesn't get forgotten.
Rule: Track TODOs as issues (Postmortem #60)
Summary
_AndroidTypeMapImplementation=trimmableand reject_AndroidTypeMapImplementation=managedthrough the existing typemap validationManagedTypeManager/ManagedTypeMapping) and the ILLinkTypeMappingStepgenerator pathRuntimeFeature.TrimmableTypeMapis disabledValidation
dotnet build src/Microsoft.Android.Runtime.NativeAOT/Microsoft.Android.Runtime.NativeAOT.csproj -p:Configuration=Debug --no-restore -v:minimal -nr:false -m:1dotnet build src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj -p:Configuration=Debug --no-restore -v:minimal -nr:false -m:1dotnet build src/Microsoft.Android.Sdk.ILLink/Microsoft.Android.Sdk.ILLink.csproj -p:Configuration=Debug -p:BuildProjectReferences=false --no-restore -v:minimal -nr:false -m:1Full
make prepare && make allis blocked locally by Android SDK/CMake staging/toolchain setup:cmake/3.30.3.stagingcould not be removed, followed by missingcmake/toolchain compiler errors.