[Java.Interop] Map nullable byte jagged arrays to java.lang.Byte by simonrozsival · Pull Request #12113 · dotnet/android

simonrozsival · 2026-07-15T08:45:38Z

Summary

treat byte? as a managed-to-JNI signature alias of sbyte? for java/lang/Byte in the reflection type manager built-ins
preserve Java-to-managed canonicalization of java/lang/Byte as sbyte?
cover rank-2 and rank-3 nullable byte jagged arrays in legacy and trimmable type-manager/device tests

Experiment evidence

Before this change, legacy reverse signature lookup had no built-in entry for System.Byte?. Looking up byte?[][] or byte?[][][] therefore failed, and JNIEnv.NewArray() fell back to java.lang.Object[][] / java.lang.Object[][][]. The element values still round-tripped. The trimmable manager already returned the intended [[Ljava/lang/Byte; and [[[Ljava/lang/Byte; signatures.

Behavior change

Legacy byte? reverse lookup now shares the existing nullable signed-byte JNI signature, producing exact java.lang.Byte array classes at ranks 2 and 3. Primitive byte/sbyte behavior, Java-to-managed sbyte? canonicalization, and collection roots are unchanged.

Tests

dotnet test tests/Java.Interop-Tests/Java.Interop-Tests.csproj --filter FullyQualifiedName~JniTypeManagerTests (6 passed; supplied installed JDK paths)
added device coverage for exact rank-2/rank-3 JNI class names and managed value round-trips; device tests were not run locally

Centralize safe runtime construction of arrays and generic Java collection wrappers so JavaConvert and JNIEnv no longer carry the trimmable-specific construction policy inline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Fix nullable annotations on factory Try methods and keep mixed dictionary creation from bypassing the explicit value-type support map. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Expand suppression justifications and comments with the source-grounded NativeAOT behavior for array and generic type construction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Consolidate primitive and nullable value-type rooting for safe arrays and Java collection wrappers into ValueTypeFactory<T>. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Update the suppression text to say first-rank value vectors bypass CreateInstanceFromArrayType and are allocated through ValueTypeFactory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…elines The new nullable/collection marshaling paths made JavaConvert.GetJniHandleConverter AOT-reachable, exposing the legacy TryMakeGenericCollectionTypeFactory helper whose MakeGenericType() calls only suppressed IL2055 (trimming) and not IL3050 (AOT), leaking 6 NativeAOT warnings. Add the matching IL3050 suppression; that branch only runs when !RuntimeFeature.TrimmableTypeMap (Mono/CoreCLR), never under NativeAOT. Update BuildReleaseArm64 SimpleDotNet CoreCLR/NativeAOT apkdesc baselines to accept the expected size increase from the rooted array/collection typemap entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…d API - Remove the unused SafeJavaCollectionFactory.TryCreateInstance(Type, object?, out IJavaObject?) speculative overload (no call sites). - Convert the three new factory files to file-scoped namespaces per repo conventions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Check the value-type factory locals directly instead of via intermediate bools so the compiler tracks the non-null flow, clearing CS8602/CS8604 in the dictionary path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

… API, avoid per-element closure - Add FromJniHandle_IDictionaryInt32Int64 to exercise the value/value dictionary branch (JavaDictionary<int,long> via the generic-virtual CreateDictionaryWithKey<TKey>), confirming that value-over-value instantiation is rooted under the trimmable/NativeAOT config. - Remove the unused throwing SafeArrayFactory.GetArrayType(Type,int) overload (only TryGetArrayType/CreateInstance are called). - Inline JavaConvert.WithLocalJniHandle in CopyManagedObjectArray to avoid allocating a capturing closure per array element. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ctions Resolve conflicts from main's IsDynamicCodeSupported [FeatureGuard] refactor and its generated-proxy collection factory, which landed in the same regions this branch rewrites to runtime Safe*Factory construction: - JNIEnv.ArrayCreateInstance / TrimmableTypeMapTypeManager.GetArrayTypes: keep main's IsDynamicCodeSupported dynamic-code fast path; route the AOT path through SafeArrayFactory (generated proxy first, then runtime construction). - JavaConvert.GetJniHandleConverter: replace main's TryGetFactoryBasedConverter (generated-proxy path, removed on this branch) with SafeJavaCollectionFactory; keep main's IsDynamicCodeSupported/ManagedTypeMap else-chain and [RequiresDynamicCode] on TryMakeGenericCollectionTypeFactory. - BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc: kept the branch baseline; MUST be regenerated from a validation build (sizes shift after the merge). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fddff664-4661-4c06-8922-316731aa2b8d

…roid-trimmable-revisit-arrays-generic-collections

Addresses the two remaining PR review comments: - Use `nameof (elementType)` instead of the string literal in `CreateManagedArrayFromObjectArray`. - Gate the trimmable generic-collection converter on a new `SafeJavaCollectionFactory.IsSupportedCollectionType ()` check that does not resolve (or root) the closed collection type. Previously the gate called `TryGetCollectionType ()` (running `MakeGenericType ()`) purely as a boolean probe, and `TryCreateFromJniHandle ()` then resolved the type again, so `MakeGenericType ()` ran twice per uncached `FromJniHandle ()` marshal for reference generic collections. The type is now resolved at most once (reference collections), or not at all (value collections handled by the rooted `ValueTypeFactory` path). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 155743e0-3f7e-489b-b351-80da73edc7d0

The NativeAOT Java-to-managed array reverse lookup (TrimmableTypeMapTypeManager.GetArrayTypes) fell back to returning only the `T[]` vector when no generated array proxy was present. That drops the JavaObjectArray<T>/JavaArray<T> (reference) and JavaArray<T>/JavaPrimitiveArray<T>/ concrete-primitive (e.g. JavaSByteArray) wrapper types that both the generated proxy and the CoreCLR path return, so marshaling a Java array to those wrapper types would fail to resolve once array proxies are no longer pre-generated. Replace the fallback with BuildRuntimeArrayTypes, which reproduces the generated JavaArrayProxy.GetArrayTypes() contract (see the generator's ModelBuilder.GetArrayTypeReferences) at runtime and AOT-safely: - Primitive leaves use PrimitiveArrayInfo's closed `typeof` tokens. - Reference leaves build [JavaObjectArray<T>, JavaArray<T>, T[]] via canonical (__Canon) MakeGenericType/MakeArrayType, the same rooting the trimmable typemap already relies on for reference generics. - Jagged (rank > 1) mirrors ExpandRankOneTypes. Behavioral note: non-primitive value-type leaves (e.g. Nullable<int>) return only the exact rooted vector (int?[]); JavaObjectArray<valueType> would require an unrooted value-type generic instantiation and the generator never emitted one. Why no test caught this: the existing TryGetArrayProxy_* tests only assert the *proxy* path (gated on generated proxies), and the marshaling round-trip tests only need `T[]`, so the no-proxy runtime fallback was never asserted. BuildRuntimeArrayTypes is a pure function, so the added TrimmableTypeMapTypeManagerTests exercise it directly on every config. Also inline JavaConvert.WithLocalJniHandle in JNIEnv.SetObjectArrayElementFromManagedValue to avoid allocating a capturing closure per element on the SetArrayItem path (matching CopyManagedObjectArray). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 155743e0-3f7e-489b-b351-80da73edc7d0

Address review: the removed ScalarContainerFactories supported byte-element collections (IList<byte>, IDictionary<byte,…>), but ValueTypeFactory only mapped sbyte, so IsSupportedCollectionType returned false for byte and the trimmable GetJniHandleConverter fell through to a silent null — a regression versus the reflection paths (which still handle byte via dynamic MakeGenericType). Add byte and byte? to ValueTypeFactory.PrimitiveTypeFactories. byte marshals to java.lang.Byte bitwise, identical to sbyte, and JavaConvert already has the byte scalar converters, so JavaList<byte>/JavaDictionary<byte,…> round-trip correctly (including values > 127). byte[] arrays are unaffected: they are handled by JNIEnv's dedicated primitive-array path (new byte[len]), which never reaches SafeArrayFactory. Add a FromJniHandle_IListByte round-trip test (with an unsigned-range value). Also document, in ValueTypeFactory.CreateDictionaryWithKey, why value/value dictionaries need no dedicated rooting token: every ValueTypeFactory<X> is statically constructed, CreateDictionary is virtually reachable for all of them (instantiating the GVM CreateDictionaryWithKey<X> for every key X), so NativeAOT's generic-virtual-method dependency analysis roots the full JavaDictionary<X,Y> cross-product over the fixed primitive/nullable set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 155743e0-3f7e-489b-b351-80da73edc7d0

Add complete nullable byte array marshaling, parse generic collection shapes once per converter, and add focused NativeAOT trimmable coverage for value/value dictionary rooting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 98b572c2-766d-43a8-8f1f-3c39df991541

Assert complete runtime array type contracts and exercise collection conversion through JniValueManager without reflection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 98b572c2-766d-43a8-8f1f-3c39df991541

Resolve APK size references using the lower values. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bec709dd-9aac-4675-91e0-b263414a86c3

Fix nullable byte jagged-array JNI signatures, cover the path with runtime tests, and run the full eligible NativeAOT trimmable suite. Stop consuming generated array proxies and the legacy container-factory accessor while retaining generator compatibility for follow-up cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bec709dd-9aac-4675-91e0-b263414a86c3

Use the measurements produced by Azure DevOps build 1508177 after the runtime factory changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bec709dd-9aac-4675-91e0-b263414a86c3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0cae6439-30ce-4c1e-b8f6-b64f3d77d4ad

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0e043be9-9070-43bc-aea4-a48438690bdf

simonrozsival · 2026-08-19T12:16:37Z

@copilot resolve the merge conflicts in this pull request

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot · 2026-08-19T12:40:22Z

@copilot resolve the merge conflicts in this pull request

Resolved in merge commit a1c4381.

simonrozsival · 2026-08-19T14:44:39Z

@copilot resolve the merge conflicts in this pull request

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot · 2026-08-19T15:14:09Z

@copilot resolve the merge conflicts in this pull request

Resolved in merge commit dab9835.

Copilot

Pull request overview

Adds nullable byte JNI mapping to java.lang.Byte, preserving sbyte? canonicalization and expanding jagged-array coverage.

Changes:

Adds legacy nullable-byte signature support.
Adds rank-2/rank-3 type-manager and device tests.
Adds a NativeAOT trimmable test lane.

Show a summary per file

File	Summary
`tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/TrimmableTypeMapTypeManagerTests.cs`	Adds jagged-array signature coverage.
`tests/Mono.Android-Tests/Mono.Android-Tests/Android.Runtime/JnienvArrayMarshaling.cs`	Adds nullable-byte array round-trip coverage; legacy-path coverage remains needed.
`external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs`	Tests nullable-byte signatures and canonicalization.
`external/Java.Interop/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs`	Adds nullable-byte mapping; the source template must also be updated.
`build-tools/automation/yaml-templates/stage-package-tests.yaml`	Adds the trimmable test lane; the property name should explicitly select the trimmable typemap.

Review details

Files reviewed: 5/5 changed files
Comments generated: 3
Review effort level: Lite

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsival · 2026-08-21T20:03:23Z

/review

github-actions · 2026-08-21T20:04:09Z

✅ Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12113

github-actions

⚠️ Needs Changes — 1 error, 0 warnings, 0 suggestions.

The nullable-byte signature mapping and rank-two/rank-three device coverage are directionally good, and all 44 reported checks are green. However, the new reflection type mapping is not paired with a byte? value marshaler, so JavaObjectArray<byte?> now allocates java.lang.Byte[] but attempts to populate it with proxy objects, causing an array-store failure. The inline comment describes the required mapping and regression coverage.

Generated by Android PR Reviewer for #12113 · gpt56 · 143.8 AIC · ⌖ 9.13 AIC · ⊞ 25.7K
_{Comment /review to run again}

Keep the reflection type and value mappings consistent for byte? and preserve unsigned byte values through Java's signed byte representation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9d3a4906-44f6-4c84-8a2f-d56df4c399fe

jonathanpeppers · 2026-08-24T14:03:01Z

+        configuration: $(XA.Build.Configuration)
+        testName: Mono.Android.NET_Tests-NativeAOTTrimmable
+        project: tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj
+        extraBuildArgs: -p:TestsFlavor=NativeAOTTrimmable -p:PublishAot=true -p:AndroidTypeMapImplementation=trimmable


Are these settings already the default for NativeAOT? If so, this is a duplicate test run?

simonrozsival and others added 24 commits July 9, 2026 12:53

[TrimmableTypeMap] Address safe collection factory review

db04690

Fix nullable annotations on factory Try methods and keep mixed dictionary creation from bypassing the explicit value-type support map. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

[TrimmableTypeMap] Document NativeAOT factory assumptions

9e0ce62

Expand suppression justifications and comments with the source-grounded NativeAOT behavior for array and generic type construction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

[TrimmableTypeMap] Share primitive value type factories

0c62664

Consolidate primitive and nullable value-type rooting for safe arrays and Java collection wrappers into ValueTypeFactory<T>. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

[TrimmableTypeMap] Clarify array factory AOT justification

53d50fb

Update the suppression text to say first-rank value vectors bypass CreateInstanceFromArrayType and are allocated through ValueTypeFactory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Merge remote-tracking branch 'origin/main' into dev/simonrozsival/and…

f663705

…roid-trimmable-revisit-arrays-generic-collections

Merge origin/main into trimmable type map work

da5f681

Resolve APK size references using the lower values. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bec709dd-9aac-4675-91e0-b263414a86c3

[TrimmableTypeMap] Update CoreCLR APK size baseline

8d6578f

Use the measurements produced by Azure DevOps build 1508177 after the runtime factory changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bec709dd-9aac-4675-91e0-b263414a86c3

[TrimmableTypeMap] Fix byte collection test build

409b05b

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0cae6439-30ce-4c1e-b8f6-b64f3d77d4ad

[tests] Scope trimmable factory coverage

1fc0450

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0e043be9-9070-43bc-aea4-a48438690bdf

[tests] Restore JavaConvert factory coverage

f87ade5

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0e043be9-9070-43bc-aea4-a48438690bdf

[Java.Interop] Map nullable byte to java.lang.Byte

cf8cd39

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0e043be9-9070-43bc-aea4-a48438690bdf

[tests] Cover nullable byte vectors

8fc311d

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0e043be9-9070-43bc-aea4-a48438690bdf

Base automatically changed from dev/simonrozsival/android-trimmable-revisit-arrays-generic-collections to main July 15, 2026 20:11

Copilot started work on behalf of simonrozsival August 19, 2026 12:19 View session

Merge origin/main and resolve conflicts

a1c4381

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot finished work on behalf of simonrozsival August 19, 2026 12:40

Copilot started work on behalf of simonrozsival August 19, 2026 14:45 View session

Merge origin/main and resolve submodule conflict

dab9835

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot finished work on behalf of simonrozsival August 19, 2026 15:14

simonrozsival marked this pull request as ready for review August 21, 2026 10:31

simonrozsival requested a review from jonathanpeppers as a code owner August 21, 2026 10:31

Copilot AI lite review requested due to automatic review settings August 21, 2026 10:31

Copilot started reviewing on behalf of simonrozsival August 21, 2026 10:32 View session

Copilot AI reviewed Aug 21, 2026

View reviewed changes

Comment thread build-tools/automation/yaml-templates/stage-package-tests.yaml Outdated

Comment thread external/Java.Interop/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs Outdated

Comment thread tests/Mono.Android-Tests/Mono.Android-Tests/Android.Runtime/JnienvArrayMarshaling.cs Outdated

Address nullable byte array review feedback

c7ca410

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsival added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Aug 21, 2026

github-actions Bot reviewed Aug 21, 2026

View reviewed changes

Comment thread external/Java.Interop/src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs

jonathanpeppers reviewed Aug 24, 2026

View reviewed changes

Navigation Menu

Uh oh!

[Java.Interop] Map nullable byte jagged arrays to java.lang.Byte - #12113

[Java.Interop] Map nullable byte jagged arrays to java.lang.Byte#12113
simonrozsival wants to merge 28 commits into
mainfrom
dev/simonrozsival/fix-legacy-nullable-byte-arrays

simonrozsival commented Jul 15, 2026

Uh oh!

simonrozsival commented Aug 19, 2026

Uh oh!

Copilot AI commented Aug 19, 2026

Uh oh!

simonrozsival commented Aug 19, 2026

Uh oh!

Copilot AI commented Aug 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonrozsival commented Aug 21, 2026

Uh oh!

github-actions Bot commented Aug 21, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

Uh oh!

jonathanpeppers Aug 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

simonrozsival commented Jul 15, 2026

Summary

Experiment evidence

Behavior change

Tests

Uh oh!

simonrozsival commented Aug 19, 2026

Uh oh!

Copilot AI commented Aug 19, 2026

Uh oh!

simonrozsival commented Aug 19, 2026

Uh oh!

Copilot AI commented Aug 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonrozsival commented Aug 21, 2026

Uh oh!

github-actions Bot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonathanpeppers Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Aug 21, 2026 •

edited

Loading