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

[Maurizio Cimadamore]

Thanks for the comments.

Btw, I realized that my patch doesn't handle very well the creation of 
buffers from on-heap addresses. That's because the 'offset' of the 
ByteBuffer::wrap factory is really an array offset, not an address 
offset - so some conversion is needed there, e.g. (address - BASE) / SCALE.

Also I need to double-check on what happens when you resize regions that 
come from arrays or buffers - some of these operation throw, but I think 
we should be able to support a MemorySegment::resize in all cases.

And, I also need to check what happens when we do a round trip: 
byte-buffer -> segment -> base address -> byte-buffer. (maybe with some 
MemoryRegion::resize thrown in there).

As for MemorySegment::ofArray I'm thinking of relaxing it to accept all 
primitive arrays - since that could be useful if a user needs to do a 
bulk copy from Java heap to an off heap array using MemoryAddress::copy.


Some comments inline:



On 27/05/2019 20:12, John Rose wrote:
> I'm glad to see this.  The code looks good.  I've been
> hoping we'd see this day!  I agree with your choices
> about ofArray(byte[]), "final", and CharBuffer.
>
> The new scope-aware type of BB will, I hope, be useful
> for people who want the capabilities of Panama, including
> better pointer safety, but prefer to work with BB APIs
> for slinging the bits.
>
> FTR, here are a couple of future moves that this one
> unlocks:
>
> * Timely deallocation.  If the _buffer field of the new BB
> class is a DBB then a deallocate operation can be defined
> which releases the underlying storage immediately, instead
> of via a Cleaner.  This is safe for the same reason scope
> deallocation is safe.  There's a catch:  The _buffer needs
> to be unaliased to any external "wild" BB or separately
> managed Panama memory block.  This typically means
> (a) the new BB constructor allocates foreign storage directly
> rather than having it handed to the constructor as an
> argument and (b) the new BB class has a boolean which
> encodes the unaliased state.  In effect, this means that
> the underlying block is handled under a discipline of
> "linear ownership", like a linear type in some formalisms.
> Which leads to the next point:

Yes, we could provide an AutoCloseable-like variant of a ByteBuffer 
which internally used a Scope to allocate memory.

Then yes, you need to keep track of aliasing to make sure it's safe to 
clean the scope.

>
> * Handoff operations on BBs.  If/when we do protocols
> for pushing linear ownership of scopes across threads,
> the BBs come along for the ride.  This will widen the
> circumstances where scoped BBs will be useful, by
> removing the constraint the all work happens in one
> thread.

Actually, we no longer have the constraint. With the new confinement 
model I've pushed last week (after discussion - see [1]), only scope 
operation such as fork/allocate/close/merge are confined to the owner 
strand.

But _all_ threads are free to read/write memory. The assumption is that 
the VH is rich enough to allow for all sort of synchronization flavor to 
be built on top.

If you truly want a scope which allocates memory that can be 
read/written by a single strand, you can do so via the CONFINED scope 
charateristics - e.g. Scope.globalScope().fork(CONFINED). I think this 
will remove a lot of pressure for some of the things you describe. Yes, 
in principle there could still be a need of changing ownership for doing 
scope operation such as allocate - but I'd like first to validate how 
common a use case that would be.


Maurizio

>
> More FTR:
>
> The elements of cross-thread handoff are simple,
> though tricky in implementation details.  A memory
> block B can (in principle) be re-scoped from an owning
> T1 to a new owner T2 if T1 synchronously finishes all
> operations on B (release fence) and then locks itself
> out from B.  The transfer to T2 is finished by reversing
> this process (acquire fence).  The midpoint of the process
> is, all by itself, a useful state, where B is locked out of
> both T1 and T2 (and all other threads); this is a neutral
> state from which any thread T2 can pick it up.  This
> neutral state is neither readable nor writable, but is
> perfect for placing B on a queue, to be picked up by
> a future thread whose identity is not yet known.  So
> hand-off factors into T1 putting B down, into neutral
> state, and T2 picking it up some time later.  A mutex
> of some sort is needed to prevent T2 and T3 both
> picking up a neutral B due to races.
>
> This is future stuff which I think will eventually happen
> but TBH might not.  The present integration between
> BB's and Panama is really neat and doesn't need the
> extra future stuff… at present.
>
> Thanks Maurizio!  This breathes new life into BBs, IMO.
>
> — John
>
>> On May 27, 2019, at 10:45 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>>
>> 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.