RFR: 8210832: Remove sneaky locking in class Monitor

Per Liden per.liden at oracle.com
Thu Jan 31 11:44:50 UTC 2019


On 01/30/2019 03:49 PM, Patricio Chilano wrote:
[...]
>>>> src/hotspot/os/posix/os_*.[ch]pp
>>>> ---------------------------------
>>>> * I'd suggest that we place the PlatformMonitor class in a separate 
>>>> file (like src/hotspot/os/posix/monitor_posix.cpp), just like we 
>>>> have done with Semaphore (in src/hotspot/os/posix/semaphore_posix.cpp).
>>> I tried to moved them but there is a small issue in that 
>>> PlatformMonitor code needs static methods defined in their current 
>>> os_*.cpp files (methods that parse timing structs). I can declare 
>>> them as public (cannot move them since they are also used by 
>>> PlatformEvent and Parker), but for the Posix version of 
>>> PlatformMonitor I would also need to do that with _condAttr and 
>>> _mutexAttr which are also defined static in that file and are needed 
>>> by PlatformMonitor::PlatformMonitor. So not sure what the right 
>>> approach is here.
>>> In any case shouldn't we aim to have all synchronization-like classes 
>>> in the same file for each platform (something like syncro_posix, 
>>> syncro_windows, etc) instead of a separate file for each of them 
>>> (semaphore_*, monitor_*, waitbarrier_*, etc). Otherwise seems 
>>> PlatformParker and PlatformEvent should also be in their own file.
>>
>> Keeping things in separate files can make sense if these things can be 
>> used standalone. A plain mutex (just like the plain semaphore we have) 
>> can come handy in many places where you just want that mutex, without 
>> having to drag in other classes or the whole os layer. Keeps 
>> dependencies under control, reduces compile times, etc.
> Ok. If you don't mind then I can do that in the follow up RFE for the 
> Monitor/Mutex rework after JDK-8217843 since David mentioned is working 
> on moving things too.

Sounds good to me.

/Per


More information about the hotspot-dev mailing list