RFR: 8238761: Asynchronous handshakes [v4]

[Robbin Ehn]

On Tue, 22 Sep 2020 03:12:01 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Looks mostly good to me!
>
> Hi Robbin,
> 
> I've gone back to refresh myself on the previous discussions and (internal) design walk-throughs to get a better sense
> of these changes. Really the "asynchronous handshake" aspect is only a small part of this. The fundamental change here
> is that handshakes are now maintained via a per-thread queue, and those handshake operations can, in the general case,
> be executed by any of the target thread, the requestor (active_handshaker) thread or the VMThread. Hence the removal of
> the various "JavaThread::current()" assumptions. Unless constrained otherwise, any handshake operation may be executed
> by the VMThread so we have to take extra care to ensure the code is written to allow this. I'm a little concerned that
> our detour into direct-handshakes actually lulled us into a false sense of security knowing that an operation would
> always execute in a JavaThread, and we have now reverted that and allowed the VMThread back in. I understand why, but
> the change in direction here caught me by surprise (as I had forgotten the bigger picture). It may not always be
> obvious that the transitive closure of the code from an operation can be safely executed by a non-JavaThread.  Then on
> top of this generalized queuing mechanism there is a filter which allows some control over which thread may perform a
> given operation - at the moment the only filter isolates "async" operations which only the target thread can execute.
> In addition another nuance is that when processing a given thread's handshake operation queue, different threads have
> different criteria for when to stop processing the queue:
> - the target thread will drain the queue completely
> - the VMThread will drain the queue of all "non-async" operations**
> - the initiator for a "direct" operation will drain the queue up to, and including, the synchronous operation they
>   submitted
> - the initiator for an "async" operation will not process any operation themselves and will simply add to the queue and
>   then continue on their way (hence the "asynchronous")
> 
> ** I do have some concerns about latency impact on the VMThread if it is used to execute operations that didn't need to be
>    executed by the VMThread!
> 
> I remain concerned about the terminology conflation that happens around "async handshakes". There are two aspects that
> need to be separated:
> - the behaviour of the thread initiating the handshake operation; and
> - which thread can execute the handshake operation
> 
> When a thread initiates a handshake operation and waits until that operation is complete (regardless of which thread
> performed it, or whether the initiator processed any other operations) that is a synchronous handshake operation. When
> a thread initiates a handshake operation and does not wait for the operation to complete (it does the
> target->queue()->add(op); and continues on its way) that is an asynchronous handshake operation.  The question of
> whether the operation must be executed by the target thread is orthogonal to whether the operation was submitted as a
> synchronous or asynchronous operation.  So I have problem when you say that an asynchronous handshake operation is one
> that must be executed by the target thread, as this is not the right characterisation at all. It is okay to constrain
> things such that an async operation is always executed by the target, but that is not what makes it an async operation.
> In the general case there is no reason why an async operation might not be executed by the VMThread, or some other
> JavaThread performing a synchronous operation on the same target.  I will go back through the actual handshake code to
> see if there are specific things I would like to see changed, but that will have to wait until tomorrow.  Thanks, David

Hi David, you are correct and you did a fine job summarizing this, thanks!

> I will go back through the actual handshake code to see if there are specific things I would like to see changed, but
> that will have to wait until tomorrow.

Great thanks!

> 
> Thanks,
> David

#

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

PR: https://git.openjdk.java.net/jdk/pull/151