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

[Jorn Vernee]

On Wed, 18 Oct 2023 09:20:10 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> 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 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.

I'm not sure what to write to make this clearer. The address that is exposed to the native target function is indeed a temporary address that is constructed from the oop and offset. It is temporary because after the native call, the GC might move the object around, which invalidates the address.

This part is meant to document that native code should not be holding on to the address until after the call completes. This also includes returning the address back to Java. The address would be invalid the moment the function returns.

Would it help if this said: `The memory region inside the Java heap is exposed to the native target function through a temporary native address`?

> 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.

This comment can be removed, as we no longer lock the GC. The same restrictions apply as a 'regular' critical call that does not allow passing heap segments. The call should complete quickly.

> src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 281:
> 
>> 279:     }
>> 280: 
>> 281:     static SegmentOffset segmentOffsetAllowHeap() {
> 
> I suppose `heapSegmentOffset` is not good because this binding applies to both native and heap segments, right?

Yes. Setting `allowHeap` to true, just allows heap segments to be used, by switching the object + offset addressing pairs. Native segments are also handled by addressing pair. So this binding applies to both.

FWIW, we don't allow heap segments for critical calls by default, since it complicates the calling convention and the work the downcall stub does, which would be detrimental to performance in a case that only needs support off-heap segments. (and when we're talking about critical calls, every nanosecond counts)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16201#discussion_r1363551493
PR Review Comment: https://git.openjdk.org/jdk/pull/16201#discussion_r1363553663
PR Review Comment: https://git.openjdk.org/jdk/pull/16201#discussion_r1363554288