[TrimmableTypeMap] Add trimmable array proxy mapping#11753
[TrimmableTypeMap] Add trimmable array proxy mapping#11753simonrozsival wants to merge 10 commits into
Conversation
Recreate the trimmable typemap array-mapping pieces from the Java.Interop integration branch so NativeAOT can resolve JNI array shapes without reflection or dynamic array construction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR extracts and lands the trimmable typemap “array slice” (from #11617) so the runtime and generator can resolve JNI array shapes via generated per-rank metadata and AOT-safe allocation (via generated JavaArrayProxy + newarr IL), avoiding reflection-based array construction on NativeAOT.
Changes:
- Introduces generated
JavaArrayProxymetadata and switches the trimmable runtime array lookup from “closed array Type” to “generated array proxy” per JNI leaf + rank. - Extends typemap generation to emit per-rank
__ArrayMapRankNentries pointing to generated array proxies, plus rank-scopedTypeMapAssociationentries matchingGetArrayTypes(). - Updates runtime array creation and compatibility checks (
JNIEnv) and trimmable container creation (JavaConvert/JavaPeerContainerFactory) to better support AOT/trimming scenarios.
Show a summary per file
| File | Description |
|---|---|
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/TrimmableTypeMapTypeManagerTests.cs | Updates runtime tests to validate TryGetArrayProxy() behavior and returned managed representations. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapModelBuilderTests.cs | Updates/extends generator model tests for array proxies, associations, primitive proxy synthesis, and PE blob round-trips. |
| src/Mono.Android/Mono.Android.csproj | Adds the new PrimitiveArrayInfo.cs to the Mono.Android build. |
| src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMap.cs | Replaces TryGetArrayType() with TryGetArrayProxy() and adds array-proxy caching. |
| src/Mono.Android/Microsoft.Android.Runtime/SingleUniverseTypeMap.cs | Renames array lookup API to return proxy types (TryGetArrayProxyType). |
| src/Mono.Android/Microsoft.Android.Runtime/PrimitiveArrayInfo.cs | Adds helper logic for primitive array wrappers/signatures/conversions (new file). |
| src/Mono.Android/Microsoft.Android.Runtime/ITypeMap.cs | Updates type map interface to resolve array proxy types instead of array Types. |
| src/Mono.Android/Microsoft.Android.Runtime/AggregateTypeMap.cs | Updates aggregate type map to forward TryGetArrayProxyType. |
| src/Mono.Android/Java.Interop/JavaPeerProxy.cs | Adds new public JavaArrayProxy : Attribute contract for generated proxies. |
| src/Mono.Android/Java.Interop/JavaPeerContainerFactory.cs | Adjusts container factory generic/DAM plumbing to support non-peer scalar element types on the trimmable path. |
| src/Mono.Android/Java.Interop/JavaConvert.cs | Adds scalar container factories and refactors factory-based conversions to support scalars + peers without MakeGenericType() on trimmable path. |
| src/Mono.Android/Android.Runtime/JNIEnv.cs | Uses generated array proxies for NativeAOT array creation; updates array compatibility checks to derive array JNI classes from element signatures. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs | Emits array proxy types, runtime type signatures for ldtoken/newarr, and rank-scoped association attribute ctor wiring. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/RootTypeMapAssemblyGenerator.cs | Ensures per-rank proxy maps are materialized alongside external maps for array rank groups. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/PEAssemblyBuilder.cs | Adds helper to encode runtime type signatures (incl. SZARRAY and primitives). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ModelBuilder.cs | Generates array proxy model entries + associations; synthesizes primitive array proxies for _Java.Interop.TypeMap. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/Model/TypeMapAssemblyData.cs | Adds model types for array proxy emission and rank-scoped associations. |
Copilot's findings
- Files reviewed: 17/17 changed files
- Comments generated: 4
Use the simple assembly-qualified managed element type name as the generated array map key, avoiding managed-to-JNI signature lookup when resolving array proxies at runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename TypeMapAttributeData.JniName to MapKey and use managedName in array-specific PE assertions so managed-key array maps are not described as JNI names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add public API baseline entries for JavaArrayProxy and rename sbyte array proxy locals in runtime tests to avoid byte/sbyte ambiguity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use the existing JavaPeerContainerFactory<T>.Instance singletons for scalar JavaConvert factories so Mono.Android can compile with the factory constructor kept private. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
⚠️ Needs Changes
Nice slice out of #11617 — the array-proxy design is genuinely AOT-friendly and the refactor is careful. One real cleanup item plus a few polish suggestions, and CI isn't green yet.
Findings
- 1
⚠️ warning —PrimitiveArrayInfo.cs(~224 lines) is compiled intoMono.Androidbut has zero callers; it's dead code as currently wired. Either integrate it (with a test), defer it to the slice that consumes it, or remove it. (inline) - 3 💡 suggestions — missing XML docs on the new public
JavaArrayProxyAPI; an unexplainedGetProxy<...>()+PopinEmitArrayMapsByRank; and a manualArgumentOutOfRangeExceptionthat could useThrowIfNegativeOrZero. (all inline)
What looks good
- Clean, mechanical
JniName→MapKeyrename throughout. - Array-proxy lookup path avoids
MakeGenericTypeby routing through the generated proxy +JavaPeerContainerFactory<T>— fullynew-based and AOT-safe. - Scalar container support via the static
ScalarContainerFactoriesmap is a tidy, allocation-light addition. - Solid coverage: PE round-trip tests, generator/model tests, and runtime
TryGetArrayProxyrank tests.
Verified as not issues (checked against full context)
FindArrayClassByElementTypeelement-type→array-class+1rank arithmetic is correct.AddArrayRank/assembly-qualified key manipulation (LastIndexOf (", ")) is sound._arrayProxyCache(ConcurrentDictionary+ static lambda + sentinel) is thread-safe.- Relaxing the
class, IJavaPeerableconstraint onJavaPeerContainerFactory<T>is intentional and required for scalar element types.
CI
The internal Azure DevOps builds (Linux/Mac/Windows) were still in progress / pending at review time — license/cla passed but the pipeline isn't green. Please confirm CI is green before merge; the
Generated by Android PR Reviewer for issue #11753 · 1.7K AIC · ⌖ 48.1 AIC · ⊞ 40K
Comment /review to run again
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Add XML doc comments to the new public JavaArrayProxy attribute and its
GetArrayTypes ()/CreateManagedArray (int) members, consistent with the
sibling JavaPeerProxy types.
Document why EmitArrayMapsByRank discards the
GetOrCreateProxyTypeMapping<__ArrayMapRank{N}> () result: the call roots the
per-rank [TypeMapAssociation] entries so the trimmer/ILC keeps the proxy
types referenced by the external map, even though the proxy map itself is
never stored at runtime.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Self-review turned up two minor 💡 nits, both purely cosmetic with no behavioral, perf, or public-API impact:
Not worth doing right now: the |
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
🤖 Automated review — [TrimmableTypeMap] Add trimmable array proxy mapping
Focused review of the array type-mapping extraction. The core redesign — replacing MakeArrayType() / Array.CreateInstanceFromArrayType with generator-emitted JavaArrayProxy types resolved through TrimmableTypeMap.TryGetArrayProxy → CreateManagedArray — is coherent and reads well for an extraction PR.
Verified as consistent:
- The new managed-type-key scheme (
"{FullName}, {assembly}", with primitives andstringpinned toSystem.Runtime) matches on both sides: generatorTypeMapAssemblyEmitter/ModelBuildervs. runtimeTryGetManagedTypeKey/GetAssemblyNameForManagedTypeKey. ArrayCreateInstanceis only reached forIJavaObject/Arrayelement types; primitive single-rank arrays keep their dedicated fast-path handlers, so thebyte[]/int[]/etc. copy paths are unaffected.- Tests cover object and primitive leaves at multiple ranks (
TryGetArrayProxy_*), and the newJavaArrayProxyAPI is added to all fourPublicAPI.Unshipped.txtfiles.
Please address before merge:
⚠️ PrimitiveArrayInfo.csis unused (~224 lines, zero callers) — wire it up or move it to the follow-up that consumes it, rather than shipping dead code inMono.Android.dll. (inline)- 💡 Stale comment in
JNIEnv.ArrayCreateInstancestill referencesIsDynamicCodeSupported/[FeatureGuard]after the switch toIsCoreClrRuntime. (inline)
CI: the internal Azure DevOps Linux > Build leg is green (so this compiles); the MSBuild/emulator test legs were still in progress at review time, so I'm not asserting test outcomes.
Automated review — not a substitute for maintainer sign-off, and I don't submit approvals. The "request changes" status reflects the unused-file item; feel free to dismiss if PrimitiveArrayInfo is intentionally staged for the next PR in the stack.
Generated by Android PR Reviewer for issue #11753 · 1.6K AIC · ⌖ 48.1 AIC · ⊞ 40K
Comment /review to run again
These nitpicks are irrelevant.
The trimmable array proxy codegen makes array creation AOT-safe and removes two reflection-based IL3050 warnings, dropping the NativeAOT MSBuild warning count from 10 to 8 (verified from the apk and aab build logs on build 1483030). Also report the actual vs expected warning count: parse the "N Warning(s)" MSBuild summary line and assert via Assert.AreEqual so a future mismatch shows the real number instead of "Expected: True But was: False". Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- JavaConvert.GetJniHandleConverter: drop the redundant "target.IsGenericType && !target.IsGenericTypeDefinition" guard at the call site; TryGetFactoryBasedConverter's local helpers already perform that check, so the guard was triplicated. - JavaPeerContainerFactory.Create<T>: document why the open JavaPeerContainerFactory<T> is unconstrained (internal scalar factories) while the public Create<T> keeps the IJavaPeerable constraint. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the where T : class, IJavaPeerable constraint so Create<T> matches the already-unconstrained JavaPeerContainerFactory<T>, and drop the asymmetry remarks that explained the now-removed mismatch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
Review summary
Thanks @simonrozsival — this cleanly extracts the array type-mapping slice from #11617. The JniName → MapKey rename (managed-type-name keys like Android.App.Activity, Mono.Android) is consistent and well covered by the model/PE round-trip tests, the per-rank TrimmableTypeMap.TryGetArrayProxy design and JavaArrayProxy abstraction read well, and the AOT-safe newarr-based CreateManagedArray is a nice touch. I also like that BuildTest2 now asserts the actual vs. expected NativeAOT warning count with a useful message instead of a bare number.
A couple of things to address before merge:
⚠️ PrimitiveArrayInfois unused — ~220 lines with no callers; either wire it up to its consumer or drop it from this PR (inline comment).- 💡 Stale comment in
JNIEnv.ArrayCreateInstance— still references the oldIsDynamicCodeSupported[FeatureGuard]after the switch toRuntimeFeature.IsCoreClrRuntime(inline comment). Behavior is correct; the comment just no longer matches the code.
Things I checked that are fine:
- IL3050 suppression is sound —
Microsoft.Android.Sdk.NativeAOT.targetssetsIsCoreClrRuntime = falsewithTrim="true", so theArray.CreateInstancebranch is removed under NativeAOT. TryGetFactoryBasedConverterdropping its innerRuntimeFeature.TrimmableTypeMapguard is safe — the caller inGetJniHandleConverterstill guards it.GetAssemblyNameForManagedTypeKeyhardcodingSystem.Runtimefor primitives/stringcorrectly mirrors the generator (runtime would otherwise seeSystem.Private.CoreLib).
Note: the internal CI build is still pending and the PR is mergeable_state: blocked, so I can't confirm a green build from here.
Requesting changes primarily for the unused PrimitiveArrayInfo; the rest is minor. Nice work overall.
Generated by Android PR Reviewer for issue #11753 · 1.3K AIC · ⌖ 47.4 AIC · ⊞ 37.9K
Comment /review to run again
Goal
Extract the array type-mapping slice from #11617 so the trimmable typemap can resolve and allocate JNI array shapes without relying on reflection-only or dynamic-code array construction paths.
#11617 implements the broader reflection-free
TrimmableTypeMapValueManager/TrimmableTypeMapTypeManagerdesign. This PR recreates the pieces of that work specifically needed for array mapping:JavaArrayProxymetadata,PrimitiveArrayInfo,JavaConvertcontainer factory support for scalar and Java peer element types,__ArrayMapRankNcodegen and loader wiring,Background from #11617
The larger #11617 design makes generated typemap metadata the source of truth for trimmable/NativeAOT Java interop. One important part of that is array handling: NativeAOT cannot safely fall back to
Array.CreateInstance()/MakeArrayType()for all shapes, and the trimmable path should not depend on late reflection to discoverT[],JavaArray<T>,JavaObjectArray<T>, or primitive array wrapper representations.Instead, #11617 generates rank-scoped typemap groups and a proxy type for each supported array shape. This PR ports that array-specific design into this branch.
Change map
Runtime
Java.Interop.JavaArrayProxy, a self-applied generated attribute/proxy base with:GetArrayTypes()for all managed representations that map to a JNI array shape.CreateManagedArray(int length)for NativeAOT-safe direct array allocation.TrimmableTypeMapfrom directTypearray lookup to generated array-proxy lookup:TryGetArrayProxy(Type elementType, int additionalRank, out JavaArrayProxy?).Android.Net.Network, Mono.Android), avoiding a managed-to-JNI signature lookup during runtime array proxy resolution.JNIEnv.ArrayCreateInstance():Array.CreateInstance().JavaArrayProxy.CreateManagedArray().PrimitiveArrayInfofor primitive JNI array metadata and wrappers.JNIEnvto derive JNI array classes from element type signatures instead of usingMakeArrayType().JavaConvert/JavaPeerContainerFactoryso trimmable conversions can create generic Java containers for scalar element types as well as Java peer types without usingMakeGenericType()on the trimmable path.JavaPeerContainerFactory<T>and itsCreate<T>helper are unconstrained (noIJavaPeerablerequirement) so scalar element types such asintandstringalso get a container factory.Code generation
ArrayProxyDataandPrimitiveArrayProxyDatato the typemap model._Java.Interop.TypeMapfor primitive JNI keywords (Z,B,C,S,I,J,F,D).TypeMap<__ArrayMapRankN>entries keyed by simple assembly-qualified managed element type names and pointing at generated array proxies, instead of pointing directly at closed array types.TypeMapAssociation<__ArrayMapRankN>entries for each managed representation returned by the proxy:JavaObjectArray<T>,JavaArray<T>,T[];T[],JavaArray<T>,JavaPrimitiveArray<T>, concrete wrappers such asJavaSByteArray;JavaObjectArray<...>and SZ-array forms.__ArrayMapRankNgroup, so rank-scoped associations participate in trimming and runtime lookup.ldtoken/newarremission.Why this shape
The previous local implementation mapped
__ArrayMapRankNentries directly to closed managed arrayTypes and usedArray.CreateInstanceFromArrayType()as the NativeAOT allocation path. The first version of this extracted PR keyed generated array maps by JNI element name. The current shape keys them by the simple assembly-qualified managed element type name so runtime lookup starts from theTypeit already has and avoids another managed-to-JNI lookup. The #11617 design is more complete because the generated proxy can describe all managed representations of a JNI array shape, not just oneT[]target, and can allocate arrays with directnewarrIL.That matters for NativeAOT because array metadata/allocation support depends on what is rooted. The generated proxy makes the needed array shape explicit and avoids late reflection fallback.
Tests
Updated coverage includes:
GetArrayTypes();_Java.Interop.TypeMap;TypeMapandTypeMapAssociationblobs;TryGetArrayType()toTryGetArrayProxy(), including managed-key rank-map lookup behavior;BuildTest2.BuildHasNoWarningsNativeAOT expected warning count updated from 10 to 8 — the generated array proxies make two previously reflection-based array-creation paths AOT-safe, removing twoIL3050warnings. The assertion now reports the actual vs expected warning count instead of a bareExpected: True But was: False.Validation
Passed locally:
dotnet test tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj --no-restore -v:minimalResult: 563 passed.
Also attempted
src/Mono.Android/Mono.Android.csprojbuild. After initializingexternal/Java.Interopand providing localjavac/jarpaths, the build progressed past C# compile checks visible in the log but was stopped when a Gradle dependency step requested interactive Azure DevOps credential login. No C# errors were present in the captured output before that prompt.NativeAOT warning impact
A measurable outcome of routing array creation through generated proxies: a NativeAOT app build now emits 8
IL3050AOT-analysis warnings instead of 10. The two removed warnings came from the reflection-based array-creation paths (Array.CreateInstance()/MakeArrayType()) that the generatedJavaArrayProxy.CreateManagedArray()replaces. Verified from theBuildHasNoWarningsapk and aab NativeAOT build logs.