[foreign-memaccess] RFR 8227394: Add MemorySegment::asByteBuffer convenience method

[Jorn Vernee]

> What do you think?

I have felt for a while that asByteBuffer morally belongs on 
MemorySegment, since a MemoryAddress models a cursor, so 'treating a 
cursor as a span of memory', as the name asByteBuffer implies feels odd, 
and indeed we still have to supply the size separately.

Like you say, by adding a MemorySegment::asByteBuffer we introduce a 
problem because some segments are too large, but this seems pretty 
similar to being able to pass a size to MemoryAddress::asByteBuffer that 
is too large, which is still possible even if it's an `int`. At the same 
time we have one less problem because we can no longer use a 
MemoryAddress that is out of bounds of the segment.

It feels like a pretty much even trade.

> I'm less convinced on the
> usefulness of turning an entire memory segment into a bytebuffer: if
> you really wanted to view the entire memory segment as a BB, maybe you
> just wanted to create a (direct) BB in the first place?

By that same line of thought, do we need MemoryAddress::asByteBuffer? 
This is meant as an interop thing, where you just have a MemorySegment 
that is given to you, and would like to turn it into a ByteBuffer.

I agree with the concern of the API explosion. I was also looking at 
removing the asByteBuffer method from MemoryAddress, and replacing it 
with a more general 'sliceAt' method, that slices the underlying 
MemorySegment at the offset denoted by the MemoryAddress itself (instead 
of having to pass that manually as the offset to MemorySegment::slice). 
I could imagine cases where a user has a MemoryAddress and wants to do 
an offset and then slice, e.g. I have a pointer to some struct, returned 
by some third party allocator API, and want to give someone else a 
pointer to access just a single field, I can try and call 
`address.segment().slice(...)`, but the problem would be that I don't 
know what to pass as offset (after all, the MemoryAddress I have might 
not be the base address of the segment).

With a `sliceAt` method, the current use of asByteBuffer(int) could be 
replaced with `address.sliceAt(newSize).asByteBuffer()`.

Prototype: 
http://cr.openjdk.java.net/~jvernee/panama/webrevs/8227394/webrev.01/

What do you think? Of course, if you'd rather hold off on fiddling with 
the API too much at this time that's fine with me :) (I'm guessing we're 
trying to target JDK 14 as an incubating feature?)

Jorn

On 2019-07-11 14:00, Maurizio Cimadamore wrote:
> Hi Jorn,
> I gave some thought about this in the past, and my conclusion was
> mixed - and I filed this in the "you ain't gonna need it" bucket
> (YAGNI).
> 
> The motivations are that (i) it is really simple to emulate the API
> with the existing methods (as the implementation of the new method
> shows), and that (ii) the new API is, by necessity, a partial API -
> not all segments can be mapped onto a byte buffer - some are too
> large; while the existing API on memory address is total (thanks to
> the explicit int parameter).
> 
> In other words, the existing API on MemoryAddress supports what I
> believe to be the most important interop idiom: slicing a big memory
> segment into multiple < 32bits buffer views. I'm less convinced on the
> usefulness of turning an entire memory segment into a bytebuffer: if
> you really wanted to view the entire memory segment as a BB, maybe you
> just wanted to create a (direct) BB in the first place?
> 
> The current model is kind of easy to explain - the moves we allow are:
> 
> * ByteBuffer -> MemorySegment
> 
> * MemoryAddress + int -> ByteBuffer
> 
> This feels minimal. If we add another dimension, as per your patch:
> 
> * MemorySegment -> ByteBuffer
> 
> Then the next question is - should we allow also:
> 
> * ByteBuffer -> MemoryAddress? (e.g. create a big segment from the
> byte buffer and then have a memory address point at the BB offset)
> 
> Which feels a bit like a slippery slope. I'd be more confident in
> adding a facility like this after some real world validation.
> 
> What do you think?
> 
> Maurizio
> 
> On 11/07/2019 12:28, Jorn Vernee wrote:
>> Hi,
>> 
>> Please review this small patch that adds an asByteBuffer convenience 
>> method to MemorySegment that returns a buffer the size of the segment.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8227394
>> Webrev: 
>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8227394/webrev.00/
>> 
>> (note that the javadoc is largely taken from 
>> MemoryAddress::asByteBuffer).
>> 
>> My reason for adding this is that I wanted to reach for this method a 
>> few times when playing with the API, but it didn't exist. So, I 
>> thought it would be useful to have.
>> 
>> (Also, I originally wanted to upload this to GitHub [1] to test out 
>> the Skara [2] tools, but apparently that's not operational yet :) )
>> 
>> Thanks,
>> Jorn
>> 
>> [1] : https://github.com/openjdk/panama/pull/1
>> [2] : https://github.com/openjdk/skara#openjdk-project-skara