[TrimmableTypeMap] Add AOT-safe array and collection factories using MakeArrayType and MakeGenericType

[simonrozsival]

Summary

This PR adds runtime-only safe factory helpers for the trimmable type map path:

• SafeArrayFactory centralizes managed array Type lookup and array instance creation.
• SafeJavaCollectionFactory centralizes generic Java collection wrapper Type lookup and wrapper creation for JavaList<T>, JavaCollection<T>, and JavaDictionary<TKey,TValue>.
• ValueTypeFactory<T> is the shared primitive/nullable value-type rooting table used by both factories.
• JNIEnv, JavaConvert, and TrimmableTypeMapTypeManager now call these helpers instead of carrying their own trimmable-specific construction policy inline.

This also adds boxed nullable primitive array support (int?[], etc.) and runtime coverage for nullable array/list/dictionary marshaling.

Why using MakeArrayType() / MakeGenericType() is OK here

These APIs are annotated with RequiresDynamicCode / RequiresUnreferencedCode because the general case is unsafe in NativeAOT/trimming:

• arbitrary array element types can lack an array EEType/template;
• arbitrary constructed generic instantiations can lack a runtime template or constructor metadata;
• value-type generic instantiations are not canonically shared the same way reference-type instantiations are.

This PR does not suppress those warnings broadly. The suppressions are isolated in the safe factories and only after constraining the shapes to cases we know NativeAOT can support.

The runtime source path this is based on is:

• RuntimeType.NativeAot.cs -> RuntimeTypeInfo.MakeArrayType() / MakeGenericType()
• ExecutionEnvironmentImplementation.MappingTables.cs
• TypeLoaderEnvironment.cs
• TypeBuilder.TryBuildArrayType() / TypeBuilder.TryBuildGenericType()
• TemplateLocator.cs
• StandardCanonicalizationAlgorithm.cs

The key NativeAOT behavior is:

• Reference types canonicalize to __Canon, and reference arrays/generic instantiations can use canonical templates.
• Value types remain value-specific, so exact value-type arrays and exact value-type generic instantiations must be rooted.
• Array.CreateInstanceFromArrayType() immediately needs the array TypeHandle and allocates through RuntimeAugments.NewArray(), so it must only see array types that are actually runtime-buildable/rooted.

The factories enforce that model:

• Reference arrays use elementType.MakeArrayType() and then Array.CreateInstanceFromArrayType() only for NativeAOT-buildable reference-array canonical shapes.
• First-rank primitive/nullable value arrays are allocated directly through ValueTypeFactory<T>.CreateArray() (new T[length]), which roots the exact T[] vector.
• Additional array ranks are jagged SZArrays. Once the first vector exists, each outer wrapper has an array reference type as its element, so it follows the reference-array canonical path.
• Reference Java collection wrappers use typeof(JavaList<>).MakeGenericType(...), etc., relying on canonical generic templates such as JavaList<__Canon>.
• Primitive/nullable value Java collection wrappers use ValueTypeFactory<T> direct generic references and constructors, rooting exact instantiations such as JavaList<int> and JavaList<int?>.
• Mixed reference/value dictionaries root the canonical shapes with dedicated type tokens (JavaDictionary<object,T> and JavaDictionary<T,object>).

Unsupported value-type cases are rejected instead of falling through to unrooted MakeGenericType() / MakeArrayType() usage.

Scope

Included:

• JavaList<T> / IList<T>
• JavaCollection<T> / ICollection<T>
• JavaDictionary<TKey,TValue> / IDictionary<TKey,TValue>
• Java boxed primitive set and nullable forms: bool, sbyte, char, short, int, long, float, double, plus nullable variants
• reference-type array and wrapper shapes

Out of scope for this PR:

• JavaSet<T>
• byte and unsigned aliases
• removing generated array proxy code
• removing JavaPeerContainerFactory

Follow-up

This PR intentionally leaves generated array proxy code, JavaPeerContainerFactory, and related obsolete support code in place. A follow-up PR will remove that generator/runtime cleanup once this runtime-only factory path is validated.

Validation

• dotnet build src/Mono.Android/Mono.Android.csproj -v:minimal /p:Configuration=Debug --no-restore /p:JavaCPath=/Library/Java/JavaVirtualMachines/microsoft-21.jdk/Contents/Home/bin/javac /p:JarPath=/Library/Java/JavaVirtualMachines/microsoft-21.jdk/Contents/Home/bin/jar /p:JavaPath=/Library/Java/JavaVirtualMachines/microsoft-21.jdk/Contents/Home/bin/java
• dotnet test tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj -v:minimal --no-restore

[simonrozsival]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[Copilot]

Pull request overview

This PR refactors the trimmable typemap marshaling path to centralize AOT-safe construction of array and generic Java collection wrapper types, and extends runtime/test coverage to support nullable primitive arrays and nullable list/dictionary marshaling.

Changes:

• Introduces SafeArrayFactory, SafeJavaCollectionFactory, and ValueTypeFactory<T> to constrain and root AOT-safe array/generic instantiations.
• Updates JNIEnv, JavaConvert, and TrimmableTypeMapTypeManager to use the new factories and adds nullable primitive array support.
• Adds new tests and updates APK descriptor baselines to reflect output changes.

Show a summary per file

File	Description
tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaConvertTest.cs	Adds runtime tests for nullable IList<T> and IDictionary<TKey,TValue> conversions via JavaConvert.FromJniHandle.
tests/Mono.Android-Tests/Mono.Android-Tests/Android.Runtime/JnienvArrayMarshaling.cs	Adds test coverage for JNIEnv nullable primitive array round-tripping and item set/get.
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.NativeAOT.apkdesc	Updates expected APK file/package sizes after runtime/type-map changes.
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc	Updates expected APK file/package sizes after runtime/type-map changes.
src/Mono.Android/Mono.Android.csproj	Adds the new factory helper source files to the build.
src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMapTypeManager.cs	Extends NativeAOT array-type discovery to use SafeArrayFactory when a generated proxy isn’t present.
src/Mono.Android/Java.Interop/ValueTypeFactory.cs	Adds the shared primitive/nullable rooting table for AOT-safe arrays and collection wrappers.
src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs	Adds constrained AOT-safe generic wrapper type creation and handle-to-wrapper creation for lists/collections/dictionaries.
src/Mono.Android/Java.Interop/SafeArrayFactory.cs	Adds constrained AOT-safe array Type lookup and instance creation (including jagged arrays).
src/Mono.Android/Java.Interop/JavaConvert.cs	Switches trimmable-path generic collection conversion to the new factory-based approach.
src/Mono.Android/Android.Runtime/JNIEnv.cs	Routes trimmable array allocation through SafeArrayFactory and adds nullable primitive array marshaling support.

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 4

[github-actions]

🤖 Android PR Review — ⚠️ Needs Changes (minor)

Runtime-only AOT-safe factories for managed arrays and Java collections on the trimmable-type-map path. The core design is well-reasoned: value types are rooted explicitly (typeof(T[]) / new T[] / direct generic constructor references), reference types ride NativeAOT's __Canon canonical templates, and unsupported value-type shapes are rejected (return null) rather than silently reaching an unrooted MakeGenericType/MakeArrayType. The [UnconditionalSuppressMessage] attributes are narrowly scoped with sound justifications after the shapes have been constrained. 👍

I verified the removed CoreCLR Array.CreateInstance catch-all is not a regression: ArrayCreateInstance is only ever reached with nullable primitives (via CreateManagedArrayFromObjectArray) or reference types (IJavaObject/Array converters). Enums are mapped to their underlying primitive by GetConverter, and GetArray<T> uses new T[cnt] directly — so no non-primitive value type flows into SafeArrayFactory.CreateInstance.

Findings (all inline)

Sev	Location	Issue
⚠️	SafeJavaCollectionFactory.cs:139	Value/value IDictionary goes through the virtual-generic-method path (CreateDictionaryWithKey<TKey> → new JavaDictionary<TKey,T>) — the AOT-riskiest construct here — and has no test. Add an IDictionary<int,long> runtime test under the NativeAOT trimmable config.
💡	SafeJavaCollectionFactory.cs:77	TryCreateInstance(Type, object?, out ...) (construct-from-items) has zero callers.
💡	SafeArrayFactory.cs:19	Throwing GetArrayType(Type,int) overload is unused.
💡	JNIEnv.cs:954	Per-element closure/delegate allocation in CopyManagedObjectArray.

Positives

• Boxed nullable-primitive array / list / dictionary support with round-trip tests (GetArray_NullableInt32, FromJniHandle_IListNullableInt32, FromJniHandle_IDictionaryNullableInt32String).
• Clear inline rationale for why the trimmable path avoids the reflection fallback and why each rooting trick exists.
• Correct JNI local/global ref lifetime handling in the new NewObjectArray helpers.

CI

All completed dotnet-android legs on the previous commit's build were green; the build for the latest commit (b959f4a) was still in progress at review time. Not itself a blocker — but the value/value dictionary test above is worth landing before merge, since that's exactly the instantiation most likely to surface only under NativeAOT.

Generated by Android PR Reviewer for #12030 · 358.6 AIC · ⌖ 28.2 AIC · ⊞ 6.8K
_{Comment /review to run again}

[simonrozsival]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[github-actions]

🤖 Code Review — ⚠️ Minor suggestions

Reviewed the AOT-safe array/collection factory refactor. Overall this is a well-constructed, carefully-reasoned change — the suppression justifications are precise, the value-type-vs-__Canon rooting model is explained clearly, and the new factories cleanly consolidate construction policy that was previously scattered across JNIEnv, JavaConvert, and TrimmableTypeMapTypeManager. Nullable-primitive array/collection marshaling has good round-trip test coverage (GetArray_NullableInt32, FromJniHandle_IListNullableInt32, FromJniHandle_IDictionaryInt32Int64).

Findings

• ⚠️ Performance (JavaConvert.cs:79) — collection type is resolved by TryGetCollectionType for a boolean gate, then re-resolved inside TryCreateFromJniHandle, causing MakeGenericType() to run twice per uncached FromJniHandle call for reference generic collections. Inline comment posted.
• 💡 Formatting (JNIEnv.cs:1106) — use nameof (elementType) instead of the string literal. Inline comment posted.

CI

The GitHub combined status shows pending (Azure DevOps dnceng-public validation not yet complete / no required checks reported). Neither finding is merge-blocking, but confirm the pipeline goes green before merge. The primitive/nullable rooting story is only exercised on the reflection-based runtimes by the added tests; NativeAOT rooting for exact value/value instantiations (JavaDictionary<int,long>) relies on generic-virtual rooting and isn't directly asserted under PublishAot in CI — worth keeping in mind for the validation follow-up.

Nice work on the documentation of the AOT reasoning.

Generated by Android PR Reviewer for #12030 · 144.5 AIC · ⌖ 18.8 AIC · ⊞ 6.8K
_{Comment /review to run again}

[github-actions]

🤖 ⚠️ Performance — The resolved collection type from TryGetCollectionType is discarded here (out _) and used only as a boolean gate. TryCreateFromJniHandle then calls TryGetCollectionType again internally, so for reference-type generic collections (e.g. IList<string>, IDictionary<string,string>) MakeGenericType() runs twice per marshal — and GetJniHandleConverter isn't cached, so this repeats on every FromJniHandle call. Consider having the converter reuse the already-resolved Type (capture it in the closure) instead of recomputing it, to avoid the redundant reflection/allocation on a hot marshaling path.

For value/value shapes this probe is also slightly at odds with the AOT-safety design: TryGetCollectionType→GetClosedCollectionType calls MakeGenericType(JavaDictionary<,>, [int,long]) as an availability check even though creation goes through the rooted ValueTypeFactory path — the probe still relies on the exact instantiation being rooted.

(Rule: Avoid redundant reflection on hot paths — repo-conventions Performance)

[github-actions]

🤖 💡 Formatting — Use nameof (elementType) instead of the string literal "elementType" so the argument name stays correct under future renames.

throw new ArgumentNullException (nameof (elementType));

(Rule: Prefer nameof for argument names — repo-conventions)