RFR(m): 8221734: Deoptimize with handshakes

[Robbin Ehn]

Hi Dan,

> src/hotspot/share/runtime/biasedLocking.cpp
>      nit - Please update copyright year for this file.
> 

Updated in 8220724.

>      Nice refactoring into more readable chunks! I'm assuming that
>      Patricio is also reviewing these changes...

Great, good!

> src/hotspot/share/runtime/deoptimization.cpp
>      L778:  bool _in_handshake;
>          nit - needs one more space of indent.

Fixed.

> 
>      Nice refactoring while adding in the handshake support.

Great!

> 
> src/hotspot/share/runtime/deoptimization.hpp
>      L147:  public:
>      L148:
>      L149:   // Deoptimizes a frame lazily. nmethod gets patched deopt happens 
> on return to the frame
>      L163:   static void fix_monitors(JavaThread* thread, frame fr, RegisterMap* 
> map)
>          Style nit: I would put the blank line on L148 above L147.

Fixed.

> 
>      L164:     { inflate_monitors(thread, fr, map); }
>          Style nit: Should be:
> 
>              static void fix_monitors(JavaThread* thread, frame fr, RegisterMap* 
> map) {
>                inflate_monitors(thread, fr, map);
>              }

Fixed.

> src/hotspot/share/runtime/mutexLocker.cpp
>      No comments. (So OsrList_lock is now 'special-1' instead of 'leaf'.
>      I presume the Compiler team is okay with that...

Since need we hold CodeCache_lock while iterating nmethods, all locks that might 
be taken needed to be pushed down under CodeCache_lock.
So I hope they are okay with that.

> Thumbs up!  I don't need to see a webrev if you fix the nits...

Thanks Dan! Fixed!

I did t6-7 over the weekend, no issues found.

/Robbin

> 
> Dan
> 
> 
>>
>> # Note
>> http://cr.openjdk.java.net/~rehn/8221734/v2/inc/webrev/src/hotspot/share/runtime/biasedLocking.cpp.sdiff.html 
>> line 630
>> This is revert to the original, I accidental had left in a temporary test 
>> change, as you can see here in full diff:
>> http://cr.openjdk.java.net/~rehn/8221734/v2/webrev/src/hotspot/share/runtime/biasedLocking.cpp.sdiff.html 
>>
>>
>> I think I manage to address all review comments.
>>
>> Dean can you please cast an extra eye on:
>> http://cr.openjdk.java.net/~rehn/8221734/v2/inc/webrev/src/hotspot/share/oops/method.cpp.sdiff.html 
>>
>> This OR should be correct.
>>
>> Dan please do the same on the biased locking changes.
>>
>> I left out the merge with MutexLocker changes, since it was not interesting.
>> There were some conflicts with JVMCI changes, so incremental contains some 
>> parts of that merge.
>>
>> Passes t1-5 and local testing.
>> I'll continue with some additional testing.
>>
>> Thanks, Robbin
>>
>> On 4/25/19 2:05 PM, Robbin Ehn wrote:
>>> Hi all, please review.
>>>
>>> Let's deopt with handshakes.
>>> Removed VM op Deoptimize, instead we handshake.
>>> Locks needs to be inflate since we are not in a safepoint.
>>>
>>> Goes on top of:
>>> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-April/033491.html 
>>>
>>>
>>> Code:
>>> http://cr.openjdk.java.net/~rehn/8221734/v1/webrev/index.html
>>> Issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8221734
>>>
>>> Passes t1-7 and multiple t1-5 runs.
>>>
>>> A few startup benchmark see a small speedup.
>>>
>>> Thanks, Robbin
>