[foreign-memaccess] RFR 8224843: Refine ByteBuffer interop support

[Maurizio Cimadamore]

I'll check - thanks

Maurizio

On 28/05/2019 11:30, Jorn Vernee wrote:
> Hi,
>
> The patch doesn't compile for me:
>
> === Output from failing command(s) repeated here ===
> * For target jdk_modules_java.base__the.java.base_batch:
> h:\cygwin64\home\Jorn\cygwin-projects-new\memaccess\src\java.base\share\classes\java\nio\ScopedByteBuffer.java:41: 
> error: no suitable constructor found for ByteBuffer(no arguments)
>         super();
>         ^
>     constructor ByteBuffer.ByteBuffer(int,int,int,int,byte[],int) is 
> not applicable
>       (actual and formal argument lists differ in length)
>     constructor ByteBuffer.ByteBuffer(int,int,int,int) is not applicable
>       (actual and formal argument lists differ in length)
> h:\cygwin64\home\Jorn\cygwin-projects-new\memaccess\src\java.base\share\classes\java\nio\ScopedByteBuffer.java:477: 
> error: put(byte[]) in ScopedByteBuffer cannot override put(byte[]) in 
> ByteBuffer
>     public ByteBuffer put(byte[] src) {
>                       ^
>   overridden method is final
> h:\cygwin64\home\Jorn\cygwin-projects-new\memaccess\src\java.base\share\classes\java\nio\ScopedByteBuffer.java:484: 
> error: hasArray() in ScopedByteBuffer cannot override hasArray() in 
> ByteBuffer
>    ... (rest of output omitted)
>
> * All command lines available in 
> /cygdrive/h/cygwin64/home/Jorn/cygwin-projects-new/memaccess/build/windows-x86_64-server-release/make-support/failure-logs.
> === End of repeated output ===
>
> I see that in the patch you made the necessary changes to 
> java.nio.Buffer, but shouldn't these changes be made to the template 
> file for ByteBuffer as well? Since ScopedByteBuffer extends ByteBuffer?
>
> Jorn
>
> Maurizio Cimadamore schreef op 2019-05-27 19:45:
>> Hi,
>> as you know, the foreign memory access API supports bidirectional
>> interop between MemoryAddress and ByteBuffer:
>>
>> 1) MemorySegment::ofByteBuffer(ByteBuffer)
>> 2) MemoryAddress:;asDirectByteBuffer(int bytes)
>>
>> That is (1) can be used to create a memory segment out of an existing
>> byte buffer, whereas (2) can be used to do the opposite, that is, to
>> convert a memory address into a byte buffer.
>>
>> While (1) works pretty reliably (but I've added some tests for it),
>> the implementation for (2) leaves to be desired:
>>
>> * The resulting byte buffer is unaware of the fact that the backing
>> memory is attached to a scope that can be closed
>> * There's no way to create a buffer if the address encapsulates some
>> heap-based memory address
>>
>> This patch solves both issues - and also adds a supported way for
>> creating a memory segment out of a heap-allocated byte array
>> (MemorySegment.ofArray(byte[])) which I think is useful.
>>
>> To solve the scope awareness issue I put together (after discussing
>> extensively with Alan, CC'ed) a delegating wrapper of ByteBuffer,
>> which delegates to the underlying buffer implementation after doing a
>> liveness check on the owning scope. That means that operations such as
>> ByteBuffer::getInt will only succeed if the scope the view is
>> associated with is still alive. Note that we need the wrapping only if
>> the associated scope is _not_ pinned - in fact, if the scope is
>> pinned, then it can't be closed, so there's no need to check for
>> liveliness.
>>
>> To do this I had to remove the 'final' modifier from some instance
>> methods in ByteBuffer and Buffer - the theory here is that these
>> 'final' have been added in early days to help the VM out, but are not
>> necessary now (also note that is not possible for a client to create a
>> custom byte buffer implementation, as all constructors are
>> package-private).
>>
>> Finally, the wrapped byte buffer implementation does not support
>> typeful views such as 'asCharBuffer' - this is tricky to support as
>> CharBuffer is not a subtype of ByteBuffer so, to go down that path
>> we'd need to wrap also CharBuffer and friends, which is doable, but
>> we're leaning towards YAGNI for now.
>>
>> Finally I've added a test which writes into memory (using var handles)
>> then reads it back using a segment-backed byte buffer (there are tests
>> for both heap and off-heap variants of the buffer). There's also a
>> test which checks interop between MappedByteBuffer and the foreign API
>> (which will likely be relevant in the context of JEP 352 [1]), and,
>> finally, a test which makes sure that all instance method in the
>> scope-wrapped buffer throws ISE after the scope has been closed.
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8224843/
>>
>> Comments welcome!
>>
>> Cheers
>> Maurizio
>>
>> [1] - https://openjdk.java.net/jeps/352