[foreign-preview] RFR: 8282061: Improve support for deterministic closure of shared scopes [v8]

[David Holmes]

On Mon, 21 Feb 2022 18:23:35 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> This patch improves the logic for closing shared resource scopes, by using the following algorithm:
>> 
>> 1. move the scope from ALIVE to CLOSED - no new thread can access segments associated with the scope
>> 2. do an initial handshake, to collect all threads that are accessing the scope concurrently
>> 3. if no thread is found, finish
>> 4. if some threads T1, T2 ... Tn are found, keep doing handshakes (only against these threads) then go back to (3). 
>> 
>> Note that the logic no longer require *three* states for shared segment, and also this logic always succeeds - that is the close operation can never fail because of a spurious access found during an handshake.
>> 
>> Note that the logic converges quickly, because handshaked threads are deoptimized - meaning that they will have to re-load the liveness state of the resource they are accessing (at which point they will just throw an exception).
>> 
>> Implementation-wise, when looking over the code with @fisk , we realized that it is possible for multiple threads to run the handhshake closure concurrently. To collect all the problematic thread, we used a lock free stack (which was already implemented in the hotspot code). Also, to keep problematic threads alive during multiple rounds of handshaking, we use a ThreadHandleList (this is also required to be able to handshake on a specific thread).
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix reference to boolean return type in ScopedMemoryAccess::closeScope

Hi Maurizio,

A few minor nits on the comments but the handshaking protocol looks good.

Thanks,
David

src/hotspot/share/prims/scopedMemoryAccess.cpp line 162:

> 160: 
> 161: /*
> 162:  * This functin performs a thread-local handshake against all threads running at the time

s/functin/function/

src/hotspot/share/prims/scopedMemoryAccess.cpp line 164:

> 162:  * This functin performs a thread-local handshake against all threads running at the time
> 163:  * the given scope (deopt) was closed. If the handshake closure finds that a thread has
> 164:  * safepointed inside a scoped method (that is, a method inside the ScopedMemoryAccess class

Do you really mean "safepointed" here, or do you just mean the point where the handshake is processed?

src/hotspot/share/prims/scopedMemoryAccess.cpp line 167:

> 165:  * annotated with the '@Scoped' annotation), whose local variables mention the scope being closed
> 166:  * (deopt), the thread is added to a problematic stack. After the handshake, each thread in
> 167:  * the problematic stack is handshaked again, individually, to check that it has exited

I suggest saying "list" rather  than "stack" as VM developers tend to assume "stack" means thread stack.

src/hotspot/share/prims/scopedMemoryAccess.cpp line 183:

> 181:     return;
> 182:   }
> 183:   // now iterate on all problematic threads, until we converge. Note: from this point on,

s/now/Now/
s/on/over/

src/hotspot/share/prims/scopedMemoryAccess.cpp line 191:

> 189:   while (element != NULL) {
> 190:     JavaThread* thread = element->_thread;
> 191:     // if the thread is not in the list handle, it means that the thread has died,

s/if/If/

-------------

PR: https://git.openjdk.java.net/panama-foreign/pull/643