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