[foreign-memaccess] on shared segments

Tue Oct 15 14:58:01 UTC 2019

Hi,

Some comments;

- MemorySegment's class javadoc mentions confined segments, but not 
shared segments. Maybe there should be a short sentence like "Shared 
segments can not be closed explicitly", with a link back to the 
packag-info javadoc as well.
- In MemorySegment::asConfined javadoc, the language "Obtains a confined 
copy of this memory segment" might be interpreted as the underlying 
memory being copied. Maybe you could use; "Transfers the underlying 
memory to a new MemorySegment instance confined to the given Thread"? I 
think "Transfer" is more descriptive of what's happening (I noticed the 
package javadoc also uses "transfer").
- Similarly for asShared.
- MemorySegment::isAccessible -> typo: "or is the segment is a shared 
segment" -> "or if the segment is a shared segment". The sentence right 
after that is also redundant with the following sentence.

Rest looks good,
Jorn

On 14/10/2019 18:12, Maurizio Cimadamore wrote:
> New revision:
>
> http://cr.openjdk.java.net/~mcimadamore/panama/shared-segments_v4
>
> (although all openjdk websites are experiencing some troubles right now)
>
> This changes the API to:
>
> - add an 'isAccessible()' method which checks 
> "isConfined(Thread.currentThread()) || isShared"
> - changes the spec for asShared and asCOnfined, allowing them to throw 
> exception when an invalid state transition takes place
>
> Maurizio
>
> On 27/09/2019 14:48, Maurizio Cimadamore wrote:
>> Uploaded new revision:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/shared-segments_v3/
>>
>> Changes:
>>
>> * Added full fences on asShared, asConfined
>> * Added new predicate for testing confinement
>> * restructured the calls to checkValidState - now basic accessors and 
>> predicates like byteSize, baseAddress, isPinned etc. do NOT check for 
>> confinement
>> * changed the javadoc of asConfined/asShared to reflect the option 
>> (2) - see below
>>
>>
>>  /**
>>      * Obtains a confined copy of this memory segment whose owner 
>> thread is set to the given thread. If the new owner thread
>>      * differs from the current owner thread, as a side-effect, this 
>> segment will be marked as <em>not alive</em>,
>>      * and subsequent operations on this segment will result in 
>> runtime errors.
>>      * @param newOwner the new owner thread.
>>      * @return a confined copy of this segment with given owner thread.
>>      * @throws UnsupportedOperationException if the segment is a 
>> shared segment (see {@link MemorySegment#isShared()}).
>>      */
>>     MemorySegment asConfined(Thread newOwner) throws 
>> UnsupportedOperationException;
>>
>>     /**
>>      * Obtains a shared copy of this memory segment which can be 
>> accessed across multiple threads. If the current segment
>>      * is not already shared (see {@link MemorySegment#isShared()}), 
>> as a side-effect, this segment will be marked as
>>      * <em>not alive</em>, and subsequent operations on this segment 
>> will result in runtime errors.
>>      * The shared copy will also be marked as <em>pinned</em> (see 
>> {@link MemorySegment#isPinned()});
>>      * as such, any attempt to close the returned segment will result 
>> in a runtime error.
>>      * @return a shared copy of this segment.
>>      */
>>     MemorySegment asShared();
>>
>>
>> I think we can't do much better if we go with (2) - that is we _have_ 
>> to say when the invalidating side-effect takes place. I'm still not 
>> 100% that (2) is the way to go; IMHO (2) supports a fiction that 
>> 'asConfined' can be used to assert confinement, rather than to 
>> proactively change it. I think clients should use it sparingly, 
>> especially now that we have full testing capabilities.
>>
>> Maurizio
>>
>> On 26/09/2019 19:10, Maurizio Cimadamore wrote:
>>> Hi,
>>> in a previous document [1] I explored the problem of allowing 
>>> concurrent access to a memory segment in a safe fashion. From that 
>>> exploration, it emerged that there was one type of race that was 
>>> particularly nasty: that is, a race between a thread A attempting to 
>>> close a segment S while a thread B is attempting to access (read or 
>>> write) S.
>>>
>>> The presence of this race makes it really hard to generalize the 
>>> existing memory access API to cases where concurrent/shared access 
>>> is needed. Of course one naive solution would be to synchronize 
>>> every access on the liveness check, but that makes performance 
>>> really poor - which would defeat the point of having such an API in 
>>> the first place.
>>>
>>> Instead, to solve that problem, in the document I posit about a 
>>> solution which uses an explicit acquire/release mechanism - that is 
>>> clients of a shared segment will need to explicitly acquire the 
>>> segment in order to be able to operate on it, and release it when 
>>> done. A shared segment can only be closed when all clients are done 
>>> with the segment - this is what ensures temporal safety. Moreover, 
>>> since each client works on its own 'acquired' copy of the shared 
>>> segment, everything is a constant and the JIT can see through the 
>>> code and optimize it in the same way as it does for confined access. 
>>> That said, we never fully committed to that solution, since the 
>>> resulting API was very complex: for things to work, part of the 
>>> MemorySegment API has to be moved under a new abstraction (in the 
>>> document called MemoryHandle) - more specifically the bits that are 
>>> responsible for creating addresses. While it's possible to devise a 
>>> confined segment that is both a MemorySegment and a MemoryHandle 
>>> (thus giving us back the old API), the general feedback I've 
>>> received is that this solution seems a bit too convoluted.
>>>
>>> When discussing about this problem with Jim, he pointed out a useful 
>>> connection and a possible way out: after all, all these 
>>> acquire/release and reference counting schemes are there to perform 
>>> a job that a JVM knows exactly how to do at speed: determining 
>>> whether an object is still used or not. So, instead of inventing new 
>>> machinery, we could simply piggy back on the mechanisms we already 
>>> have - that is GC and Cleaners.
>>>
>>> The key realization, in the shared case, can be summarized as: 
>>> performance, safety, deterministic deallocation, pick two! Since 
>>> we're not willing to compromise on safety, or on performance, 
>>> letting go of the deterministic de-allocation goal (only for shared 
>>> segments) seems a reasonable conclusion.
>>>
>>> In other words, there are now two kinds of segments: /confined/ 
>>> segment and /shared/ segments. A segment always starts off as 
>>> confined, and has an owning thread. You can update the owning thread 
>>> - effectively nuking the existing segment and obtaining a new 
>>> segment that is confined on a new thread. This allows clients to 
>>> achieve serialized thread-confinement use cases - where multiple 
>>> threads operate on a piece of memory one at a time. Confined 
>>> segments are operated upon as usual: you allocate a segment, you use 
>>> it, you close it (or you use a try with resources to do it all 
>>> automagically).
>>>
>>> If clients want more - e.g. full concurrent access, an API point is 
>>> provided to turn a confined segment into a shared one. Again, what 
>>> happens here is that the existing segment will be nuked, and a new 
>>> shared segment will be created. But, this shared segment _cannot be 
>>> closed_ (e.g. it is pinned, using the existing API terminology). So, 
>>> how are off-heap resources released if we can't close the segment? 
>>> Well, we let the GC take care of it - by registering the segment on 
>>> a Cleaner, and have the cleaner call some cleanup code once the 
>>> segment is no longer referenced (in reality, things are a bit 
>>> different, in the sense that what we really  key on is the _scope_ 
>>> of a segment, which might be shared across multiple views, but the 
>>> essence is the same). In other words, deallocation for shared 
>>> segments works pretty much the same way deallocation of direct 
>>> buffer work.
>>>
>>> With this move, we are able to retain the simplicity of the existing 
>>> API, while also being able to support efficient and safe concurrent 
>>> access.
>>>
>>> A webrev implementing this change is available here:
>>>
>>> http://cr.openjdk.java.net/~mcimadamore/panama/shared-segments_v2/
>>>
>>> Implementation-wise things are, I think, quite straightforward. I 
>>> took sometime to refactor the code, to make the various scope 
>>> subclasses disappear. We now have a single memory segment 
>>> implementation and two scopes: shared and confined. The confined 
>>> scope takes a 'Runnable' cleanup action which is used (i) when 
>>> closing the confined segment or (ii) passed onto the Cleaner by the 
>>> shared scope if the segment is upgraded to 'shared' state. Also, 
>>> since shared segment now can now be picked up by Cleaner when no 
>>> longer referenced, it is crucial that we add in reachability fences 
>>> around Unsafe operations (same way as direct buffer does really). 
>>> This is because sometimes the GC can aggressively collect unused 
>>> objects stored in local variables during method execution. Adding 
>>> these fences doesn't negatively impact performances (in fact, I'm 
>>> told these fences are a no-op in Hotspot).
>>>
>>> I also took some effort to update some of the javadoc which are 
>>> rendered invalid by this change.
>>>
>>> Comments welcome
>>>
>>> Maurizio
>>>
>>> [1] - http://cr.openjdk.java.net/~mcimadamore/panama/confinement.html
>>>