RFR(m): 8221734: Deoptimize with handshakes

[Daniel D. Daugherty]

On 5/3/19 6:31 AM, Robbin Ehn wrote:
> Hi, please see this update:
>
> Inc:
> http://cr.openjdk.java.net/~rehn/8221734/v2/inc/webrev/index.html
> Full:
> http://cr.openjdk.java.net/~rehn/8221734/v2/webrev/

src/hotspot/share/aot/aotCodeHeap.cpp
     No comments.

src/hotspot/share/aot/aotCompiledMethod.cpp
     No comments.

src/hotspot/share/code/codeCache.cpp
     No comments.

src/hotspot/share/code/nmethod.cpp
     No comments.

src/hotspot/share/code/nmethod.hpp
     No comments.

src/hotspot/share/gc/z/zBarrierSetNMethod.cpp
     No comments.

src/hotspot/share/gc/z/zNMethod.cpp
     No comments.

src/hotspot/share/jvmci/jvmciEnv.cpp
     No comments.

src/hotspot/share/oops/markOop.hpp
     No changes to this file.

src/hotspot/share/oops/method.cpp
     No comments.

src/hotspot/share/oops/method.hpp
     No comments.

src/hotspot/share/prims/jvmtiEventController.cpp
     No comments.

src/hotspot/share/prims/methodHandles.cpp
     No comments.

src/hotspot/share/prims/whitebox.cpp
     No comments.

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

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

src/hotspot/share/runtime/biasedLocking.hpp
     No comments.

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

     Nice refactoring while adding in the handshake support.

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.

     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);
             }

src/hotspot/share/runtime/mutex.hpp
     No comments.

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

src/hotspot/share/runtime/mutexLocker.hpp
     No comments.

src/hotspot/share/runtime/synchronizer.cpp
     No comments.

src/hotspot/share/runtime/thread.cpp
     No comments.

src/hotspot/share/runtime/thread.hpp
     No comments.

src/hotspot/share/runtime/vmOperations.cpp
     No comments.

src/hotspot/share/runtime/vmOperations.hpp
     No comments.

src/hotspot/share/services/dtraceAttacher.cpp
     No comments.

 > Dan please do the same on the biased locking changes.
     I did so and they look fine.

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

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