Repository navigation
Document trimmable JNI interoperability - #12892
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Resolve the outstanding documentation accuracy and terminology nits before approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
This documentation-only PR adds an internal guide for trimmable TypeMap JNI interoperability across CoreCLR and NativeAOT.
Changes:
- Adds detailed generation, registration, peer-lifetime, trimming, and troubleshooting guidance.
- Links the guide from the documentation index.
| File | Summary |
|---|---|
Documentation/README.md |
Adds the interoperability guide to the documentation index. |
Documentation/guides/internals/JavaJNI_Interop.md |
Documents the trimmable TypeMap JNI pipeline; outstanding nits address callback naming, generator output passes, virtual dispatch, deferred registration, and managed RegisterNatives terminology. |
72ca1d3 to
fe79b6a
Compare
9b5e966 to
b476dd2
Compare
b476dd2 to
036fc05
Compare
66cc744 to
63de9b7
Compare
63de9b7 to
cfcb10c
Compare
cfcb10c to
2358193
Compare
2358193 to
e2059d7
Compare
e2059d7 to
798dce7
Compare
798dce7 to
c682bf2
Compare
c682bf2 to
bb8c3b0
Compare
bb8c3b0 to
4851d3c
Compare
4851d3c to
79e0555
Compare
Describe generated Java wrappers, managed TypeMaps, JNI registration, peer lifetime, and CoreCLR/NativeAOT interop. Link the guide from the documentation index. Co-authored-by: Copilot App <[email protected]>
Explain the former registration concepts using current TypeMap and JCW examples, managed UTF-8 JNI registration, ABI marshalling, startup, peer lifecycle, and trimming. Co-authored-by: Copilot App <[email protected]>
Keep Markdown paragraphs on one source line while preserving list, table, heading, and fenced code structure. Co-authored-by: Copilot App <[email protected]>
Clarify the trimmable TypeMap behavior after the parent runtime review fix. Co-authored-by: Copilot App <[email protected]>
Clarify that trimmable app builds generate platform wrappers rather than using a prebuilt reference-pack jar, alongside the selected runtime support jar. Co-authored-by: Copilot App <[email protected]>
Preserve the post-test-removal link corrections when replaying the guide on main. Co-authored-by: Copilot App <[email protected]>
5de0317 to
6d470c9
Compare
Explain that virtual Java dispatch may round-trip into a generated JCW callback and point JNI troubleshooting at managed IAndroidCallableWrapper registrations. Co-authored-by: Copilot App <[email protected]>
|
@dalexsoto review |
dalexsoto
left a comment
dalexsoto
left a comment
There was a problem hiding this comment.
The two earlier virtual-dispatch and managed-registration troubleshooting concerns are fixed. Two independent current-path assertions still need correction in Documentation/guides/internals/JavaJNI_Interop.md:
-
Removed API described as compatible (line 127).
Runtime.registerdoes not remain a compatibility stub at this revision:src/java-runtime/java/mono/android/Runtime.javahas onlyregisterNatives(Class), and the oldJava_mono_android_Runtime_registerentrypoint is absent from bothsrc/native/clr/host/host-jni.ccandsrc/native/clr/libnet-android.map.txt. An old caller cannot resolve the documented stub. Remove that compatibility promise or explicitly state that the old API was removed; restoring it is not requested. -
Callback-frame ownership misdescribed (line 184).
TypeMapAssemblyEmitter.EmitUcoForwarderBody(lines 1200-1228) emits a bridge wait and a catch that callsAndroidEnvironmentInternal.UnhandledException; it does not emitBeginMarshalMethod,OnUserUnhandledException, or anEndMarshalMethodfinally region. Direct exports and constructor entrypoints do own that frame, and modern binding callbacks can establish it insideJniMarshal.SafeInvoke. Describe these protection routes separately so JNI-transition and exception debugging follows the actual layer. This is a documentation correction, not a claim that modern callbacks are unprotected.
Both changed files, the full replacement guide, and its seven material integration paths were inspected at the pinned revision; no additional high-confidence blocker remains in this static review.
Clarify that Runtime.register was removed and distinguish normal callback forwarders from direct export and constructor transition frames. Co-authored-by: Copilot App <[email protected]>
|
Addressed both points from @dalexsoto’s review in 9893d8d:
The guide’s source links and Markdown structure have been checked. |
dalexsoto
left a comment
dalexsoto
left a comment
There was a problem hiding this comment.
Both findings from review 5412125255 are fixed. The guide now correctly describes removal of the old Runtime.register API and separates registered-binding forwarders, modern callback-owned JNI transitions, and direct export/constructor-owned frames, including both newly linked helpers. The earlier virtual round-trip and generated-managed registration troubleshooting corrections remain intact.
The complete current guide/index and all seven material static integration paths leave no remaining high-confidence blocker within the accepted current-only scope. No local build, test or device success is claimed.
- Remove unsupported typemap fixtures from Android builds and retire JVM-only Java.Interop tests, Java fixtures, and obsolete remapping rules. Keep Android-compatible cases and shared marshaler coverage. - Drop the desktop-only Java.Interop stage from public, internal, and official pipelines and its unused template; retain Android device-test lanes. - Preserve reflection-discovered Android coverage with target-time JcwGen and Mono.Android/Java.Interop test roots under the now-default trimmable type map. Select the Android-safe GetThis Java source and test jar in the referenced Android test library even when its property has not yet been set at evaluation time. Stack: follows #12892 on `simonrozsival-document-trimmable-jni-interop` (native stack #12891). The obsolete `TestInstrumentation` category/name exclusions were removed in the parent branch.
Pull Request
title and
description
should follow the
commit-messages.mdworkflow documentation, and in particular should include:Summary
Replace the outdated JNI interoperability guide with a source-backed guide to the trimmable TypeMap used by CoreCLR and NativeAOT. The old text describes removed MonoVM/LLVM typemaps and native marshal registration; this guide instead explains generated JCWs and managed TypeMap assemblies, the Java-to-managed and managed-to-Java call paths (including possible virtual-dispatch round trips), JNI
RegisterNatives, UTF-8 registration data, peer lifetime, trimming, and troubleshooting. Link it from the documentation index.Validation
git diff --check: passed.The PR now targets
mainafter #12890 merged. No stack metadata was changed.