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

Thu Jul 11 14:31:53 UTC 2019

On 11/07/2019 13:57, Jorn Vernee wrote:
>> 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.

This is a more convincing argument - e.g. moving the method from one 
place to another is something we can consider - the fact that the 
conversion method lives on MA is, in a way, a result of past design 
choices where MA was also modelling a segment. Now we have both MA and 
MS and it's a  fair question to ask where does this belong.

The advantages of having the conversion on MemorySegment are, IMHO, 
symmetry with creation (MemorySegment.ofByteBuffer) and, as you mention, 
a cleaner abstraction mapping: a segment is a range, a byte buffer is 
also a range. On the other hand, having the conversion on MemoryAddress 
caters better to those use cases where you have a big memory segment 
(size greater than 32 bits) and then create sliced buffer views of it. 
To support something like this with MemorySegment::asByteBuffer you 
would need to create many segment slices upfront, and then create the 
buffer views from there. But this is also probably a wash:

e.g.

segment.slice(i, 100).asByteBuffer();

vs.

segment.baseAddress().offset(i).asByteBuffer(100);

With respect to loss of totality, as you point out, converting an MA has 
the issue that the provided size might exceed that of the segment; 
converting a MS has the problem that the MS might be too big. So again, 
a wash.

So, on second thought, maybe just moving the method from MA to MS is all 
we need? (in the name of a better abstraction mapping)

As for sliceAt - I'm not sure. You can always get to the base address of 
a segment of any given memory address. To solve the cases you are 
thinking of, you can go two ways: add sliceAt - which allows you to 
remain in the MA world - or add a MemoryAddress::offset() method which 
returns a long (this should be a safe operation, as the offset is 
relative to the segment anyway), and then feed that back into 
MemorySegment::slice

Of the two, the latter feels simpler?

Maurizio

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