[NativeAOT] Remove managed NativeAOT type map in favor of the trimmable type map#11643
[NativeAOT] Remove managed NativeAOT type map in favor of the trimmable type map#11643simonrozsival wants to merge 6 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)
simonrozsival
added
copilot
simonrozsival
changed the title
|
@copilot address code review |
Updated |
…Reference The Java.Interop bump (external/Java.Interop -> 6820a9cb) shifted the trim-analysis warning on `ManagedTypeManager.GetTypeForSimpleReference`: its return value flows from the un-annotated `out` parameter of `ManagedTypeMapping.TryGetType (string, out Type)`, so it no longer satisfies the method's `[return: DynamicallyAccessedMembers]` and the trimmer now reports IL2068 (previously IL2063). This unsuppressed warning inflates the trim warning count and breaks the `BuildHasTrimmerWarnings` expectations. Mirror the existing IL2063 suppression on the sibling `TrimmableTypeMapTypeManager.GetTypeForSimpleReference`. `ManagedTypeManager` is removed in #11643. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@copilot resolve the merge conflicts in this pull request and look for opportunity to remove the System.IO.Hashing package reference and the version number in Versions.props |
- Resolve merge conflicts: - ManagedTypeManager.cs: keep deletion (from our PR) - Mono.Android.csproj: include AndroidReflectionJniValueManager.cs (from main), remove ManagedTypeManager/ManagedTypeMapping (our PR) - Remove System.IO.Hashing PackageReference from Mono.Android.csproj and Microsoft.Android.Runtime.NativeAOT.csproj; these projects no longer use it after ManagedTypeMapping.cs (the sole XxHash3 user) was deleted Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Done in ef34ac3:
|
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Done. Resolved conflicts with the latest main:
Also added the missing |
#11798) ## Problem When R8 code shrinking is enabled, R8 removes any Java type that nothing keeps. .NET for Android keeps the Java callable wrappers (JCWs) of bound managed types by generating `-keep` rules from the **acw-map** (managed peer ↔ Java type). That works for generated JCWs, but **user-authored `AndroidJavaSource` `.java` files marked `Bind` != `true` have no managed peer**, so they never appear in the acw-map — and therefore get no keep rule. With shrinking on, R8 silently deletes them, and the app fails at runtime when it references those classes (often as a `NoClassDefFoundError` that only reproduces in Release/shrunk builds). ## Fix Pass user `AndroidJavaSource` (`Bind` != `True`) to the `R8` task and emit `-keep class <package>.<Type> { *; }` for each, so user Java survives shrinking: * **`D8.targets`** — collect `@(AndroidJavaSource)` items where `%(Bind) != 'True'` into `_R8KeepJavaSource` and pass them to `R8` via the new `JavaSourceFiles` property. * **`R8.cs`** — append keep rules in the same block that emits acw-map keeps. `GetUserJavaTypes()` derives the fully-qualified name as `<package>.<FileNameWithoutExtension>` (Java requires the public top-level type name to match the file name); `ReadJavaPackage()` parses the `package` declaration, skipping comments and stopping at the first `import`/type declaration. Names are de-duplicated. * **`R8Tests.cs`** — unit tests covering `ReadJavaPackage`: package present, trailing space before `;`, comment headers, no package, and `package` after `import`/type (ignored). ## Why split out This is being carved out of the trimmable typemap mega-PR #11617. Although the bug surfaced while bringing up the NativeAOT trimmable path, **it is not trimmable-specific** — the acw-map keep logic and R8 shrinking apply equally to legacy/MonoVM and CoreCLR, so user `AndroidJavaSource` was at risk of being shrunk there too. Landing it independently de-risks #11617 and ships a general correctness fix sooner. ## Context / related PRs - #11617 — parent mega-PR (reflection-free trimmable typemap) this is sliced from. - #11643 — removes the managed NativeAOT typemap in favor of the trimmable one (the path that exposed this gap). - Sibling slices already split out off `main`: #11794 (AndroidApplicationJavaClass/multidex), #11796 (manifest generation parity). Other merged groundwork: #11749, #11751, #11752, #11753, #11764, #11769. ## Verification 24/24 R8 unit tests pass (18 existing + 6 new). `BuildAfterMultiDexIsNotRequired` verified locally on NativeAOT and CoreCLR — user Java is retained after shrinking. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary Adapts the local agent CI guidance to the current dotnet/android PR validation pipeline. **Docs/skills only — no product code.** PR validation runs on the public **`dotnet-android`** Azure DevOps pipeline on `dev.azure.com/dnceng-public` (project `public`, definition `333`) for every PR. ## Changes - Rework the **`ci-status`** skill around the single public `dotnet-android` build. - Extract build IDs from GitHub check links and query `dnceng-public`/`public` directly. - Handle fork/direct PRs as trigger/approval differences, not different pipeline selections. - Use `az rest` + `ResultsByBuild` for failed-test queries because the normal AZDO test `runs` API path is broken on dnceng-public. - Add failure summaries, cross-config failed-test matrix reporting, crash/incomplete-lane detection, and clearer gating-vs-flaky verdicts. - Update `.github/copilot-instructions.md` for the public pipeline. ## Validation Exercised the updated `ci-status` skill end-to-end against live PRs and builds: - In-progress build with a gating failure (#11659) — flagged the failed package-test stage and failed tests. - Green build with non-blocking flaky device-test failures (#11667) — reported green from build/check status while surfacing the failures as non-gating. - Fully red build (#11643) — reported red checks and failed tests. All `az`, `gh`, and `az rest` command paths used by the skill were run against `dnceng-public` and confirmed working.
Summary
_AndroidTypeMapImplementation=trimmableand reject_AndroidTypeMapImplementation=managedthrough the existing typemap validationManagedTypeManager/ManagedTypeMapping) and the ILLinkTypeMappingStepgenerator pathRuntimeFeature.TrimmableTypeMapis disabledSystem.IO.Hashingpackage references fromMono.AndroidandMicrosoft.Android.Runtime.NativeAOT(the only runtime user,ManagedTypeMapping.cs, was deleted); also remove theSystem.IO.Hashing.dllcopy fromMono.Android's_CopyToPackDirstarget since it is no longer needed in the Android runtime packDotNetRunnow exercisesNativeAOTwith the trimmable typemap instead of immediately ignoring all NativeAOT casesValidation
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.