8164683: Solaris: JVM abuses thread preemption control
authorcoleenp
Fri, 10 Aug 2018 09:36:01 -0400
changeset 51376 181e6a03249b
parent 51375 b812a85b3aa4
child 51377 f23aeb97ae17
8164683: Solaris: JVM abuses thread preemption control Summary: Complete removal of preemption control and command line arguments (were deprecated in 11). Reviewed-by: hseigel, pchilanomate, dholmes
src/hotspot/os/aix/os_aix.cpp
src/hotspot/os/bsd/os_bsd.cpp
src/hotspot/os/linux/os_linux.cpp
src/hotspot/os/solaris/os_solaris.cpp
src/hotspot/os/windows/os_windows.cpp
src/hotspot/share/compiler/compileBroker.cpp
src/hotspot/share/runtime/globals.hpp
src/hotspot/share/runtime/mutex.cpp
src/hotspot/share/runtime/objectMonitor.cpp
src/hotspot/share/runtime/os.hpp
src/hotspot/share/runtime/safepoint.cpp
src/hotspot/share/runtime/thread.cpp
src/hotspot/share/runtime/thread.hpp
src/hotspot/share/runtime/vmThread.cpp
--- a/src/hotspot/os/aix/os_aix.cpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/os/aix/os_aix.cpp	Fri Aug 10 09:36:01 2018 -0400
@@ -2705,10 +2705,6 @@
   return (ret == 0) ? OS_OK : OS_ERR;
 }
 
-// Hint to the underlying OS that a task switch would not be good.
-// Void return because it's a hint and can fail.
-void os::hint_no_preempt() {}
-
 ////////////////////////////////////////////////////////////////////////////////
 // suspend/resume support
 
--- a/src/hotspot/os/bsd/os_bsd.cpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/os/bsd/os_bsd.cpp	Fri Aug 10 09:36:01 2018 -0400
@@ -2465,10 +2465,6 @@
   return (*priority_ptr != -1 || errno == 0 ? OS_OK : OS_ERR);
 }
 
-// Hint to the underlying OS that a task switch would not be good.
-// Void return because it's a hint and can fail.
-void os::hint_no_preempt() {}
-
 ////////////////////////////////////////////////////////////////////////////////
 // suspend/resume support
 
--- a/src/hotspot/os/linux/os_linux.cpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/os/linux/os_linux.cpp	Fri Aug 10 09:36:01 2018 -0400
@@ -4192,10 +4192,6 @@
   return (*priority_ptr != -1 || errno == 0 ? OS_OK : OS_ERR);
 }
 
-// Hint to the underlying OS that a task switch would not be good.
-// Void return because it's a hint and can fail.
-void os::hint_no_preempt() {}
-
 ////////////////////////////////////////////////////////////////////////////////
 // suspend/resume support
 
--- a/src/hotspot/os/solaris/os_solaris.cpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/os/solaris/os_solaris.cpp	Fri Aug 10 09:36:01 2018 -0400
@@ -78,7 +78,6 @@
 # include <link.h>
 # include <poll.h>
 # include <pthread.h>
-# include <schedctl.h>
 # include <setjmp.h>
 # include <signal.h>
 # include <stdio.h>
@@ -742,7 +741,6 @@
   OSThread* osthr = thread->osthread();
 
   osthr->set_lwp_id(_lwp_self());  // Store lwp in case we are bound
-  thread->_schedctl = (void *) schedctl_init();
 
   log_info(os, thread)("Thread is alive (tid: " UINTX_FORMAT ").",
     os::current_thread_id());
@@ -812,7 +810,6 @@
   // Store info on the Solaris thread into the OSThread
   osthread->set_thread_id(thread_id);
   osthread->set_lwp_id(_lwp_self());
-  thread->_schedctl = (void *) schedctl_init();
 
   if (UseNUMA) {
     int lgrp_id = os::numa_get_group_id();
@@ -3407,13 +3404,6 @@
   return OS_OK;
 }
 
-
-// Hint to the underlying OS that a task switch would not be good.
-// Void return because it's a hint and can fail.
-void os::hint_no_preempt() {
-  schedctl_start(schedctl_init());
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // suspend/resume support
 
--- a/src/hotspot/os/windows/os_windows.cpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/os/windows/os_windows.cpp	Fri Aug 10 09:36:01 2018 -0400
@@ -3609,11 +3609,6 @@
   return OS_OK;
 }
 
-
-// Hint to the underlying OS that a task switch would not be good.
-// Void return because it's a hint and can fail.
-void os::hint_no_preempt() {}
-
 void os::interrupt(Thread* thread) {
   debug_only(Thread::check_for_dangling_thread_pointer(thread);)
 
--- a/src/hotspot/share/compiler/compileBroker.cpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/share/compiler/compileBroker.cpp	Fri Aug 10 09:36:01 2018 -0400
@@ -1777,12 +1777,6 @@
       possibly_add_compiler_threads();
     }
 
-    // Give compiler threads an extra quanta.  They tend to be bursty and
-    // this helps the compiler to finish up the job.
-    if (CompilerThreadHintNoPreempt) {
-      os::hint_no_preempt();
-    }
-
     // Assign the task to the current thread.  Mark this compilation
     // thread as active for the profiler.
     CompileTaskWrapper ctw(task);
--- a/src/hotspot/share/runtime/globals.hpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/share/runtime/globals.hpp	Fri Aug 10 09:36:01 2018 -0400
@@ -2078,12 +2078,6 @@
           "(-1 means no change)")                                           \
           range(-1, 127)                                                    \
                                                                             \
-  product(bool, CompilerThreadHintNoPreempt, false,                         \
-          "(Solaris only) Give compiler threads an extra quanta")           \
-                                                                            \
-  product(bool, VMThreadHintNoPreempt, false,                               \
-          "(Solaris only) Give VM thread an extra quanta")                  \
-                                                                            \
   product(intx, JavaPriority1_To_OSPriority, -1,                            \
           "Map Java priorities to OS priorities")                           \
           range(-1, 127)                                                    \
--- a/src/hotspot/share/runtime/mutex.cpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/share/runtime/mutex.cpp	Fri Aug 10 09:36:01 2018 -0400
@@ -370,11 +370,6 @@
       // CONSIDER: Delay += 1 + (Delay/4); Delay &= 0x7FF ;
     }
 
-    // Consider checking _owner's schedctl state, if OFFPROC abort spin.
-    // If the owner is OFFPROC then it's unlike that the lock will be dropped
-    // in a timely fashion, which suggests that spinning would not be fruitful
-    // or profitable.
-
     // Stall for "Delay" time units - iterations in the current implementation.
     // Avoid generating coherency traffic while stalled.
     // Possible ways to delay:
@@ -553,7 +548,6 @@
   // Only the holder of OnDeck can manipulate EntryList or detach the RATs from cxq.
   // Avoid ABA - allow multiple concurrent producers (enqueue via push-CAS)
   // but only one concurrent consumer (detacher of RATs).
-  // Consider protecting this critical section with schedctl on Solaris.
   // Unlike a normal lock, however, the exiting thread "locks" OnDeck,
   // picks a successor and marks that thread as OnDeck.  That successor
   // thread will then clear OnDeck once it eventually acquires the outer lock.
--- a/src/hotspot/share/runtime/objectMonitor.cpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/share/runtime/objectMonitor.cpp	Fri Aug 10 09:36:01 2018 -0400
@@ -2088,8 +2088,7 @@
 // NotRunnable() -- informed spinning
 //
 // Don't bother spinning if the owner is not eligible to drop the lock.
-// Peek at the owner's schedctl.sc_state and Thread._thread_values and
-// spin only if the owner thread is _thread_in_Java or _thread_in_vm.
+// Spin only if the owner thread is _thread_in_Java or _thread_in_vm.
 // The thread must be runnable in order to drop the lock in timely fashion.
 // If the _owner is not runnable then spinning will not likely be
 // successful (profitable).
@@ -2097,7 +2096,7 @@
 // Beware -- the thread referenced by _owner could have died
 // so a simply fetch from _owner->_thread_state might trap.
 // Instead, we use SafeFetchXX() to safely LD _owner->_thread_state.
-// Because of the lifecycle issues the schedctl and _thread_state values
+// Because of the lifecycle issues, the _thread_state values
 // observed by NotRunnable() might be garbage.  NotRunnable must
 // tolerate this and consider the observed _thread_state value
 // as advisory.
@@ -2105,18 +2104,12 @@
 // Beware too, that _owner is sometimes a BasicLock address and sometimes
 // a thread pointer.
 // Alternately, we might tag the type (thread pointer vs basiclock pointer)
-// with the LSB of _owner.  Another option would be to probablistically probe
+// with the LSB of _owner.  Another option would be to probabilistically probe
 // the putative _owner->TypeTag value.
 //
 // Checking _thread_state isn't perfect.  Even if the thread is
 // in_java it might be blocked on a page-fault or have been preempted
-// and sitting on a ready/dispatch queue.  _thread state in conjunction
-// with schedctl.sc_state gives us a good picture of what the
-// thread is doing, however.
-//
-// TODO: check schedctl.sc_state.
-// We'll need to use SafeFetch32() to read from the schedctl block.
-// See RFE #5004247 and http://sac.sfbay.sun.com/Archives/CaseLog/arc/PSARC/2005/351/
+// and sitting on a ready/dispatch queue.
 //
 // The return value from NotRunnable() is *advisory* -- the
 // result is based on sampling and is not necessarily coherent.
--- a/src/hotspot/share/runtime/os.hpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/share/runtime/os.hpp	Fri Aug 10 09:36:01 2018 -0400
@@ -897,7 +897,6 @@
   static int java_to_os_priority[CriticalPriority + 1];
   // Hint to the underlying OS that a task switch would not be good.
   // Void return because it's a hint and can fail.
-  static void hint_no_preempt();
   static const char* native_thread_creation_failed_msg() {
     return OS_NATIVE_THREAD_CREATION_FAILED_MSG;
   }
--- a/src/hotspot/share/runtime/safepoint.cpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/share/runtime/safepoint.cpp	Fri Aug 10 09:36:01 2018 -0400
@@ -354,26 +354,24 @@
           // See the comments in synchronizer.cpp for additional remarks on spinning.
           //
           // In the future we might:
-          // 1. Modify the safepoint scheme to avoid potentially unbounded spinning.
+          // -- Modify the safepoint scheme to avoid potentially unbounded spinning.
           //    This is tricky as the path used by a thread exiting the JVM (say on
           //    on JNI call-out) simply stores into its state field.  The burden
           //    is placed on the VM thread, which must poll (spin).
-          // 2. Find something useful to do while spinning.  If the safepoint is GC-related
+          // -- Find something useful to do while spinning.  If the safepoint is GC-related
           //    we might aggressively scan the stacks of threads that are already safe.
-          // 3. Use Solaris schedctl to examine the state of the still-running mutators.
-          //    If all the mutators are ONPROC there's no reason to sleep or yield.
-          // 4. YieldTo() any still-running mutators that are ready but OFFPROC.
-          // 5. Check system saturation.  If the system is not fully saturated then
+          // -- YieldTo() any still-running mutators that are ready but OFFPROC.
+          // -- Check system saturation.  If the system is not fully saturated then
           //    simply spin and avoid sleep/yield.
-          // 6. As still-running mutators rendezvous they could unpark the sleeping
+          // -- As still-running mutators rendezvous they could unpark the sleeping
           //    VMthread.  This works well for still-running mutators that become
           //    safe.  The VMthread must still poll for mutators that call-out.
-          // 7. Drive the policy on time-since-begin instead of iterations.
-          // 8. Consider making the spin duration a function of the # of CPUs:
+          // -- Drive the policy on time-since-begin instead of iterations.
+          // -- Consider making the spin duration a function of the # of CPUs:
           //    Spin = (((ncpus-1) * M) + K) + F(still_running)
           //    Alternately, instead of counting iterations of the outer loop
           //    we could count the # of threads visited in the inner loop, above.
-          // 9. On windows consider using the return value from SwitchThreadTo()
+          // -- On windows consider using the return value from SwitchThreadTo()
           //    to drive subsequent spin/SwitchThreadTo()/Sleep(N) decisions.
 
           if (int(iterations) == -1) { // overflow - something is wrong.
@@ -561,20 +559,6 @@
         // Start suspended threads
         jtiwh.rewind();
         for (; JavaThread *current = jtiwh.next(); ) {
-          // A problem occurring on Solaris is when attempting to restart threads
-          // the first #cpus - 1 go well, but then the VMThread is preempted when we get
-          // to the next one (since it has been running the longest).  We then have
-          // to wait for a cpu to become available before we can continue restarting
-          // threads.
-          // FIXME: This causes the performance of the VM to degrade when active and with
-          // large numbers of threads.  Apparently this is due to the synchronous nature
-          // of suspending threads.
-          //
-          // TODO-FIXME: the comments above are vestigial and no longer apply.
-          // Furthermore, using solaris' schedctl in this particular context confers no benefit
-          if (VMThreadHintNoPreempt) {
-            os::hint_no_preempt();
-          }
           ThreadSafepointState* cur_state = current->safepoint_state();
           assert(cur_state->type() != ThreadSafepointState::_running, "Thread not suspended at safepoint");
           cur_state->restart();
--- a/src/hotspot/share/runtime/thread.cpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/share/runtime/thread.cpp	Fri Aug 10 09:36:01 2018 -0400
@@ -282,7 +282,6 @@
   _hashStateW = 273326509;
 
   _OnTrap   = 0;
-  _schedctl = NULL;
   _Stalled  = 0;
   _TypeTag  = 0x2BAD;
 
@@ -4802,7 +4801,6 @@
 //    (List, LOCKBIT:1).  We could also add a SUCCBIT or an explicit _succ variable
 //    to provide the usual futile-wakeup optimization.
 //    See RTStt for details.
-// *  Consider schedctl.sc_nopreempt to cover the critical section.
 //
 
 
--- a/src/hotspot/share/runtime/thread.hpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/share/runtime/thread.hpp	Fri Aug 10 09:36:01 2018 -0400
@@ -726,8 +726,6 @@
   jint _hashStateX;                           // thread-specific hashCode generator state
   jint _hashStateY;
   jint _hashStateZ;
-  void * _schedctl;
-
 
   volatile jint rng[4];                      // RNG for spin loop
 
--- a/src/hotspot/share/runtime/vmThread.cpp	Fri Aug 10 09:30:26 2018 -0400
+++ b/src/hotspot/share/runtime/vmThread.cpp	Fri Aug 10 09:36:01 2018 -0400
@@ -482,13 +482,6 @@
       EventMark em("Executing VM operation: %s", vm_operation()->name());
       assert(_cur_vm_operation != NULL, "we should have found an operation to execute");
 
-      // Give the VM thread an extra quantum.  Jobs tend to be bursty and this
-      // helps the VM thread to finish up the job.
-      // FIXME: When this is enabled and there are many threads, this can degrade
-      // performance significantly.
-      if( VMThreadHintNoPreempt )
-        os::hint_no_preempt();
-
       // If we are at a safepoint we will evaluate all the operations that
       // follow that also require a safepoint
       if (_cur_vm_operation->evaluate_at_safepoint()) {