8023461: Thread holding lock at safepoint that vm can block on: MethodCompileQueue_lock
Reviewed-by: kvn, iveresov
--- a/hotspot/src/share/vm/compiler/compileBroker.cpp Tue May 13 11:32:10 2014 -0700
+++ b/hotspot/src/share/vm/compiler/compileBroker.cpp Tue Mar 11 15:06:34 2014 +0400
@@ -704,13 +704,39 @@
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());
@@ -730,6 +756,16 @@
--_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() {
@@ -2006,7 +2042,7 @@
// 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
--- a/hotspot/src/share/vm/compiler/compileBroker.hpp Tue May 13 11:32:10 2014 -0700
+++ b/hotspot/src/share/vm/compiler/compileBroker.hpp Tue Mar 11 15:06:34 2014 +0400
@@ -196,7 +196,11 @@
CompileTask* _first;
CompileTask* _last;
+ CompileTask* _first_stale;
+
int _size;
+
+ void purge_stale_tasks();
public:
CompileQueue(const char* name, Monitor* lock) {
_name = name;
@@ -204,6 +208,7 @@
_first = NULL;
_last = NULL;
_size = 0;
+ _first_stale = NULL;
}
const char* name() const { return _name; }
@@ -211,6 +216,7 @@
void add(CompileTask* task);
void remove(CompileTask* task);
+ void remove_and_mark_stale(CompileTask* task);
CompileTask* first() { return _first; }
CompileTask* last() { return _last; }
@@ -219,6 +225,7 @@
bool is_empty() const { return _first == NULL; }
int size() const { return _size; }
+
// Redefine Classes support
void mark_on_stack();
void free_all();
--- a/hotspot/src/share/vm/oops/method.hpp Tue May 13 11:32:10 2014 -0700
+++ b/hotspot/src/share/vm/oops/method.hpp Tue Mar 11 15:06:34 2014 +0400
@@ -200,10 +200,11 @@
// 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) {
@@ -220,8 +221,9 @@
}
// 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();
}
}
@@ -268,10 +270,11 @@
}
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();
}
}
@@ -346,26 +349,28 @@
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);
}
@@ -390,9 +395,12 @@
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();
--- a/hotspot/src/share/vm/prims/whitebox.cpp Tue May 13 11:32:10 2014 -0700
+++ b/hotspot/src/share/vm/prims/whitebox.cpp Tue Mar 11 15:06:34 2014 +0400
@@ -495,8 +495,8 @@
#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
--- a/hotspot/src/share/vm/runtime/advancedThresholdPolicy.cpp Tue May 13 11:32:10 2014 -0700
+++ b/hotspot/src/share/vm/runtime/advancedThresholdPolicy.cpp Tue Mar 11 15:06:34 2014 +0400
@@ -75,11 +75,14 @@
// 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 @@
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 @@
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 @@
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;