RFR (L, tedious) 8222811: Consolidate MutexLockerEx and MutexLocker

[Robbin Ehn]

Hi Coleen,

> 
> This could be a further change, ie easy to make.   I would rather not add this 
> to this already large (in terms of source files) change.  I originally thought 
> we should remove MonitorLocker.  At least I don't think it should allow NULL.

Yes, no need to address something like that in this patch.

> 
>>
>> Also if we remove the 3 sometimes lock and only have 'always' and 'never' locks,
>> having the two version as different types would give you compile errors 
>> instead of runtime asserts.
> 
> We need to revise the discussion whether to require it to be explicit vs. 
> implicit though.   But this is good because now we can decide what to do about 
> this one: https://bugs.openjdk.java.net/browse/JDK-8074355

Yes

Thanks, Robbin

> 
> Thanks,
> Coleen
> 
>>
>> Thanks, Robbin
>>
>>>
>>> So I have a follow on patch wrt MonitorLocker.  Actually I've removed it.  
>>> Erik thought having a NULL monitor wait was crazy, even though we have a case 
>>> of that in the GC.   The code is split whether it uses the MonitorLocker 
>>> local variable vs. the actual lock.
>>>
>>> The only code that consistently uses the MonitorLocker local like this is 
>>> ZGC, so I was going to discuss the next change with you first anyway.
>>>
>>> I see your point about not having to say whether you're safepoint checking 
>>> twice though.  People I've been talking to on the project are of two minds.  
>>> One set wants it always explicit, the other implicit.  I personally want 
>>> consistency over all else, which is why I added wait_without_safepoint_check 
>>> to MonitorLocker.
>>>
>>> I could change this with this patch.  It's easy enough to do, especially if 
>>> you see value in MonitorLocker, which was going to be my next discussion with 
>>> you.
>>>
>>> Thanks,
>>> Coleen
>>>
>>>
>>>>
>>>> cheers,
>>>> Per
>>>>
>>>> On 4/23/19 5:06 PM, coleen.phillimore at oracle.com wrote:
>>>>> Summary: Make MutexLocker be MutexLockerEx implementation, remove 
>>>>> MutexLockerEx calls. Also added wait_without_safepoint_check().
>>>>>
>>>>> Also made safepoint_check_flag an enum, renamed MonitorLockerEx to 
>>>>> MonitorLocker, removed MutexUnlockerEx, and fixed formatting where affected 
>>>>> by this change.
>>>>>
>>>>> There's a MutexLocker constructor with a Thread parameter that would 
>>>>> actually be more performant if the thread is passed from the current 
>>>>> context rather than calling Thread::current() in Monitor::lock.  I think 
>>>>> the Thread parameter should be moved to first so that we can have 
>>>>> MutexLocker look like the Handle constructor calls, for example.  I didn't 
>>>>> make this change in this.
>>>>>
>>>>>     MutexLocker ml(THREAD, Monitor_lock).
>>>>>
>>>>> The interesting changes are in:
>>>>>
>>>>> http://cr.openjdk.java.net/~coleenp/2019/8222811.01/webrev/src/hotspot/share/runtime/mutexLocker.hpp.udiff.html 
>>>>>
>>>>> http://cr.openjdk.java.net/~coleenp/2019/8222811.01/webrev/src/hotspot/share/runtime/mutex.hpp.udiff.html 
>>>>>
>>>>> http://cr.openjdk.java.net/~coleenp/2019/8222811.01/webrev/src/hotspot/share/runtime/mutex.cpp.udiff.html 
>>>>>
>>>>>
>>>>> The rest is pattern substitution.
>>>>>
>>>>> Ran mach5 tier1-6 tests.
>>>>>
>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/2019/8222811.01/webrev
>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8222811
>>>>>
>>>>> Thanks!
>>>>> Coleen
>>>>>
>>>>>
>>>
>