RFR: 8241234: Unify monitor enter/exit runtime entries

[Daniel D. Daugherty]

On 3/30/20 10:15 AM, Yudi Zheng wrote:
> Hi Daniel,
>
> Thanks for the review! I have uploaded a new version with your comments addressed:
> http://cr.openjdk.java.net/~yzheng/8241234/webrev.04/
>
>> src/hotspot/share/runtime/sharedRuntime.hpp
>>      Please don't forget to update the copyright year before you push.
> Fixed.
>
>> src/hotspot/share/runtime/sharedRuntime.cpp
>>      L2104:   ObjectSynchronizer::exit(obj, lock, THREAD);
>>          The use of 'THREAD' here and 'TRAPS' in the function itself
>>          standout more now, but that's something for me to cleanup.
> Also, I noticed that C2 was using CHECK
>>     ObjectSynchronizer::enter(h_obj, lock, CHECK);
> While C1 and JVMCI were using THREAD:
>>     ObjectSynchronizer::enter(h_obj, lock->lock(), THREAD);
> I have no idea when to use what, and hope unifying to the C2 entries would help.
> Let me know if there is something I should address in this patch. Otherwise, I would
> rather leave it to the expert, i.e., you ;)

Yes, please leave it for me to clean up.


>> src/hotspot/share/c1/c1_Runtime1.cpp
>>      old L718:   assert(thread == JavaThread::current(), "threads must correspond");
>>          Removed in favor of the assert in SharedRuntime::monitor_enter_helper().
>>          Okay that makes sense.
>>
>>      old L721:   EXCEPTION_MARK;
>>          Removed in favor of the same in SharedRuntime::monitor_enter_helper().
>>          Okay that makes sense.
>>
>> src/hotspot/share/jvmci/jvmciRuntime.cpp
>>      old L403:   assert(thread == JavaThread::current(), "threads must correspond");
>>      old L406:   EXCEPTION_MARK;
>>          Same as for c1_Runtime1.cpp
> I assume I don’t need to do anything regarding the comments above.

Correct. Just observations on the old code.


>>      L390:     TRACE_jvmci_3("%s: entered locking slow case with obj="...
>>      L394:   TRACE_jvmci_3("%s: exiting locking slow with obj="
>>      L417:     TRACE_jvmci_3("%s: exited locking slow case with obj="
>>          But this is no longer the "slow" case so I'm a bit confused.
>>
>>          Update: I see there's a comment about the tracing being removed.
>>          I have no opinion on that since it is JVM/CI code, but the word
>>          "slow" needs to be adjusted if you keep it.
> I removed all the tracing code.

Thanks for cleaning that up.

Dan

>
> Many thanks,
> Yudi