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

Thu Jul 11 16:51:06 UTC 2019

> This sounds good as well.
>
> I was thinking about the lump/split discussion around MA and MS, 
> particularly where we draw the lines of responsibility between the 
> two. I think MS should be used to manage a memory resource 
> (allocating, slicing, closing), while MA should simply be used to 
> access the resource (dereferencing, offset). So, after all, I think 
> any method for slicing the segment (i.e. managing the resource) should 
> be in MS as well.
>
> I'm not so sure about the name offset() though, since that seems to 
> clash a bit with ForeignUnsafe::getOffset(), and 
> MemoryAddress::offset(long). Maybe an alternative is offsetOf(), or 
> toSegmentOffset()?

We have other cases in the JDK where the same name is used for getter 
and setter (e.g. ByteBuffer::order). So I'm not overly worried about 
that, and I still prefer that that using get/set.

Another option worth looking into would be, IMHO, to explore 
alternatives for the offset(long) method. In a way it feels the use of 
offset is ambiguous here, it is both a noun (which justifies the getter 
semantics) and a verb (which justifies the setter/wither semantics). For 
instance, one possible renaming for offset(long) is advance/move

address.advance(42).offset();

I think that reads well? (also implies that negative offsets are kind of 
not accepted?)

Maurizio

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