RFR: 8238761: Asynchronous handshakes [v4]

[Robbin Ehn]

On Mon, 21 Sep 2020 21:18:20 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Robbin Ehn has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev
>> excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since
>> the last revision:
>>  - Update after Dan and David
>>  - Merge branch 'master' into 8238761-asynchrounous-handshakes
>>  - Removed double check, fix comment, removed not needed function, updated logs
>>  - Fixed double checks
>>    Added NSV
>>    ProcessResult to enum
>>    Fixed logging
>>    Moved _active_handshaker to private
>>  - Rebase version 1.0
>
> src/hotspot/share/runtime/handshake.hpp line 55:
> 
>> 53: };
>> 54:
>> 55: class AsynchHandshakeClosure : public HandshakeClosure {
> 
> Can you make this minor change?  Asynch to english speakers looks like a-cinch and if left as Async is a-sink. Can you
> remove the 'h's ?  I see David above left off the extra h, which is what one expects this to be named.

Fixed

> src/hotspot/share/runtime/handshake.cpp line 394:
> 
>> 392:   {
>> 393:     NoSafepointVerifier nsv;
>> 394:     process_self_inner();
> 
> Can you remove process_self_inner and just inline it here since this is it's only caller and both are short functions?
> If you don't want to, that's fine.  I found myself searching for any other callers of this, that's all.

We might remove ThreadInVMForHandshake and NoSafepointVerifier in the future.
ThreadInVMForHandshake is needed while we have the _suspend_flag and we have two different request to safepoint inside
handshake due java heap allocation inside handshakes. Having them outside like now it's clearer if we do such changes,
so I'll keep it this way.

> test/hotspot/jtreg/runtime/handshake/HandshakeDirectTest.java line 42:
> 
>> 40: public class HandshakeDirectTest  implements Runnable {
>> 41:     static final int WORKING_THREADS = 32;
>> 42:     static final int DIRECT_HANDSHAKES_MARK = 500000;
> 
> Could this timeout?

I have seen no signs of it and have been through several mach5 runs with no issues.

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

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