RFR (S): 8228857: Refactor PlatformMonitor into PlatformMutex and PlatformMonitor
Daniel D. Daugherty
daniel.daugherty at oracle.com
Wed Aug 7 15:37:20 UTC 2019
On 8/1/19 7:03 PM, David Holmes wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8228857
> webrev: http://cr.openjdk.java.net/~dholmes/8228857/webrev/
src/hotspot/os/posix/os_posix.hpp
L245: // Platform specific implementations that underpin VM
Monitor/Mutex classes
Perhaps "Mutex/Monitor" instead of "Monitor/Mutex" now that
Mutex is the base class. And it will be consistent with the
new comments that you've added.
src/hotspot/os/posix/os_posix.inline.hpp
No comments.
src/hotspot/os/posix/os_posix.cpp
L2277: // Platform Monitor implementation
Perhaps: // Platform Mutex/Monitor implementation
like you do in other places.
src/hotspot/os/solaris/os_solaris.hpp
L338: // Platform specific implementations that underpin VM
Monitor/Mutex classes
Perhaps "Mutex/Monitor" instead of "Monitor/Mutex" now that
Mutex is the base class.
src/hotspot/os/solaris/os_solaris.cpp
No comments.
src/hotspot/os/windows/os_windows.hpp
L190: // Platform specific implementations that underpin VM
Monitor/Mutex classes
Perhaps "Mutex/Monitor" instead of "Monitor/Mutex" now that
Mutex is the base class.
src/hotspot/os/windows/os_windows.inline.hpp
L98: inline os::PlatformMonitor::~PlatformMonitor() {
I don't remember why there is no 'DeleteConditionVariable()'
call here. It might be worth a comment to remind the reader.
Thumbs up. If you choose to make the comment tweaks above, I don't
need to see another webrev.
Dan
>
> As suggested by Per Liden when PlatformMonitor was introduced by
> JDK-8210832 we should refactor PlatformMonitor into a simpler
> PlatformMutex extended by PlatformMonitor. That would potentially
> allow other synchronization objects e.g. Zlocks, to be replaced by a
> suitable PlatformX class, and allows us to redefine JVM_RawMonitor
> support to use it (forthcoming change).
>
> The refactoring would be obvious and simple if not for the macOS
> PThread allocation bug workaround. I had to change the Posix variant
> so that we only use the Impl helper class on macOS, so that we can
> separate the allocation of the pthread_mutex_t and pthread_cond_t. For
> macOS both get allocated in the PlatformMutex.
>
> There are not actual usage changes in this RFE.
>
> Testing: mach5 tiers 1-3
>
> Thanks,
> David
> -----
>
More information about the hotspot-dev
mailing list