RFR (S) 8230003: Make Monitor inherit from Mutex

[Patricio Chilano]

Hi Coleen,

Change looks good to me! Other minor nits that might require 
s/monitor/mutex:

src/hotspot/share/runtime/mutex.cpp
      assert(_owner != thread, "deadlock: blocking on monitor owned by 
current thread");

src/hotspot/share/runtime/interfaceSupport.inline.hpp
      // receives an extra argument compared to ThreadBlockInVM, the 
address of a pointer to the monitor we

src/hotspot/share/runtime/interfaceSupport.inline.hpp
      //   the monitor that is only partially acquired.

src/hotspot/share/runtime/interfaceSupport.inline.hpp
     release_monitor()


Thanks,
Patricio
On 8/22/19 9:44 AM, coleen.phillimore at oracle.com wrote:
>
>
> On 8/22/19 2:45 AM, David Holmes wrote:
>> Hi Coleen,
>>
>> On 22/08/2019 12:52 pm, coleen.phillimore at oracle.com wrote:
>>> Summary: Reverse inheritance that makes more sense.
>>>
>>> See bug for more description.  Tested with hs-tier1-3 with 
>>> linux-x64-debug and hs-tier1 on Oracle platforms.
>>>
>>> open webrev at 
>>> http://cr.openjdk.java.net/~coleenp/2019/8230003.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8230003
>>
>> src/hotspot/share/runtime/interfaceSupport.inline.hpp
>>
>> !   Mutex** _in_flight_monitor_adr;
>>
>> It looks a little odd to still have "monitor" in the local variable 
>> and thread field name. That's another 18 changes to make. Separate 
>> cleanup okay.
>>
>
> I fixed it, it was a easy additional change to look at.
>> ---
>>
>> src/hotspot/share/runtime/mutex.hpp
>>
>> // The default length of monitor name was originally chosen to be 64 
>> to avoid
>> // false sharing. Now, PaddedMonitor is available for this purpose.
>> // TODO: Check if _name[MONITOR_NAME_LEN] should better get replaced 
>> by const char*.
>> static const int MONITOR_NAME_LEN = 64;
>>
>> s/monitor/mutex throughout?
> I fixed this too.
>
> We should have an RFE for this.
>
>>
>> ---
>>
>> src/hotspot/share/runtime/mutex.cpp
>>
>>       log_trace(vmmonitor)("JavaThread " INTPTR_FORMAT " on %d 
>> attempt trying to acquire vmmonitor %s", p2i(self), retry_cnt, _name);
>>
>> There's only a single logging line in all the mutex/monitor code. Is 
>> it worth changing vmmonitor to vmmutex ?
>
> I did this too.
>
> And removed the comment that Robbin pointed out.
>
> Rebuilt, will run tier1 sanity.
>
> incremental: 
> http://cr.openjdk.java.net/~coleenp/2019/8230003.02.incr/webrev
> full: http://cr.openjdk.java.net/~coleenp/2019/8230003.02/webrev
>
> Thanks,
> Coleen
>>
>> Thanks,
>> David
>> -----
>>
>>> Thanks,
>>> Coleen
>>>
>>>
>>> *
>>> *
>