[foreign] from memory segments to byte buffers

[Maurizio Cimadamore]

On 22/03/2021 13:10, Remi Forax wrote:
> ----- Mail original -----
>> De: "Maurizio Cimadamore" <maurizio.cimadamore at oracle.com>
>> À: "panama-dev at openjdk.java.net'" <panama-dev at openjdk.java.net>
>> Envoyé: Lundi 22 Mars 2021 13:40:18
>> Objet: [foreign] from memory segments to byte buffers
>> Hi,
>> after putting together the PR for the ResourceScope abstraction [1] it
>> occurred to me that, perhaps, support for byte buffer views in the
>> memory segment API is more complex than it needs to be.
>>
>> Currently, when creating a byte buffer out of a memory segment, we need
>> to save the segment scope in the buffer, and apply scope checks whenever
>> the buffer is accessed. Among the things the scope check will enforce,
>> the most important ones are:
>>
>> * confinement check: if the scope associated with the segment from which
>> the buffer has been derived is confined, make sure that the buffer is
>> accessed from same thread
>> * liveness check: if the scope associated with the segment  from which
>> the buffer has been derived is closed, access to the buffer is not allowed
>>
>> Since in the previous API there was no way to disallow a call to
>> MemorySegment::close, the only way we had to enforce correctness on the
>> buffer was to apply same scope checks we applied on memory segments.
>> When combined with the ability of also creating a segment from a byte
>> buffer (the dual case), this makes for some convoluted code, as we have
>> to ensure that doing segment -> buffer -> segment gives you back the
>> same scope you started with.
>>
>> Now we have, possibly, a new weapon: when creating a byte buffer view
>> from a segment, we could acquire the segment. This would mean that the
>> segment would remain non-closeable as long as the buffer view (or any
>> slices derived from it) are reachable.
>>
>> This would solve all issues related with liveness - but there would
>> still be problems when it comes to confinement. Here we have some options:
>>
>> * disallow creating a byte buffer view from a confined segment - after
>> all, the byte buffer API is not confined (note that for segments created
>> using the so called "default scope" this restriction would not apply,
>> since default segments are shared)
>> * still allow creation of BB views from confined segments, but insert
>> confinement checks where needed
>>
>> What do people think about this simplification? How important it is to
>> retain deterministic deallocation in the face of byte buffer views? One
>> clear advantage of dropping that use case is that no further change
>> would be required to the BB API in order e.g. to support async file
>> system operation: since a byte buffer now would keep the originating
>> scope open, there would be no fear of the memory backing the byte buffer
>> disappearing while performing some async IO on it.
>>
>> The drawback is that if anything inside an application requires some
>> conversion from a segment into a BB (e.g. to interop with some legacy
>> library), that conversion has a potential of affecting deterministic
>> deallocation of the segment, from code which might be oblivious to the
>> fact that somewhere some BB view has been created (and maybe the BB has
>> already been operated on, so there's no "bug" in the application, but
>> the GC/Cleaner might be slow at triggering the ReosurceScope release
>> operation).
>>
>> We can of course keep everything as is (as we have done most of the
>> work) I just wanted to make sure we understood the complexity of what
>> the impl does today, and make sure this complexity was necessary.
> You are asking us if we want to get ride of a safe version of Unsafe.closeByteBuffer(), right ?
>
I think you are conflating different aspects - e.g. I would separate 
users of the memory segment API who just create a segment to obtain a 
closeable ByteBuffer and then use ByteBuffer only - e.g. (with current API)

try (MemorySegment segment = ...) {
    ByteBuffer buffer = segment.asByteBuffer();
    //never look at the segment ever again
    ...
}

This case is a degenerate case, which I hope will be addressed at some 
point by adding a ByteBuffer::allocateDirect(..., ResourceScope) 
factory; if you had that you'd just write this:

try (ResourceScope scope = ...) {
    ByteBuffer buffer = ByteBuffer.allocateDirect(100, scope);
    ...
}

Does that change your perception?

> Now that we have got a taste of being able to de-allocate a BB deterministically *AND* at the same time being able to pass it around whatever we want like before, it will be hard to go back to the world you are proposing, the actual semantics is very convenient because it doesn't require to change the existing codes.
I don't think the "it doesn't require changes to the existing code" is 
really true though: if you have a byte buffer backed by a memory segment 
that is closeable, then your code is not sound unless you use the 
ResourceScope.acquire method when you set up the byte buffer.

So my email was more about "should the API do the acquire automatically" ?

As currently written, the API allows for more freedom, and, I believe, 
the complexity we see now, we will see at some point if we really do add 
scoped byte buffer (see above) anyway.

But the current API also allows you to shoot yourself in the foot if you 
create a buffer from a segment (especially in the shared case) and then 
treat the buffer as a "regular buffer" (which it's not).

Maurizio


>
>> Maurizio
> Rémi