8061552: Contended Locking speedup PlatformEvent unpark bucket
authordcubed
Wed, 29 Oct 2014 09:19:59 -0700
changeset 27458 eb5f1b4f01e1
parent 27457 adbe834be3a0
child 27459 7fce477785a3
8061552: Contended Locking speedup PlatformEvent unpark bucket Summary: JEP-143/JDK-8046133 - optimization #2 - speedup PlatformEvent unpark bucket. Reviewed-by: acorn, dice, dholmes Contributed-by: dave.dice@oracle.com, karen.kinnear@oracle.com, daniel.daugherty@oracle.com
hotspot/src/os/bsd/vm/os_bsd.cpp
hotspot/src/os/linux/vm/os_linux.cpp
hotspot/src/os/solaris/vm/os_solaris.cpp
hotspot/src/os/windows/vm/os_windows.cpp
--- a/hotspot/src/os/bsd/vm/os_bsd.cpp	Tue Jul 01 13:29:24 2014 -0700
+++ b/hotspot/src/os/bsd/vm/os_bsd.cpp	Wed Oct 29 09:19:59 2014 -0700
@@ -4118,7 +4118,18 @@
 }
 
 
-// Refer to the comments in os_solaris.cpp park-unpark.
+// Refer to the comments in os_solaris.cpp park-unpark. The next two
+// comment paragraphs are worth repeating here:
+//
+// Assumption:
+//    Only one parker can exist on an event, which is why we allocate
+//    them per-thread. Multiple unparkers can coexist.
+//
+// _Event serves as a restricted-range semaphore.
+//   -1 : thread is blocked, i.e. there is a waiter
+//    0 : neutral: thread is running or ready,
+//        could have been signaled after a wait started
+//    1 : signaled - thread is running or ready
 //
 // Beware -- Some versions of NPTL embody a flaw where pthread_cond_timedwait() can
 // hang indefinitely.  For instance NPTL 0.60 on 2.4.21-4ELsmp is vulnerable.
@@ -4203,6 +4214,11 @@
 }
 
 void os::PlatformEvent::park() {       // AKA "down()"
+  // Transitions for _Event:
+  //   -1 => -1 : illegal
+  //    1 =>  0 : pass - return immediately
+  //    0 => -1 : block; then set _Event to 0 before returning
+
   // Invariant: Only the thread associated with the Event/PlatformEvent
   // may call park().
   // TODO: assert that _Assoc != NULL or _Assoc == Self
@@ -4240,6 +4256,11 @@
 }
 
 int os::PlatformEvent::park(jlong millis) {
+  // Transitions for _Event:
+  //   -1 => -1 : illegal
+  //    1 =>  0 : pass - return immediately
+  //    0 => -1 : block; then set _Event to 0 before returning
+
   guarantee(_nParked == 0, "invariant");
 
   int v;
@@ -4303,11 +4324,11 @@
 
 void os::PlatformEvent::unpark() {
   // Transitions for _Event:
-  //    0 :=> 1
-  //    1 :=> 1
-  //   -1 :=> either 0 or 1; must signal target thread
-  //          That is, we can safely transition _Event from -1 to either
-  //          0 or 1.
+  //    0 => 1 : just return
+  //    1 => 1 : just return
+  //   -1 => either 0 or 1; must signal target thread
+  //         That is, we can safely transition _Event from -1 to either
+  //         0 or 1.
   // See also: "Semaphores in Plan 9" by Mullender & Cox
   //
   // Note: Forcing a transition from "-1" to "1" on an unpark() means
@@ -4330,15 +4351,16 @@
   status = pthread_mutex_unlock(_mutex);
   assert_status(status == 0, status, "mutex_unlock");
   if (AnyWaiters != 0) {
+    // Note that we signal() *after* dropping the lock for "immortal" Events.
+    // This is safe and avoids a common class of  futile wakeups.  In rare
+    // circumstances this can cause a thread to return prematurely from
+    // cond_{timed}wait() but the spurious wakeup is benign and the victim
+    // will simply re-test the condition and re-park itself.
+    // This provides particular benefit if the underlying platform does not
+    // provide wait morphing.
     status = pthread_cond_signal(_cond);
     assert_status(status == 0, status, "cond_signal");
   }
-
-  // Note that we signal() _after dropping the lock for "immortal" Events.
-  // This is safe and avoids a common class of  futile wakeups.  In rare
-  // circumstances this can cause a thread to return prematurely from
-  // cond_{timed}wait() but the spurious wakeup is benign and the victim will
-  // simply re-test the condition and re-park itself.
 }
 
 
--- a/hotspot/src/os/linux/vm/os_linux.cpp	Tue Jul 01 13:29:24 2014 -0700
+++ b/hotspot/src/os/linux/vm/os_linux.cpp	Wed Oct 29 09:19:59 2014 -0700
@@ -5415,7 +5415,18 @@
 }
 
 
-// Refer to the comments in os_solaris.cpp park-unpark.
+// Refer to the comments in os_solaris.cpp park-unpark. The next two
+// comment paragraphs are worth repeating here:
+//
+// Assumption:
+//    Only one parker can exist on an event, which is why we allocate
+//    them per-thread. Multiple unparkers can coexist.
+//
+// _Event serves as a restricted-range semaphore.
+//   -1 : thread is blocked, i.e. there is a waiter
+//    0 : neutral: thread is running or ready,
+//        could have been signaled after a wait started
+//    1 : signaled - thread is running or ready
 //
 // Beware -- Some versions of NPTL embody a flaw where pthread_cond_timedwait() can
 // hang indefinitely.  For instance NPTL 0.60 on 2.4.21-4ELsmp is vulnerable.
@@ -5514,6 +5525,11 @@
 }
 
 void os::PlatformEvent::park() {       // AKA "down()"
+  // Transitions for _Event:
+  //   -1 => -1 : illegal
+  //    1 =>  0 : pass - return immediately
+  //    0 => -1 : block; then set _Event to 0 before returning
+
   // Invariant: Only the thread associated with the Event/PlatformEvent
   // may call park().
   // TODO: assert that _Assoc != NULL or _Assoc == Self
@@ -5551,6 +5567,11 @@
 }
 
 int os::PlatformEvent::park(jlong millis) {
+  // Transitions for _Event:
+  //   -1 => -1 : illegal
+  //    1 =>  0 : pass - return immediately
+  //    0 => -1 : block; then set _Event to 0 before returning
+
   guarantee(_nParked == 0, "invariant");
 
   int v;
@@ -5614,11 +5635,11 @@
 
 void os::PlatformEvent::unpark() {
   // Transitions for _Event:
-  //    0 :=> 1
-  //    1 :=> 1
-  //   -1 :=> either 0 or 1; must signal target thread
-  //          That is, we can safely transition _Event from -1 to either
-  //          0 or 1.
+  //    0 => 1 : just return
+  //    1 => 1 : just return
+  //   -1 => either 0 or 1; must signal target thread
+  //         That is, we can safely transition _Event from -1 to either
+  //         0 or 1.
   // See also: "Semaphores in Plan 9" by Mullender & Cox
   //
   // Note: Forcing a transition from "-1" to "1" on an unpark() means
@@ -5641,15 +5662,16 @@
   status = pthread_mutex_unlock(_mutex);
   assert_status(status == 0, status, "mutex_unlock");
   if (AnyWaiters != 0) {
+    // Note that we signal() *after* dropping the lock for "immortal" Events.
+    // This is safe and avoids a common class of  futile wakeups.  In rare
+    // circumstances this can cause a thread to return prematurely from
+    // cond_{timed}wait() but the spurious wakeup is benign and the victim
+    // will simply re-test the condition and re-park itself.
+    // This provides particular benefit if the underlying platform does not
+    // provide wait morphing.
     status = pthread_cond_signal(_cond);
     assert_status(status == 0, status, "cond_signal");
   }
-
-  // Note that we signal() _after dropping the lock for "immortal" Events.
-  // This is safe and avoids a common class of  futile wakeups.  In rare
-  // circumstances this can cause a thread to return prematurely from
-  // cond_{timed}wait() but the spurious wakeup is benign and the victim will
-  // simply re-test the condition and re-park itself.
 }
 
 
--- a/hotspot/src/os/solaris/vm/os_solaris.cpp	Tue Jul 01 13:29:24 2014 -0700
+++ b/hotspot/src/os/solaris/vm/os_solaris.cpp	Wed Oct 29 09:19:59 2014 -0700
@@ -5372,31 +5372,32 @@
 // to immediately return 0 your code should still work,
 // albeit degenerating to a spin loop.
 //
-// An interesting optimization for park() is to use a trylock()
-// to attempt to acquire the mutex.  If the trylock() fails
-// then we know that a concurrent unpark() operation is in-progress.
-// in that case the park() code could simply set _count to 0
-// and return immediately.  The subsequent park() operation *might*
-// return immediately.  That's harmless as the caller of park() is
-// expected to loop.  By using trylock() we will have avoided a
-// avoided a context switch caused by contention on the per-thread mutex.
+// In a sense, park()-unpark() just provides more polite spinning
+// and polling with the key difference over naive spinning being
+// that a parked thread needs to be explicitly unparked() in order
+// to wake up and to poll the underlying condition.
 //
-// TODO-FIXME:
-// 1.  Reconcile Doug's JSR166 j.u.c park-unpark with the
-//     objectmonitor implementation.
-// 2.  Collapse the JSR166 parker event, and the
-//     objectmonitor ParkEvent into a single "Event" construct.
-// 3.  In park() and unpark() add:
-//     assert (Thread::current() == AssociatedWith).
-// 4.  add spurious wakeup injection on a -XX:EarlyParkReturn=N switch.
-//     1-out-of-N park() operations will return immediately.
+// Assumption:
+//    Only one parker can exist on an event, which is why we allocate
+//    them per-thread. Multiple unparkers can coexist.
 //
 // _Event transitions in park()
 //   -1 => -1 : illegal
 //    1 =>  0 : pass - return immediately
-//    0 => -1 : block
+//    0 => -1 : block; then set _Event to 0 before returning
+//
+// _Event transitions in unpark()
+//    0 => 1 : just return
+//    1 => 1 : just return
+//   -1 => either 0 or 1; must signal target thread
+//         That is, we can safely transition _Event from -1 to either
+//         0 or 1.
 //
 // _Event serves as a restricted-range semaphore.
+//   -1 : thread is blocked, i.e. there is a waiter
+//    0 : neutral: thread is running or ready,
+//        could have been signaled after a wait started
+//    1 : signaled - thread is running or ready
 //
 // Another possible encoding of _Event would be with
 // explicit "PARKED" == 01b and "SIGNALED" == 10b bits.
@@ -5456,6 +5457,11 @@
 }
 
 void os::PlatformEvent::park() {           // AKA: down()
+  // Transitions for _Event:
+  //   -1 => -1 : illegal
+  //    1 =>  0 : pass - return immediately
+  //    0 => -1 : block; then set _Event to 0 before returning
+
   // Invariant: Only the thread associated with the Event/PlatformEvent
   // may call park().
   assert(_nParked == 0, "invariant");
@@ -5497,6 +5503,11 @@
 }
 
 int os::PlatformEvent::park(jlong millis) {
+  // Transitions for _Event:
+  //   -1 => -1 : illegal
+  //    1 =>  0 : pass - return immediately
+  //    0 => -1 : block; then set _Event to 0 before returning
+
   guarantee(_nParked == 0, "invariant");
   int v;
   for (;;) {
@@ -5542,11 +5553,11 @@
 
 void os::PlatformEvent::unpark() {
   // Transitions for _Event:
-  //    0 :=> 1
-  //    1 :=> 1
-  //   -1 :=> either 0 or 1; must signal target thread
-  //          That is, we can safely transition _Event from -1 to either
-  //          0 or 1.
+  //    0 => 1 : just return
+  //    1 => 1 : just return
+  //   -1 => either 0 or 1; must signal target thread
+  //         That is, we can safely transition _Event from -1 to either
+  //         0 or 1.
   // See also: "Semaphores in Plan 9" by Mullender & Cox
   //
   // Note: Forcing a transition from "-1" to "1" on an unpark() means
@@ -5566,8 +5577,13 @@
   assert_status(status == 0, status, "mutex_unlock");
   guarantee(AnyWaiters == 0 || AnyWaiters == 1, "invariant");
   if (AnyWaiters != 0) {
-    // We intentional signal *after* dropping the lock
-    // to avoid a common class of futile wakeups.
+    // Note that we signal() *after* dropping the lock for "immortal" Events.
+    // This is safe and avoids a common class of  futile wakeups.  In rare
+    // circumstances this can cause a thread to return prematurely from
+    // cond_{timed}wait() but the spurious wakeup is benign and the victim
+    // will simply re-test the condition and re-park itself.
+    // This provides particular benefit if the underlying platform does not
+    // provide wait morphing.
     status = os::Solaris::cond_signal(_cond);
     assert_status(status == 0, status, "cond_signal");
   }
--- a/hotspot/src/os/windows/vm/os_windows.cpp	Tue Jul 01 13:29:24 2014 -0700
+++ b/hotspot/src/os/windows/vm/os_windows.cpp	Wed Oct 29 09:19:59 2014 -0700
@@ -4792,27 +4792,46 @@
 // 3.  Collapse the interrupt_event, the JSR166 parker event, and the objectmonitor ParkEvent
 //     into a single win32 CreateEvent() handle.
 //
+// Assumption:
+//    Only one parker can exist on an event, which is why we allocate
+//    them per-thread. Multiple unparkers can coexist.
+//
 // _Event transitions in park()
 //   -1 => -1 : illegal
 //    1 =>  0 : pass - return immediately
-//    0 => -1 : block
+//    0 => -1 : block; then set _Event to 0 before returning
+//
+// _Event transitions in unpark()
+//    0 => 1 : just return
+//    1 => 1 : just return
+//   -1 => either 0 or 1; must signal target thread
+//         That is, we can safely transition _Event from -1 to either
+//         0 or 1.
 //
-// _Event serves as a restricted-range semaphore :
-//    -1 : thread is blocked
-//     0 : neutral  - thread is running or ready
-//     1 : signaled - thread is running or ready
+// _Event serves as a restricted-range semaphore.
+//   -1 : thread is blocked, i.e. there is a waiter
+//    0 : neutral: thread is running or ready,
+//        could have been signaled after a wait started
+//    1 : signaled - thread is running or ready
 //
-// Another possible encoding of _Event would be
-// with explicit "PARKED" and "SIGNALED" bits.
+// Another possible encoding of _Event would be with
+// explicit "PARKED" == 01b and "SIGNALED" == 10b bits.
+//
 
 int os::PlatformEvent::park(jlong Millis) {
+  // Transitions for _Event:
+  //   -1 => -1 : illegal
+  //    1 =>  0 : pass - return immediately
+  //    0 => -1 : block; then set _Event to 0 before returning
+
   guarantee(_ParkHandle != NULL , "Invariant");
   guarantee(Millis > 0          , "Invariant");
-  int v;
 
   // CONSIDER: defer assigning a CreateEvent() handle to the Event until
   // the initial park() operation.
-
+  // Consider: use atomic decrement instead of CAS-loop
+
+  int v;
   for (;;) {
     v = _Event;
     if (Atomic::cmpxchg(v-1, &_Event, v) == v) break;
@@ -4860,9 +4879,15 @@
 }
 
 void os::PlatformEvent::park() {
+  // Transitions for _Event:
+  //   -1 => -1 : illegal
+  //    1 =>  0 : pass - return immediately
+  //    0 => -1 : block; then set _Event to 0 before returning
+
   guarantee(_ParkHandle != NULL, "Invariant");
   // Invariant: Only the thread associated with the Event/PlatformEvent
   // may call park().
+  // Consider: use atomic decrement instead of CAS-loop
   int v;
   for (;;) {
     v = _Event;
@@ -4891,11 +4916,11 @@
   guarantee(_ParkHandle != NULL, "Invariant");
 
   // Transitions for _Event:
-  //    0 :=> 1
-  //    1 :=> 1
-  //   -1 :=> either 0 or 1; must signal target thread
-  //          That is, we can safely transition _Event from -1 to either
-  //          0 or 1.
+  //    0 => 1 : just return
+  //    1 => 1 : just return
+  //   -1 => either 0 or 1; must signal target thread
+  //         That is, we can safely transition _Event from -1 to either
+  //         0 or 1.
   // See also: "Semaphores in Plan 9" by Mullender & Cox
   //
   // Note: Forcing a transition from "-1" to "1" on an unpark() means