On Tue, 22 Sep 2020 03:12:01 GMT, David Holmes <dhol...@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
