RFR: 8238761: Asynchronous handshakes [v4]

[Robbin Ehn]

On Tue, 22 Sep 2020 07:00:06 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

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

@coleenp I think you placed your comment:
"The "driver" concept is odd. Should it really be caller? Like the thread that called VMHandshake?"
On the commit or somewhere else, not a review comment, I can't reply.

Anyhow I can reply to it here:
The answer is caller makes me think of requester, but we don't know if it's the requester executing this.
Therefore I referred to the thread executing handshakes on the target/handshakee as driver.

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

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