RFR: 8244340: Handshake processing thread lacks yielding

[Patricio Chilano]

On 5/6/20 5:53 AM, Robbin Ehn wrote:
> 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/
Thanks Robbin, looks good!


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