[TrimmableTypeMap] Enable JNI replacement APIs#11270
[TrimmableTypeMap] Enable JNI replacement APIs#11270simonrozsival wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends JNI remapping support to the trimmable typemap runtime path and updates the build/runtime plumbing so remapping metadata can be reused across native and trimmable type managers. It fits into the ongoing trimmable typemap work by bringing replacement-type/method handling closer to feature parity with the existing runtime path.
Changes:
- Extract shared JNI remapping lookup logic into a new
JniRemappingLookuphelper and wire it into both runtime type manager implementations. - Pass remapping XML into native application-config generation and add a build test for preserving remapping counts across an incremental build where remap native code generation is skipped.
- Re-enable previously excluded Java.Interop remapping tests for the trimmable typemap lane.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.RuntimeTests/NUnitInstrumentation.cs | Removes trimmable-only exclusions for Java.Interop remapping tests. |
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets | Passes remapping XML into native app-config generation. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/TrimmableTypeMapBuildTests.cs | Adds incremental-build coverage for remapping counts in application config. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs | Falls back to reading remap metadata directly when task-object state is unavailable. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateJniRemappingNativeCode.cs | Extracts reusable XML parsing/counting logic for remapping metadata. |
| src/Mono.Android/Mono.Android.csproj | Includes the new shared remapping lookup source file. |
| src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMapTypeManager.cs | Enables replacement type/method and desugar fallback lookup for trimmable typemap. |
| src/Mono.Android/Microsoft.Android.Runtime/ManagedTypeManager.cs | Reuses the shared fallback-type helper. |
| src/Mono.Android/Microsoft.Android.Runtime/JniRemappingLookup.cs | Introduces shared native remapping lookup helpers. |
| src/Mono.Android/Android.Runtime/AndroidRuntime.cs | Replaces duplicated AndroidTypeManager remapping logic with shared helper calls. |
simonrozsival
force-pushed
the
dev/simonrozsival/trimmable-replacements
branch
from
d530869 to
9eb074f
Compare
simonrozsival
changed the base branch from
trimmable-typemap-startup-fixes
to
main
Share JNI remapping lookup between native and trimmable typemap type managers and enable replacement type and method lookup for TrimmableTypeMapTypeManager. Re-enable the CoreCLRTrimmable Java.Interop replacement tests covered by the implementation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
force-pushed
the
dev/simonrozsival/trimmable-replacements
branch
from
9eb074f to
8c1d5a2
Compare
|
/review |
simonrozsival
added
copilot
|
✅ Android PR Reviewer completed successfully! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ LGTM — Clean refactoring with a subtle correctness fix
Summary: Extracts shared JNI remapping logic from AndroidTypeManager into a new JniRemappingLookup helper class, reuses it across all three type managers, and enables replacement type/method lookup for TrimmableTypeMapTypeManager. Well-structured and minimal diff.
Positive callouts:
- The shared
JniRemappingLookupeliminates three copies of the desugar-type construction and provides a single point of truth - Nullable improvements on the interop struct fields (
string?+ explicit validation) are a correctness win over the old non-nullable fields thatMarshal.PtrToStructurecould silently return null for - Typo fix:
"one one of"→"one of"in the log message - Test exclusion removal is consistent with the feature enablement
One item to document: The shared code appends $_CC to the first fallback type (DesugarFoo$_CC), while the old AndroidTypeManager returned just DesugarFoo. This aligns all three type managers and is likely the correct behavior for D8/R8 companion classes, but worth noting in the commit message since it's a behavioral change beyond pure refactoring.
| Severity | Count |
|---|---|
| 💡 Suggestion | 3 |
CI: license/cla ✅, dotnet-android (public) ✅. Internal Xamarin.Android-PR pipeline status not visible from public checks — maintainer should verify.
Generated by Android PR Reviewer for issue #11270 · ● 4.9M
| ? $"{jniSimpleReference.Substring (0, slash + 1)}Desugar{jniSimpleReference.Substring (slash + 1)}" | ||
| : $"Desugar{jniSimpleReference}"; | ||
|
|
||
| var typeWithPrefix = $"{desugarType}$_CC"; |
There was a problem hiding this comment.
🤖 💡 Correctness — The old AndroidTypeManager.GetStaticMethodFallbackTypesCore set typeWithPrefix = desugarType.ToString() (e.g. com/example/DesugarFoo — no $_CC), while this shared implementation appends $_CC (→ com/example/DesugarFoo$_CC).
This aligns AndroidTypeManager with ManagedTypeManager and TrimmableTypeMapTypeManager, both of which already included $_CC. Given that D8/R8 desugared companion classes use $_CC, the old AndroidTypeManager was likely the odd one out, and device tests passing confirms this works — just worth a note in the commit message that this is an intentional fix alongside the refactoring.
Rule: Commit messages omit non-obvious choices (Postmortem #69)
Summary
TrimmableTypeMapTypeManagerImplementation note
The shared replacement lookup logic lives in
dotnet/androidasJniRemappingLookuprather than moving into the common Java.Interop type-manager base class. The base layer is in theexternal/Java.Interopsubmodule, while this lookup depends on Android-specific runtime state and native entry points (JNIEnvInit.jniRemappingInUse,_monodroid_lookup_replacement_type, and_monodroid_lookup_replacement_method_info). Keeping the helper in this repo avoids a Java.Interop/submodule change while still sharing the logic betweenAndroidTypeManagerandTrimmableTypeMapTypeManager.Test Plan
./dotnet-local.sh test bin/TestDebug/net10.0/Xamarin.Android.Build.Tests.dll --filter "Name=Build_WithTrimmableTypeMap_RemappingCountsAreInApplicationConfig" -- NUnit.NumberOfTestWorkers=1Mono.Android.NET-Testsdevice lane: 887 total, 0 failuresMono.Android.NET-Testsdevice lane: 273 total, 0 failuresMono.Android.NET-Testsdevice lane: 273 total, 0 failuresRebased on
main.Closes #10968