RFR(m): 8221734: Deoptimize with handshakes
Daniel D. Daugherty
daniel.daugherty at oracle.com
Fri May 3 21:13:14 UTC 2019
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
More information about the hotspot-runtime-dev
mailing list