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

[Maurizio Cimadamore]

On Thu, 9 Nov 2023 15:34:38 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/foreign/Linker.java line 792:
>> 
>>> 790:          * @param allowHeapAccess whether the linked function should allow access to the Java heap.
>>> 791:          */
>>> 792:         static Option critical(boolean allowHeapAccess) {
>> 
>> Speaking of public API, I'm surprised to see critical function property conflated with ability to perform on-heap accesses. These aspects look orthogonal to me. Any particular reason not to represent them as 2 separate `Option`s?
>> 
>> Even though it's straightforward to support on-heap accesses during critical function calls, object pinning would support that for non-critical function calls as well, but proposed API doesn't cover it and new API will be required. What's the plan here?
>
>> Even though it's straightforward to support on-heap accesses during critical function calls, object pinning would support that for non-critical function calls as well, but proposed API doesn't cover it and new API will be required. What's the plan here?
> 
> The issue is that most GCs don't support object pinning. I don't think we want an API that only works for some GCs. But, if we do, there's a better API that we can have for just pinning support, which is a `MemorySegment::pin(Arena)` method that returns a MemorySegment wrapping the pinned array. That would allow doing multiple native calls with just a single pin operation, and also allows embedding pointers to pinned segments in other data structures.
> 
> For the current approach where we make the array accessible for the duration of the native call: without pinning support, other GCs would have to use GCLocker. That means that the native call also has to be relatively short-lived, at which point I figured we might as well drop the thread state transition, since that has the same requirement. I.e. we detect that the call is short-lived, and do the optimization ourselves without the hint from the user (`critical`). This coincidentally also greatly simplifies the implementation. In a prior iteration I did have a separate `allowHeap` `Option` that implied `critical`. But it was suggested to just merge the two together in that case.

I stand by the current design: a GCLocker-based mechanism (as the current implementation is) needs to have similar restrictions both on-heap access and also removal of state transitions. It's true that a more general notion of pinning is possible, which doesn't necessarily require special support from the linker (because we can turn an heap segment into a native segment by pinning it and _then_ pass that to the linker). But at this point in this support for region-based pinning is not mature enough to justify such an API (and, if we'll ever get to that point, that would not invalidate the critical linker options).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16201#discussion_r1388411265