8023461: Thread holding lock at safepoint that vm can block on: MethodCompileQueue_lock

Reviewed-by: kvn, iveresov

src/share/vm/compiler/compileBroker.cpp

@@ -688,13 +688,39 @@ CompileTask* CompileQueue::get() {
     return NULL;
   }
 
-  CompileTask* task = CompilationPolicy::policy()->select_task(this);
+  CompileTask* task;
+  {
+    No_Safepoint_Verifier nsv;
+    task = CompilationPolicy::policy()->select_task(this);
+  }
   remove(task);
+  purge_stale_tasks(); // may temporarily release MCQ lock
   return task;
 }
 
-void CompileQueue::remove(CompileTask* task)
-{
+// Clean & deallocate stale compile tasks.
+// Temporarily releases MethodCompileQueue lock.
+void CompileQueue::purge_stale_tasks() {
+  assert(lock()->owned_by_self(), "must own lock");
+  if (_first_stale != NULL) {
+    // Stale tasks are purged when MCQ lock is released,
+    // but _first_stale updates are protected by MCQ lock.
+    // Once task processing starts and MCQ lock is released,
+    // other compiler threads can reuse _first_stale.
+    CompileTask* head = _first_stale;
+    _first_stale = NULL;
+    {
+      MutexUnlocker ul(lock());
+      for (CompileTask* task = head; task != NULL; ) {
+        CompileTask* next_task = task->next();
+        CompileTaskWrapper ctw(task); // Frees the task
+        task = next_task;
+      }
+    }
+  }
+}
+
+void CompileQueue::remove(CompileTask* task) {
    assert(lock()->owned_by_self(), "must own lock");
   if (task->prev() != NULL) {
     task->prev()->set_next(task->next());
@@ -714,6 +740,16 @@ void CompileQueue::remove(CompileTask* task)
   --_size;
 }
 
+void CompileQueue::remove_and_mark_stale(CompileTask* task) {
+  assert(lock()->owned_by_self(), "must own lock");
+  remove(task);
+
+  // Enqueue the task for reclamation (should be done outside MCQ lock)
+  task->set_next(_first_stale);
+  task->set_prev(NULL);
+  _first_stale = task;
+}
+
 // methods in the compile queue need to be marked as used on the stack
 // so that they don't get reclaimed by Redefine Classes
 void CompileQueue::mark_on_stack() {
@@ -2011,7 +2047,7 @@ void CompileBroker::invoke_compiler_on_method(CompileTask* task) {
 
   // Note that the queued_for_compilation bits are cleared without
   // protection of a mutex. [They were set by the requester thread,
-  // when adding the task to the complie queue -- at which time the
+  // when adding the task to the compile queue -- at which time the
   // compile queue lock was held. Subsequently, we acquired the compile
   // queue lock to get this task off the compile queue; thus (to belabour
   // the point somewhat) our clearing of the bits must be occurring

src/share/vm/compiler/compileBroker.hpp

@@ -188,7 +188,11 @@ class CompileQueue : public CHeapObj<mtCompiler> {
   CompileTask* _first;
   CompileTask* _last;
 
+  CompileTask* _first_stale;
+
   int _size;
+
+  void purge_stale_tasks();
  public:
   CompileQueue(const char* name, Monitor* lock) {
     _name = name;
@@ -196,6 +200,7 @@ class CompileQueue : public CHeapObj<mtCompiler> {
     _first = NULL;
     _last = NULL;
     _size = 0;
+    _first_stale = NULL;
   }
 
   const char*  name() const                      { return _name; }
@@ -203,6 +208,7 @@ class CompileQueue : public CHeapObj<mtCompiler> {
 
   void         add(CompileTask* task);
   void         remove(CompileTask* task);
+  void         remove_and_mark_stale(CompileTask* task);
   CompileTask* first()                           { return _first; }
   CompileTask* last()                            { return _last;  }
 
@@ -211,6 +217,7 @@ class CompileQueue : public CHeapObj<mtCompiler> {
   bool         is_empty() const                  { return _first == NULL; }
   int          size()     const                  { return _size;          }
 
+
   // Redefine Classes support
   void mark_on_stack();
   void delete_all();

src/share/vm/oops/method.hpp

@@ -233,10 +233,11 @@ class Method : public Metadata {
   // Tracking number of breakpoints, for fullspeed debugging.
   // Only mutated by VM thread.
   u2   number_of_breakpoints()             const {
-    if (method_counters() == NULL) {
+    MethodCounters* mcs = method_counters();
+    if (mcs == NULL) {
       return 0;
     } else {
-      return method_counters()->number_of_breakpoints();
+      return mcs->number_of_breakpoints();
     }
   }
   void incr_number_of_breakpoints(TRAPS)         {
@@ -253,8 +254,9 @@ class Method : public Metadata {
   }
   // Initialization only
   void clear_number_of_breakpoints()             {
-    if (method_counters() != NULL) {
-      method_counters()->clear_number_of_breakpoints();
+    MethodCounters* mcs = method_counters();
+    if (mcs != NULL) {
+      mcs->clear_number_of_breakpoints();
     }
   }
 
@@ -301,10 +303,11 @@ class Method : public Metadata {
   }
 
   int  interpreter_throwout_count() const        {
-    if (method_counters() == NULL) {
+    MethodCounters* mcs = method_counters();
+    if (mcs == NULL) {
       return 0;
     } else {
-      return method_counters()->interpreter_throwout_count();
+      return mcs->interpreter_throwout_count();
     }
   }
 
@@ -379,26 +382,28 @@ class Method : public Metadata {
       return method_counters()->interpreter_invocation_count();
     }
   }
-  void set_prev_event_count(int count, TRAPS)    {
-    MethodCounters* mcs = get_method_counters(CHECK);
+  void set_prev_event_count(int count) {
+    MethodCounters* mcs = method_counters();
     if (mcs != NULL) {
       mcs->set_interpreter_invocation_count(count);
     }
   }
   jlong prev_time() const                        {
-    return method_counters() == NULL ? 0 : method_counters()->prev_time();
+    MethodCounters* mcs = method_counters();
+    return mcs == NULL ? 0 : mcs->prev_time();
   }
-  void set_prev_time(jlong time, TRAPS)          {
-    MethodCounters* mcs = get_method_counters(CHECK);
+  void set_prev_time(jlong time) {
+    MethodCounters* mcs = method_counters();
     if (mcs != NULL) {
       mcs->set_prev_time(time);
     }
   }
   float rate() const                             {
-    return method_counters() == NULL ? 0 : method_counters()->rate();
+    MethodCounters* mcs = method_counters();
+    return mcs == NULL ? 0 : mcs->rate();
   }
-  void set_rate(float rate, TRAPS) {
-    MethodCounters* mcs = get_method_counters(CHECK);
+  void set_rate(float rate) {
+    MethodCounters* mcs = method_counters();
     if (mcs != NULL) {
       mcs->set_rate(rate);
     }
@@ -416,9 +421,12 @@ class Method : public Metadata {
   static MethodCounters* build_method_counters(Method* m, TRAPS);
 
   int interpreter_invocation_count() {
-    if (TieredCompilation) return invocation_count();
-    else return (method_counters() == NULL) ? 0 :
-                 method_counters()->interpreter_invocation_count();
+    if (TieredCompilation) {
+      return invocation_count();
+    } else {
+      MethodCounters* mcs = method_counters();
+      return (mcs == NULL) ? 0 : mcs->interpreter_invocation_count();
+    }
   }
   int increment_interpreter_invocation_count(TRAPS) {
     if (TieredCompilation) ShouldNotReachHere();

src/share/vm/prims/whitebox.cpp

@@ -526,8 +526,8 @@ WB_ENTRY(void, WB_ClearMethodState(JNIEnv* env, jobject o, jobject method))
 
 #ifdef TIERED
     mcs->set_rate(0.0F);
-    mh->set_prev_event_count(0, THREAD);
-    mh->set_prev_time(0, THREAD);
+    mh->set_prev_event_count(0);
+    mh->set_prev_time(0);
 #endif
   }
 WB_END

src/share/vm/runtime/advancedThresholdPolicy.cpp

@@ -75,11 +75,14 @@ void AdvancedThresholdPolicy::initialize() {
 
 // update_rate() is called from select_task() while holding a compile queue lock.
 void AdvancedThresholdPolicy::update_rate(jlong t, Method* m) {
-  JavaThread* THREAD = JavaThread::current();
+  // Skip update if counters are absent.
+  // Can't allocate them since we are holding compile queue lock.
+  if (m->method_counters() == NULL)  return;
+
   if (is_old(m)) {
     // We don't remove old methods from the queue,
     // so we can just zero the rate.
-    m->set_rate(0, THREAD);
+    m->set_rate(0);
     return;
   }
 
@@ -95,14 +98,15 @@ void AdvancedThresholdPolicy::update_rate(jlong t, Method* m) {
   if (delta_s >= TieredRateUpdateMinTime) {
     // And we must've taken the previous point at least 1ms before.
     if (delta_t >= TieredRateUpdateMinTime && delta_e > 0) {
-      m->set_prev_time(t, THREAD);
-      m->set_prev_event_count(event_count, THREAD);
-      m->set_rate((float)delta_e / (float)delta_t, THREAD); // Rate is events per millisecond
-    } else
+      m->set_prev_time(t);
+      m->set_prev_event_count(event_count);
+      m->set_rate((float)delta_e / (float)delta_t); // Rate is events per millisecond
+    } else {
       if (delta_t > TieredRateUpdateMaxTime && delta_e == 0) {
         // If nothing happened for 25ms, zero the rate. Don't modify prev values.
-        m->set_rate(0, THREAD);
+        m->set_rate(0);
       }
+    }
   }
 }
 
@@ -164,7 +168,6 @@ CompileTask* AdvancedThresholdPolicy::select_task(CompileQueue* compile_queue) {
   for (CompileTask* task = compile_queue->first(); task != NULL;) {
     CompileTask* next_task = task->next();
     Method* method = task->method();
-    MethodData* mdo = method->method_data();
     update_rate(t, method);
     if (max_task == NULL) {
       max_task = task;
@@ -175,8 +178,7 @@ CompileTask* AdvancedThresholdPolicy::select_task(CompileQueue* compile_queue) {
         if (PrintTieredEvents) {
           print_event(REMOVE_FROM_QUEUE, method, method, task->osr_bci(), (CompLevel)task->comp_level());
         }
-        CompileTaskWrapper ctw(task); // Frees the task
-        compile_queue->remove(task);
+        compile_queue->remove_and_mark_stale(task);
         method->clear_queued_for_compilation();
         task = next_task;
         continue;