8228857: Refactor PlatformMonitor into PlatformMutex and PlatformMonitor

Reviewed-by: kbarrett, dcubed, pliden

src/hotspot/os/posix/os_posix.cpp

@@ -1695,8 +1695,8 @@ static void pthread_init_common(void) {
   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.
+  // Solaris has it's own PlatformMutex, distinct from the one for POSIX.
-  NOT_SOLARIS(os::PlatformMonitor::init();)
+  NOT_SOLARIS(os::PlatformMutex::init();)
 }
 
 #ifndef SOLARIS
@@ -2274,33 +2274,29 @@ void Parker::unpark() {
   }
 }
 
-// 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);
-  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);
-  assert_status(status == 0, status, "mutex_destroy");
-}
 
 #if PLATFORM_MONITOR_IMPL_INDIRECT
 
-pthread_mutex_t os::PlatformMonitor::_freelist_lock;
+os::PlatformMutex::Mutex::Mutex() : _next(NULL) {
-os::PlatformMonitor::Impl* os::PlatformMonitor::_freelist = NULL;
+  int status = pthread_mutex_init(&_mutex, _mutexAttr);
+  assert_status(status == 0, status, "mutex_init");
+}
 
-void os::PlatformMonitor::init() {
+os::PlatformMutex::Mutex::~Mutex() {
+  int status = pthread_mutex_destroy(&_mutex);
+  assert_status(status == 0, status, "mutex_destroy");
+}
+
+pthread_mutex_t os::PlatformMutex::_freelist_lock;
+os::PlatformMutex::Mutex* os::PlatformMutex::_mutex_freelist = NULL;
+
+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 @@ struct os::PlatformMonitor::WithFreeListLocked : public StackObj {
   }
 };
 
-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;
+  _impl->_next = _cond_freelist;
-  _freelist = _impl;
+  _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

src/hotspot/os/posix/os_posix.hpp

@@ -234,7 +234,7 @@ class PlatformParker : public CHeapObj<mtSynchronizer> {
 
 // 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 @@ class PlatformParker : public CHeapObj<mtSynchronizer> {
 #define PLATFORM_MONITOR_IMPL_INDIRECT 0
 #endif
 
-// Platform specific implementation that underpins VM Monitor/Mutex class
+// Platform specific implementations that underpin VM Mutex/Monitor classes
-class PlatformMonitor : public CHeapObj<mtSynchronizer> {
-  class Impl : public CHeapObj<mtSynchronizer> {
-  public:
-    pthread_mutex_t _mutex;
-    pthread_cond_t _cond;
-    Impl* _next;
 
-    Impl();
+class PlatformMutex : public CHeapObj<mtSynchronizer> {
-    ~Impl();
+#if PLATFORM_MONITOR_IMPL_INDIRECT
+  class Mutex : public CHeapObj<mtSynchronizer> {
+   public:
+    pthread_mutex_t _mutex;
+    Mutex* _next;
+
+    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;
-  pthread_mutex_t* mutex() { return &(_impl->_mutex); }
-  pthread_cond_t* cond() { return &(_impl->_cond); }
 
+ protected:
   class WithFreeListLocked;
-  static pthread_mutex_t _freelist_lock;
+  pthread_mutex_t* mutex() { return &(_impl->_mutex); }
-  static Impl* _freelist;
 
  public:
-  PlatformMonitor();            // Use freelist allocation of impl.
+  PlatformMutex();              // Use freelist allocation of impl.
-  ~PlatformMonitor();
+  ~PlatformMutex();
 
   static void init();           // Initialize the freelist.
 
 #else
 
-  Impl _impl;
+  pthread_mutex_t _mutex;
 
-  pthread_mutex_t* mutex() { return &(_impl._mutex); }
+ protected:
-  pthread_cond_t* cond() { return &(_impl._cond); }
+  pthread_mutex_t* mutex() { return &_mutex; }
 
  public:
   static void init() {}         // Nothing needed for the non-indirect case.
 
-  // Default constructor and destructor.
+  PlatformMutex();
+  ~PlatformMutex();
 
 #endif // PLATFORM_MONITOR_IMPL_INDIRECT
 
+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<mtSynchronizer> {
+   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();
+
+#else
+
+  pthread_cond_t _cond;
+  pthread_cond_t* cond() { return &_cond; }
+
+ public:
+  PlatformMonitor();
+  ~PlatformMonitor();
+
+#endif // PLATFORM_MONITOR_IMPL_INDIRECT
+
+ private:
+  // Disable copying
+  PlatformMonitor(const PlatformMonitor&);
+  PlatformMonitor& operator=(const PlatformMonitor&);
+
+ public:
   int wait(jlong millis);
   void notify();
   void notify_all();

src/hotspot/os/posix/os_posix.inline.hpp

@@ -47,19 +47,19 @@ inline int os::Posix::clock_getres(clockid_t clock_id, struct timespec *tp) {
 
 #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;

src/hotspot/os/solaris/os_solaris.cpp

@@ -5149,36 +5149,42 @@ void Parker::unpark() {
   }
 }
 
-// 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

src/hotspot/os/solaris/os_solaris.hpp

@@ -335,18 +335,34 @@ class PlatformParker : public CHeapObj<mtSynchronizer> {
   }
 };
 
-// Platform specific implementation that underpins VM Monitor/Mutex class
+// Platform specific implementations that underpin VM Mutex/Monitor classes
-class PlatformMonitor : public CHeapObj<mtSynchronizer> {
+
- private:
+class PlatformMutex : public CHeapObj<mtSynchronizer> {
+  // 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:
   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();

src/hotspot/os/windows/os_windows.hpp

@@ -187,18 +187,34 @@ class PlatformParker : public CHeapObj<mtSynchronizer> {
 
 } ;
 
-// Platform specific implementation that underpins VM Monitor/Mutex class
+// Platform specific implementations that underpin VM Mutex/Monitor classes
-class PlatformMonitor : public CHeapObj<mtSynchronizer> {
+
- private:
+class PlatformMutex : public CHeapObj<mtSynchronizer> {
+  // 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:
   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();

src/hotspot/os/windows/os_windows.inline.hpp

@@ -81,26 +81,33 @@ inline void os::exit(int num) {
   win32::exit_process_or_thread(win32::EPT_PROCESS, num);
 }
 
-// Platform Monitor implementation
+// Platform Mutex/Monitor implementation
 
-inline os::PlatformMonitor::PlatformMonitor() {
+inline os::PlatformMutex::PlatformMutex() {
-  InitializeConditionVariable(&_cond);
   InitializeCriticalSection(&_mutex);
 }
 
-inline os::PlatformMonitor::~PlatformMonitor() {
+inline os::PlatformMutex::~PlatformMutex() {
   DeleteCriticalSection(&_mutex);
 }
 
-inline void os::PlatformMonitor::lock() {
+inline os::PlatformMonitor::PlatformMonitor() {
+  InitializeConditionVariable(&_cond);
+}
+
+inline os::PlatformMonitor::~PlatformMonitor() {
+  // There is no DeleteConditionVariable API
+}
+
+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);
 }