RFR: 8244340: Handshake processing thread lacks yielding

[Patricio Chilano]

Hi Robbin,

Thanks for fixing this!

I tested the patch in a Windows Server 2012 virtual machine (not 
VirtualBox), with two virtual cpus assigned. Without the patch, running 
the simple HelloWorld program takes about 2.5seconds most of the times 
when setting “/affinity 1” and around 300ms when setting “affinity /FF”. 
With the patch I get the same timing for both cases, about 300ms. It 
would be great if Miklos can also check it against his environment to 
see if his issue got fixed.

Only small comments in handshake.cpp:
- In VM_HandshakeOneThread, last log_handshake_info() should be (pr == 
HandshakeState::_success) instead of by_vm_thread. Maybe just define 
"HandshakeState::ProcessResult pr" instead of by_vm_thread. Same with 
Handshake::execute_direct() to get rid of by_handshaker.
- In Handshake::execute_direct() inside the "if" statement we could just 
break after verifying _success.
- I would remove _spin_time_ns and just keep _max_spin_time_ns, since 
both are only set at construction only.
- I think a better name for _last_wait_ns should be last_spin_start_ns 
because it is updated also when we see the state changed.

Thanks Robbin!

Patricio
On 5/5/20 10:08 AM, Robbin Ehn wrote:
> Hi all, please review.
>
> As the issue describes there are problems with this infinitely spinning.
> To avoid any performance regression we do not yield while we do or see
> progress. But when we enter a period of no progression we 'quickly' give
> up.
>
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8244340
>
> Code:
> http://cr.openjdk.java.net/~rehn/8244340/v1/webrev/
>
> In the pathologically case we can easily see 10x improvement starting 
> the VM.
> I have found no performance regressions or performance gains in our
> standard benchmark setups.
> But I see much better average completion times for handshakes.
> For example specJVM2008 serial with ZGC shows 4x faster time to
> completion for handshakes in my setup, but little or no effect on score
> (ops/m).
>
> Passes t1-3.
>
> Thanks, Robbin