RFR: 8244340: Handshake processing thread lacks yielding

[Robbin Ehn]

Hi Patricio,

Here is v2 inc:
http://cr.openjdk.java.net/~rehn/8244340/v2/inc/
Full:
http://cr.openjdk.java.net/~rehn/8244340/v2/full/

On 2020-05-05 21:44, Patricio Chilano wrote:
> Hi Robbin,
> 
> Thanks for fixing this!

Thanks!

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

Great. They don't seem to be building the JDK them self.

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

Fixed.

> - In Handshake::execute_direct() inside the "if" statement we could just 
> break after verifying _success.

The if statement is removed.

> - I would remove _spin_time_ns and just keep _max_spin_time_ns, since 
> both are only set at construction only.

Fixed.

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

Fixed.

Thanks, Robbin

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