RFR: 8254693: Add Panama feature to pass heap segments to native code [v2]

[Maurizio Cimadamore]

On Wed, 18 Oct 2023 04:44:26 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Add the ability to pass heap segments to native code. This requires using `Linker.Option.critical(true)` as a linker option. It has the same limitations as normal critical calls, namely: upcalls into Java are not allowed, and the native function should return relatively quickly. Heap segments are exposed to native code through temporary native addresses that are valid for the duration of the native call.
>> 
>> The motivation for this is supporting existing Java array-based APIs that might have to pass multi-megabyte size arrays to native code, and are current relying on Get-/ReleasePrimitiveArrayCritical from JNI. Where making a copy of the array would be overly prohibitive.
>> 
>> Components of this patch:
>> 
>> - New binding operator `SegmentBase`, which gets the base object of a `MemorySegment`.
>> - Rename `UnboxAddress` to `SegmentOffset`. Add flag to specify whether processing heap segments should be allowed.
>> - `CallArranger` impls use new binding operators when `Linker.Option.critical(/* allowHeap= */ true)` is specified.
>> - `NativeMethodHandle`/`NativeEntryPoint` allow `Object` in their signatures.
>> - The object/oop + offset is exposed as temporary address to native code.
>> - Since we stay in the `_thread_in_Java` state, we can safely expose the oops passed to the downcall stub to native code, without needing GCLocker. These oops are valid until we poll for safepoint, which we never do (invoking pure native code).
>> - Only x64 and AArch64 for now.
>> - I've refactored `ArgumentShuffle` in the C++ code to no longer rely on callbacks to get the set of source and destination registers (using `CallingConventionClosure`), but instead just rely on 2 equal size arrays with source and destination registers. This allows filtering the input java registers before passing them to `ArgumentShuffle`, which is required to filter out registers holding segment offsets. Replacing placeholder registers is also done as a separate pre-processing step now. See changes in: https://github.com/openjdk/jdk/pull/16201/commits/d2b40f1117d63cc6d74e377bf88cdcf6d15ff866
>> - I've factored out `DowncallStubGenerator` in the x64 and AArch64 code to use a common `DowncallLinker::StubGenerator`.
>> - Fallback linker is also supported using JNI's `GetPrimitiveArrayCritical`/`ReleasePrimitiveArrayCritical`
>> 
>> Aside: fixed existing issue with `DowncallLinker` not properly acquiring segments in interpreted mode.
>> 
>> Numbers for the included benchmark on my machine are:
>> 
>> 
>> Benchmar...
>
> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
> 
>   drop unused in_reg_spiller

src/java.base/share/classes/java/lang/foreign/Linker.java line 792:

> 790:          * such as loss of performance, or JVM crashes.
> 791:          * <p>
> 792:          * Critical functions can optionally allow access to the Java heap. This allows a client to pass heap

Suggestion:

         * Critical functions can optionally allow access to the Java heap. This allows clients to pass heap

src/java.base/share/classes/java/lang/foreign/Linker.java line 795:

> 793:          * memory segments as addresses, where normally only off-heap memory segments would be allowed. The memory region
> 794:          * inside the Java heap is exposed through a temporary native address that is valid for the duration of the
> 795:          * function call. As such, these temporary addresses, or any addresses derived from them, should not be used

The API does not expose temporary addresses - so it is not 100% clear when reading what this para refers to. I suppose you mean a native function that "captures" an object's address and then returns it, so that the client wraps it in a zero-length memory segment? I can't decide on top of my head if this is too cornery or not, even for this javadoc.

src/java.base/share/classes/java/lang/foreign/Linker.java line 800:

> 798:          * prohibitive in terms of performance.
> 799:          *
> 800:          * @implNote As a consequence of allowing heap access, the JVM will either lock the garbage collector, or pin the

This is a very low-level comment. Which is fine per se. But it doesn't say what does it mean for the developer using this functionality. I think you want to say that GC is impacted in some way or form. If so, please say that.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16201#discussion_r1363538621
PR Review Comment: https://git.openjdk.org/jdk/pull/16201#discussion_r1363541304
PR Review Comment: https://git.openjdk.org/jdk/pull/16201#discussion_r1363544042