[Java.Interop] Use native UTF-8 JNI remaps - #12795
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved remapping wiring, fast-path, diagnostic, and test-coverage issues remain.
Get a fresh assessment by requesting another Copilot review.
Review tier: Lite
Findings: 3
Open (4)
What changed in this PR
This pull request adds native UTF-8 pointer support for Java.Interop JNI remapping to reduce managed string conversions.
Changes:
- Adds pointer-backed remapping metadata and lookup overloads.
- Updates instance/static method and field remapping.
- Updates the unshipped API baseline.
| File | Description |
|---|---|
external/Java.Interop/src/Java.Interop/PublicAPI.Unshipped.txt |
Records new UTF-8 pointer APIs. |
external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs |
Adds native pointer lookup overloads. |
external/Java.Interop/src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs |
Adds pointer-backed replacement metadata. |
external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs |
Updates static method remapping. |
external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticFields.cs |
Updates static field remapping. |
external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs |
Updates instance method and constructor remapping. |
external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceFields.cs |
Updates instance field remapping. |
external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.cs |
Adds shared remapping helper dispatch. |
|
/review |
|
✅ 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.
⚠️ Needs Changes
Findings: 0 errors · 2 warnings · 0 suggestions
The mixed UTF-8/string fallback handling added in the follow-up commit resolves the earlier all-or-nothing fast-path issue, and the JNI exception cleanup paths remain consistent with the existing implementation. Before merging, the new public raw-pointer contract should explicitly define its lifetime/ownership requirements, and the pointer-backed and mixed lookup branches need direct regression coverage.
CI is still in progress with no failures reported so far; Java.Interop tests have passed on macOS and Windows.
Generated by Android PR Reviewer for #12795 · copilot · gpt56 · 132.3 AIC · ⌖ 11.4 AIC · ⊞ 21K
Comment /review to run again
0e742f9 to
2857f0c
Compare
simonrozsival
added
the
ready-to-review
|
/review |
|
✅ Android PR Reviewer completed successfully! Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "azcliprod.blob.core.windows.net"See Network Configuration for more information.
|
There was a problem hiding this comment.
❌ Reject
Findings: 1 error · 1 warning · 0 suggestions
The pointer/span JNI lookup paths preserve the intended allocation-free remapping behavior, including mixed native/fallback operands, and the Java.Interop macOS and Windows CI lanes pass. However, the new lazy DEBUG metadata retains unmanaged pointers beyond the lifetime currently promised by the public API, which can produce a use-after-free in diagnostics. The failure diagnostic also reports the string representation even when JNI lookup used the independent UTF-8 representation.
CI is not green: Azure build 1599102 failed in MAUI Tests MAUI Integration; all other reported lanes passed. The AZDO failure details were unavailable because this environment has no Azure DevOps login, so I could not determine whether that gate is related to this PR.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
azcliprod.blob.core.windows.net
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "azcliprod.blob.core.windows.net"See Network Configuration for more information.
Generated by Android PR Reviewer for #12795 · copilot · gpt56 · 147.2 AIC · ⌖ 11.4 AIC · ⊞ 21K
Comment /review to run again
simonrozsival
removed
the
ready-to-review
|
/review |
simonrozsival
added
the
ready-to-review
|
✅ Android PR Reviewer completed successfully!
|
There was a problem hiding this comment.
⚠️ Needs Changes
Findings: 0 errors · 1 warning · 0 suggestions
The UTF-8 pointer lifetime contract is now explicit, JNI lookups preserve pointer-backed and mixed operands correctly, and all 44 CI checks are green. The remaining gap is regression coverage: converting the shared fixture entirely to pointer-backed outputs removes coverage of the retained string compatibility path and leaves the managed-name/native-signature branch unexercised.
Generated by Android PR Reviewer for #12795 · copilot · gpt56 · 105.2 AIC · ⌖ 11.2 AIC · ⊞ 21K
Comment /review to run again
d0d5773 to
31c38f7
Compare
|
/review |
|
✅ Android PR Reviewer completed successfully! Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "azcliprod.blob.core.windows.net"See Network Configuration for more information.
|
There was a problem hiding this comment.
⚠️ Needs Changes
Findings: 0 errors · 0 warnings · 1 suggestion
The UTF-8 pointer paths preserve the managed-string fallback, handle mixed pointer/span operands consistently, and now carry an explicit runtime-scoped lifetime contract. The remaining suggestion is to document pointer-over-string precedence for custom JniTypeManager implementations.
CI is still in progress: 19 of 44 checks have passed, 24 are running, and 1 is queued; no failures are currently reported. The Java.Interop macOS and Windows test lanes have passed.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
azcliprod.blob.core.windows.net
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "azcliprod.blob.core.windows.net"See Network Configuration for more information.
Generated by Android PR Reviewer for #12795 · copilot · gpt56 · 148.6 AIC · ⌖ 8.4 AIC · ⊞ 18.1K
Comment /review to run again
Carry stable UTF-8 replacement type and method-name pointers through remapping results and use them directly for JNI lookup while preserving string fallbacks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Document the unmanaged memory ownership and lifetime contract for remapping pointers, and exercise pointer-backed instance/static method lookups with signature fallback in the JVM tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Carry replacement type, method name, and signature pointers through JniPeerMembers and JNI lookup without eagerly decoding native memory. Preserve string-based compatibility paths and defer decoding to explicit diagnostics and Debug metadata access. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Require replacement pointers to remain stable for the associated runtime because cached JNI metadata may retain them, and use UTF-8-first precedence when formatting failed lookup diagnostics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The UTF-8-signature compatibility case relies on the test fixture's custom JniTypeManager, which Android does not support. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep the mixed representation coverage in the host type manager while registering the equivalent generated remap for Android test runs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
b3d3e61 to
f12babb
Compare
Return string and UTF-8 replacement representations from one virtual lookup so pointer-aware managers can report handled misses without triggering a duplicate string-table search. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Route string replacement queries through the combined lookup and make non-zero UTF-8 pointers authoritative when an override supplies both representations. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
jonathanpeppers
left a comment
jonathanpeppers
left a comment
There was a problem hiding this comment.
I just had the one question, otherwise looks good. 👍
| #if DEBUG | ||
| string? name, signature; | ||
| IntPtr nameUtf8, signatureUtf8; | ||
| #endif // !DEBUG |
There was a problem hiding this comment.
Is it worth having the DEBUG values here at all? Or are they useful during development?
There was a problem hiding this comment.
yeah, only useful during development
Motivation
The existing Intune/MAM JNI method-remapping path stores target type and method names as stable NUL-terminated UTF-8 strings in generated native data. The old path materialized those names as managed UTF-16 strings and then encoded them back to UTF-8 for
FindClassandGetMethodID.That round trip is unnecessary and becomes more important as remapping is reused by larger consumers such as R8.
Approach
ReplacementMethodInfo.JniTypelookup paths for pointer/pointer and mixed pointer/span member names and signatures.JniPeerMembersretain a replacement type pointer and use it directly forFindClassand later member-remapping lookups.JniMethodInfoDebug metadata and decode them only ifName,Signature, orToString()is explicitly requested.JniTypeManagerimplementations.The successful generated-remapping path therefore passes the pregenerated UTF-8 type, method name, and optional signature directly to JNI without copying them or converting them to a managed string.
This PR is method-only and does not add R8, field remapping, reverse-type mapping, inherited-member fallback, or NativeAOT remapping support.
Relationship to other PRs
mainand provides the Java.Interop representation and JNI lookup primitives.Validation
JniPeerMembersTests: 12 passed, 1 skipped.