# HG changeset patch # User dholmes # Date 1565756280 14400 # Node ID 807d192fb7ddffd7b2f6cb97fbeac56bb3a6dd9b # Parent 6bbb4af131e307ca0ae794d1d71a8341f5a90404 8228857: Refactor PlatformMonitor into PlatformMutex and PlatformMonitor Reviewed-by: kbarrett, dcubed, pliden diff -r 6bbb4af131e3 -r 807d192fb7dd src/hotspot/os/posix/os_posix.cpp --- a/src/hotspot/os/posix/os_posix.cpp Wed Aug 14 01:40:29 2019 +0000 +++ b/src/hotspot/os/posix/os_posix.cpp Wed Aug 14 00:18:00 2019 -0400 @@ -1695,8 +1695,8 @@ if ((status = pthread_mutexattr_settype(_mutexAttr, PTHREAD_MUTEX_NORMAL)) != 0) { fatal("pthread_mutexattr_settype: %s", os::strerror(status)); } - // Solaris has it's own PlatformMonitor, distinct from the one for POSIX. - NOT_SOLARIS(os::PlatformMonitor::init();) + // Solaris has it's own PlatformMutex, distinct from the one for POSIX. + NOT_SOLARIS(os::PlatformMutex::init();) } #ifndef SOLARIS @@ -2274,33 +2274,29 @@ } } -// Platform Monitor implementation +// Platform Mutex/Monitor implementation -os::PlatformMonitor::Impl::Impl() : _next(NULL) { - int status = pthread_cond_init(&_cond, _condAttr); - assert_status(status == 0, status, "cond_init"); - status = pthread_mutex_init(&_mutex, _mutexAttr); +#if PLATFORM_MONITOR_IMPL_INDIRECT + +os::PlatformMutex::Mutex::Mutex() : _next(NULL) { + int status = pthread_mutex_init(&_mutex, _mutexAttr); assert_status(status == 0, status, "mutex_init"); } -os::PlatformMonitor::Impl::~Impl() { - int status = pthread_cond_destroy(&_cond); - assert_status(status == 0, status, "cond_destroy"); - status = pthread_mutex_destroy(&_mutex); +os::PlatformMutex::Mutex::~Mutex() { + int status = pthread_mutex_destroy(&_mutex); assert_status(status == 0, status, "mutex_destroy"); } -#if PLATFORM_MONITOR_IMPL_INDIRECT +pthread_mutex_t os::PlatformMutex::_freelist_lock; +os::PlatformMutex::Mutex* os::PlatformMutex::_mutex_freelist = NULL; -pthread_mutex_t os::PlatformMonitor::_freelist_lock; -os::PlatformMonitor::Impl* os::PlatformMonitor::_freelist = NULL; - -void os::PlatformMonitor::init() { +void os::PlatformMutex::init() { int status = pthread_mutex_init(&_freelist_lock, _mutexAttr); assert_status(status == 0, status, "freelist lock init"); } -struct os::PlatformMonitor::WithFreeListLocked : public StackObj { +struct os::PlatformMutex::WithFreeListLocked : public StackObj { WithFreeListLocked() { int status = pthread_mutex_lock(&_freelist_lock); assert_status(status == 0, status, "freelist lock"); @@ -2312,24 +2308,78 @@ } }; -os::PlatformMonitor::PlatformMonitor() { +os::PlatformMutex::PlatformMutex() { { WithFreeListLocked wfl; - _impl = _freelist; + _impl = _mutex_freelist; if (_impl != NULL) { - _freelist = _impl->_next; + _mutex_freelist = _impl->_next; _impl->_next = NULL; return; } } - _impl = new Impl(); + _impl = new Mutex(); +} + +os::PlatformMutex::~PlatformMutex() { + WithFreeListLocked wfl; + assert(_impl->_next == NULL, "invariant"); + _impl->_next = _mutex_freelist; + _mutex_freelist = _impl; +} + +os::PlatformMonitor::Cond::Cond() : _next(NULL) { + int status = pthread_cond_init(&_cond, _condAttr); + assert_status(status == 0, status, "cond_init"); +} + +os::PlatformMonitor::Cond::~Cond() { + int status = pthread_cond_destroy(&_cond); + assert_status(status == 0, status, "cond_destroy"); +} + +os::PlatformMonitor::Cond* os::PlatformMonitor::_cond_freelist = NULL; + +os::PlatformMonitor::PlatformMonitor() { + { + WithFreeListLocked wfl; + _impl = _cond_freelist; + if (_impl != NULL) { + _cond_freelist = _impl->_next; + _impl->_next = NULL; + return; + } + } + _impl = new Cond(); } os::PlatformMonitor::~PlatformMonitor() { WithFreeListLocked wfl; assert(_impl->_next == NULL, "invariant"); - _impl->_next = _freelist; - _freelist = _impl; + _impl->_next = _cond_freelist; + _cond_freelist = _impl; +} + +#else + +os::PlatformMutex::PlatformMutex() { + int status = pthread_mutex_init(&_mutex, _mutexAttr); + assert_status(status == 0, status, "mutex_init"); +} + +os::PlatformMutex::~PlatformMutex() { + int status = pthread_mutex_destroy(&_mutex); + assert_status(status == 0, status, "mutex_destroy"); +} + +os::PlatformMonitor::PlatformMonitor() { + int status = pthread_cond_init(&_cond, _condAttr); + assert_status(status == 0, status, "cond_init"); +} + +os::PlatformMonitor::~PlatformMonitor() { + int status = pthread_cond_destroy(&_cond); + assert_status(status == 0, status, "cond_destroy"); } #endif // PLATFORM_MONITOR_IMPL_INDIRECT diff -r 6bbb4af131e3 -r 807d192fb7dd src/hotspot/os/posix/os_posix.hpp --- a/src/hotspot/os/posix/os_posix.hpp Wed Aug 14 01:40:29 2019 +0000 +++ b/src/hotspot/os/posix/os_posix.hpp Wed Aug 14 00:18:00 2019 -0400 @@ -234,7 +234,7 @@ // Workaround for a bug in macOSX kernel's pthread support (fixed in Mojave?). // Avoid ever allocating a pthread_mutex_t at the same address as one of our -// former pthread_cond_t, by using a freelist of mutex/condvar pairs. +// former pthread_cond_t, by using freelists of mutexes and condvars. // Conditional to avoid extra indirection and padding loss on other platforms. #ifdef __APPLE__ #define PLATFORM_MONITOR_IMPL_INDIRECT 1 @@ -242,53 +242,98 @@ #define PLATFORM_MONITOR_IMPL_INDIRECT 0 #endif -// Platform specific implementation that underpins VM Monitor/Mutex class -class PlatformMonitor : public CHeapObj { - class Impl : public CHeapObj { - public: +// Platform specific implementations that underpin VM Mutex/Monitor classes + +class PlatformMutex : public CHeapObj { +#if PLATFORM_MONITOR_IMPL_INDIRECT + class Mutex : public CHeapObj { + public: pthread_mutex_t _mutex; - pthread_cond_t _cond; - Impl* _next; + Mutex* _next; - Impl(); - ~Impl(); + Mutex(); + ~Mutex(); }; -#if PLATFORM_MONITOR_IMPL_INDIRECT + Mutex* _impl; - Impl* _impl; + static pthread_mutex_t _freelist_lock; // used for mutex and cond freelists + static Mutex* _mutex_freelist; + protected: + class WithFreeListLocked; pthread_mutex_t* mutex() { return &(_impl->_mutex); } - pthread_cond_t* cond() { return &(_impl->_cond); } + + public: + PlatformMutex(); // Use freelist allocation of impl. + ~PlatformMutex(); + + static void init(); // Initialize the freelist. + +#else + + pthread_mutex_t _mutex; + + protected: + pthread_mutex_t* mutex() { return &_mutex; } + + public: + static void init() {} // Nothing needed for the non-indirect case. + + PlatformMutex(); + ~PlatformMutex(); + +#endif // PLATFORM_MONITOR_IMPL_INDIRECT - class WithFreeListLocked; - static pthread_mutex_t _freelist_lock; - static Impl* _freelist; +private: + // Disable copying + PlatformMutex(const PlatformMutex&); + PlatformMutex& operator=(const PlatformMutex&); + + public: + void lock(); + void unlock(); + bool try_lock(); +}; + +class PlatformMonitor : public PlatformMutex { +#if PLATFORM_MONITOR_IMPL_INDIRECT + class Cond : public CHeapObj { + public: + pthread_cond_t _cond; + Cond* _next; + + Cond(); + ~Cond(); + }; + + Cond* _impl; + + static Cond* _cond_freelist; + + pthread_cond_t* cond() { return &(_impl->_cond); } public: PlatformMonitor(); // Use freelist allocation of impl. ~PlatformMonitor(); - static void init(); // Initialize the freelist. - #else - Impl _impl; - - pthread_mutex_t* mutex() { return &(_impl._mutex); } - pthread_cond_t* cond() { return &(_impl._cond); } + pthread_cond_t _cond; + pthread_cond_t* cond() { return &_cond; } public: - static void init() {} // Nothing needed for the non-indirect case. - - // Default constructor and destructor. + PlatformMonitor(); + ~PlatformMonitor(); #endif // PLATFORM_MONITOR_IMPL_INDIRECT + private: + // Disable copying + PlatformMonitor(const PlatformMonitor&); + PlatformMonitor& operator=(const PlatformMonitor&); + public: - void lock(); - void unlock(); - bool try_lock(); int wait(jlong millis); void notify(); void notify_all(); diff -r 6bbb4af131e3 -r 807d192fb7dd src/hotspot/os/posix/os_posix.inline.hpp --- a/src/hotspot/os/posix/os_posix.inline.hpp Wed Aug 14 01:40:29 2019 +0000 +++ b/src/hotspot/os/posix/os_posix.inline.hpp Wed Aug 14 00:18:00 2019 -0400 @@ -47,19 +47,19 @@ #ifndef SOLARIS -// Platform Monitor implementation +// Platform Mutex/Monitor implementation -inline void os::PlatformMonitor::lock() { +inline void os::PlatformMutex::lock() { int status = pthread_mutex_lock(mutex()); assert_status(status == 0, status, "mutex_lock"); } -inline void os::PlatformMonitor::unlock() { +inline void os::PlatformMutex::unlock() { int status = pthread_mutex_unlock(mutex()); assert_status(status == 0, status, "mutex_unlock"); } -inline bool os::PlatformMonitor::try_lock() { +inline bool os::PlatformMutex::try_lock() { int status = pthread_mutex_trylock(mutex()); assert_status(status == 0 || status == EBUSY, status, "mutex_trylock"); return status == 0; diff -r 6bbb4af131e3 -r 807d192fb7dd src/hotspot/os/solaris/os_solaris.cpp --- a/src/hotspot/os/solaris/os_solaris.cpp Wed Aug 14 01:40:29 2019 +0000 +++ b/src/hotspot/os/solaris/os_solaris.cpp Wed Aug 14 00:18:00 2019 -0400 @@ -5149,36 +5149,42 @@ } } -// Platform Monitor implementation +// Platform Mutex/Monitor implementations + +os::PlatformMutex::PlatformMutex() { + int status = os::Solaris::mutex_init(&_mutex); + assert_status(status == 0, status, "mutex_init"); +} + +os::PlatformMutex::~PlatformMutex() { + int status = os::Solaris::mutex_destroy(&_mutex); + assert_status(status == 0, status, "mutex_destroy"); +} + +void os::PlatformMutex::lock() { + int status = os::Solaris::mutex_lock(&_mutex); + assert_status(status == 0, status, "mutex_lock"); +} + +void os::PlatformMutex::unlock() { + int status = os::Solaris::mutex_unlock(&_mutex); + assert_status(status == 0, status, "mutex_unlock"); +} + +bool os::PlatformMutex::try_lock() { + int status = os::Solaris::mutex_trylock(&_mutex); + assert_status(status == 0 || status == EBUSY, status, "mutex_trylock"); + return status == 0; +} os::PlatformMonitor::PlatformMonitor() { int status = os::Solaris::cond_init(&_cond); assert_status(status == 0, status, "cond_init"); - status = os::Solaris::mutex_init(&_mutex); - assert_status(status == 0, status, "mutex_init"); } os::PlatformMonitor::~PlatformMonitor() { int status = os::Solaris::cond_destroy(&_cond); assert_status(status == 0, status, "cond_destroy"); - status = os::Solaris::mutex_destroy(&_mutex); - assert_status(status == 0, status, "mutex_destroy"); -} - -void os::PlatformMonitor::lock() { - int status = os::Solaris::mutex_lock(&_mutex); - assert_status(status == 0, status, "mutex_lock"); -} - -void os::PlatformMonitor::unlock() { - int status = os::Solaris::mutex_unlock(&_mutex); - assert_status(status == 0, status, "mutex_unlock"); -} - -bool os::PlatformMonitor::try_lock() { - int status = os::Solaris::mutex_trylock(&_mutex); - assert_status(status == 0 || status == EBUSY, status, "mutex_trylock"); - return status == 0; } // Must already be locked diff -r 6bbb4af131e3 -r 807d192fb7dd src/hotspot/os/solaris/os_solaris.hpp --- a/src/hotspot/os/solaris/os_solaris.hpp Wed Aug 14 01:40:29 2019 +0000 +++ b/src/hotspot/os/solaris/os_solaris.hpp Wed Aug 14 00:18:00 2019 -0400 @@ -335,18 +335,34 @@ } }; -// Platform specific implementation that underpins VM Monitor/Mutex class -class PlatformMonitor : public CHeapObj { +// Platform specific implementations that underpin VM Mutex/Monitor classes + +class PlatformMutex : public CHeapObj { + // Disable copying + PlatformMutex(const PlatformMutex&); + PlatformMutex& operator=(const PlatformMutex&); + + protected: + mutex_t _mutex; // Native mutex for locking + + public: + PlatformMutex(); + ~PlatformMutex(); + void lock(); + void unlock(); + bool try_lock(); +}; + +class PlatformMonitor : public PlatformMutex { private: - mutex_t _mutex; // Native mutex for locking cond_t _cond; // Native condition variable for blocking + // Disable copying + PlatformMonitor(const PlatformMonitor&); + PlatformMonitor& operator=(const PlatformMonitor&); public: PlatformMonitor(); ~PlatformMonitor(); - void lock(); - void unlock(); - bool try_lock(); int wait(jlong millis); void notify(); void notify_all(); diff -r 6bbb4af131e3 -r 807d192fb7dd src/hotspot/os/windows/os_windows.hpp --- a/src/hotspot/os/windows/os_windows.hpp Wed Aug 14 01:40:29 2019 +0000 +++ b/src/hotspot/os/windows/os_windows.hpp Wed Aug 14 00:18:00 2019 -0400 @@ -187,18 +187,34 @@ } ; -// Platform specific implementation that underpins VM Monitor/Mutex class -class PlatformMonitor : public CHeapObj { +// Platform specific implementations that underpin VM Mutex/Monitor classes + +class PlatformMutex : public CHeapObj { + // Disable copying + PlatformMutex(const PlatformMutex&); + PlatformMutex& operator=(const PlatformMutex&); + + protected: + CRITICAL_SECTION _mutex; // Native mutex for locking + + public: + PlatformMutex(); + ~PlatformMutex(); + void lock(); + void unlock(); + bool try_lock(); +}; + +class PlatformMonitor : public PlatformMutex { private: - CRITICAL_SECTION _mutex; // Native mutex for locking CONDITION_VARIABLE _cond; // Native condition variable for blocking + // Disable copying + PlatformMonitor(const PlatformMonitor&); + PlatformMonitor& operator=(const PlatformMonitor&); public: PlatformMonitor(); ~PlatformMonitor(); - void lock(); - void unlock(); - bool try_lock(); int wait(jlong millis); void notify(); void notify_all(); diff -r 6bbb4af131e3 -r 807d192fb7dd src/hotspot/os/windows/os_windows.inline.hpp --- a/src/hotspot/os/windows/os_windows.inline.hpp Wed Aug 14 01:40:29 2019 +0000 +++ b/src/hotspot/os/windows/os_windows.inline.hpp Wed Aug 14 00:18:00 2019 -0400 @@ -81,26 +81,33 @@ win32::exit_process_or_thread(win32::EPT_PROCESS, num); } -// Platform Monitor implementation +// Platform Mutex/Monitor implementation + +inline os::PlatformMutex::PlatformMutex() { + InitializeCriticalSection(&_mutex); +} + +inline os::PlatformMutex::~PlatformMutex() { + DeleteCriticalSection(&_mutex); +} inline os::PlatformMonitor::PlatformMonitor() { InitializeConditionVariable(&_cond); - InitializeCriticalSection(&_mutex); } inline os::PlatformMonitor::~PlatformMonitor() { - DeleteCriticalSection(&_mutex); + // There is no DeleteConditionVariable API } -inline void os::PlatformMonitor::lock() { +inline void os::PlatformMutex::lock() { EnterCriticalSection(&_mutex); } -inline void os::PlatformMonitor::unlock() { +inline void os::PlatformMutex::unlock() { LeaveCriticalSection(&_mutex); } -inline bool os::PlatformMonitor::try_lock() { +inline bool os::PlatformMutex::try_lock() { return TryEnterCriticalSection(&_mutex); }