On Tue, 24 Oct 2023 15:09:57 GMT, Jorn Vernee <jver...@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 two additional 
> commits since the last revision:
> 
>  - a -> an
>  - add note to downcallHandle about passing heap segments by-reference

src/hotspot/share/prims/foreignGlobals.cpp line 150:

> 148:     if (signature[i] != T_VOID) {
> 149:       out_regs.push(as_VMStorage(pair.first(), signature[i]));
> 150:     }

Since we only care about registers-sized values. It is safe to ignore upper 
halves of `T_LONG` and `T_DOUBLE` (i.e. `T_VOID`).

src/hotspot/share/prims/nativeEntryPoint.cpp line 55:

> 53:   for (int i = 0, bt_idx = 0; i < pcount; i++) {
> 54:     oop type_oop = java_lang_invoke_MethodType::ptype(type, i);
> 55:     BasicType bt = java_lang_Class::as_BasicType(type_oop);

We can now see `T_OBJECT` here as well, so we just look at the basic type.

src/hotspot/share/prims/nativeEntryPoint.cpp line 60:

> 58:     if (reg_oop != nullptr) {
> 59:       input_regs.push(ForeignGlobals::parse_vmstorage(reg_oop));
> 60:     }

Some registers are `null`, which indicates that the corresponding value passed 
to the downcall stub should not be forwarded to the native call, but is instead 
used directly in the downcall stub (e.g. the offset of an oop).

src/hotspot/share/prims/upcallLinker.cpp line 182:

> 180:   return (jlong) UpcallLinker::make_upcall_stub(
> 181:     mh_j, entry, out_sig_bt, total_out_args, ret_type,
> 182:     abi, conv, needs_return_buffer, checked_cast<int>(ret_buf_size));

Note that we no longer pass the input signature here, which is instead derived 
from the output signature in the implementation of `make_upcall_stub`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16201#discussion_r1371738783
PR Review Comment: https://git.openjdk.org/jdk/pull/16201#discussion_r1371734701
PR Review Comment: https://git.openjdk.org/jdk/pull/16201#discussion_r1371736101
PR Review Comment: https://git.openjdk.org/jdk/pull/16201#discussion_r1371734122

Reply via email to