[foreign-memaccess] on shared segments

Thu Oct 10 12:30:13 UTC 2019

Picking this up again...

I think the javadoc changes I've shown are the best we can do given the 
circumstances; we have to specify under which conditions the original 
segment is invalidated.

Going the other path, and always invalidate, or throw an exception if 
there would be no ownership change leads to a more regular API 
documentation. I think there is an argument to be made (and which has 
been made)  that clients might find this too harsh.

But I want to counter that, and hold on to a model where ownership 
changes are something _serious_ which should only be done by the segment 
owner (the client who created the segment in the first place). I don't 
think that, realistically, clients should randomly call 
asShared/asConfined and hope for the best.

Pulling on this string, I think that throwing exceptions for 'bad' 
ownership changes is actually a plus, not a minus, as they will often 
point out at flaws in the ownership state transition (e.g. a client A 
assumed to be the owner of the thread, and transfer ownership to B; but, 
because of a bug, B was already the owner - so I think this is a 
surprising situation that must be flagged somehow, rather than silently 
returning original segment).

Also, along these lines, I also wonder if a client wouldn't be more 
interested in knowing whether the thread they are on has access to the 
segment (regardless of the shared vs. confined distinction).

Something like

isAccessible() -> isShared() || isConfined(Thread.currentThread)

IMHO, something like that is probably much more useful in terms of 
'checking' whether a client has access or not.

So, concluding, my recommendation going forward would be:

* replace fine-grained predicates for isShared/isConfined, with more 
usable variant which checks if "I have access to the segment" (and, 
possibly, does thread T have access to the segment)
* throw exceptions whenever unexpected state "loopy" transition occur 
(shared -> shared, or confined(T) -> confined(T)), on the basis that 
these reveal bug in the ownership transfer logic

Comments?

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