RFR: 8241234: Unify monitor enter/exit runtime entries

[coleen.phillimore at oracle.com]

On 3/24/20 1:14 PM, Yudi Zheng wrote:
> Hi Coleen,
>
> Thanks for the review! Sorry I missed your email. And thanks Vladimir 
> for the reminder.
>
>>> I think you should have monitor_enter_helper and maybe 
>>> monitor_exi_helper both take Handle as arguments.  This looks very 
>>> strange.
>
> Shall we change ObjectSynchronizer::exit to accept Handle as argument 
> as well?

I think you should.
>
>>> +void SharedRuntime::monitor_enter_helper(oopDesc* _obj, BasicLock* 
>>> lock, JavaThread* thread) {
>>> Because it creates a Handle anyway.  We try to eagerly pass Handle 
>>> so that we don't have to search through the code to make sure there 
>>> aren't any unhandled oops.
>>> The _obj parameter name doesn't follow coding conventions (I was 
>>> looking for it to be a field in something).
>
> I took the parameter name from the original methods:
> JRT_BLOCK_ENTRY(void, 
> SharedRuntime::complete_monitor_locking_C(oopDesc* _obj, BasicLock* 
> lock, JavaThread* thread))
> JRT_LEAF(void, SharedRuntime::complete_monitor_unlocking_C(oopDesc* 
> _obj, BasicLock* lock, JavaThread * THREAD))
> Will Change that.
>
>>> + oop obj(_obj);
>>> Is there not already a implicit conversion from oopDesc* to oop?
>
> Also took from the original methods. Will get rid of that.

Great. thank you.
Coleen
>
> Thanks,
> Yudi
>
>>> Thanks,
>>> Coleen
>>> On 3/20/20 11:03 AM, Yudi Zheng wrote:
>>>> Hello,
>>>>
>>>> Please review this patch that unifies the monitor enter/exit 
>>>> runtime entries used by C1, C2, and JVMCI.
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8061553 added support for 
>>>> faster "slow" locking paths.
>>>> These paths are not currently used by JVMCI (or C1) leading to 
>>>> slower performance on programs
>>>> that have a lot of short lived locked sections (such as dacapo:h2).
>>>>
>>>> This patch creates helper methods based on C2’s monitor enter/exit 
>>>> entries, and shares with the C1,
>>>> JVMCI runtimes. It will prevent these runtimes from missing future 
>>>> updates.
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8241234
>>>> http://cr.openjdk.java.net/~yzheng/8241234/webrev.00/
>>>>
>>>> Many thanks,
>>>> Yudi
>