changeset 34196:72152eea3d39

8141330: [JVMCI] avoid deadlock between application thread and JVMCI compiler thread under -Xbatch Reviewed-by: twisti Contributed-by: Doug Simon <doug.simon@oracle.com>
author twisti
date Wed, 18 Nov 2015 09:43:31 -1000
parents 89011d12ebd3
children bef14f844171
files hotspot/src/share/vm/compiler/compileBroker.cpp hotspot/src/share/vm/compiler/compileTask.cpp hotspot/src/share/vm/compiler/compileTask.hpp
diffstat 3 files changed, 65 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/hotspot/src/share/vm/compiler/compileBroker.cpp	Wed Nov 18 03:03:43 2015 +0300
+++ b/hotspot/src/share/vm/compiler/compileBroker.cpp	Wed Nov 18 09:43:31 2015 -1000
@@ -238,10 +238,27 @@
   task->set_code_handle(NULL);
   thread->set_env(NULL);
   if (task->is_blocking()) {
-    MutexLocker notifier(task->lock(), thread);
-    task->mark_complete();
-    // Notify the waiting thread that the compilation has completed.
-    task->lock()->notify_all();
+    bool free_task = false;
+    {
+      MutexLocker notifier(task->lock(), thread);
+      task->mark_complete();
+#if INCLUDE_JVMCI
+      if (CompileBroker::compiler(task->comp_level())->is_jvmci() &&
+        !task->has_waiter()) {
+        // The waiting thread timed out and thus did not free the task.
+        free_task = true;
+      }
+#endif
+      if (!free_task) {
+        // Notify the waiting thread that the compilation has completed
+        // so that it can free the task.
+        task->lock()->notify_all();
+      }
+    }
+    if (free_task) {
+      // The task can only be freed once the task lock is released.
+      CompileTask::free(task);
+    }
   } else {
     task->mark_complete();
 
@@ -1302,6 +1319,11 @@
   return new_task;
 }
 
+// 1 second should be long enough to complete most JVMCI compilations
+// and not too long to stall a blocking JVMCI compilation that
+// is trying to acquire a lock held by the app thread that submitted the
+// compilation.
+static const long BLOCKING_JVMCI_COMPILATION_TIMEOUT = 1000;
 
 /**
  *  Wait for the compilation task to complete.
@@ -1318,30 +1340,47 @@
   thread->set_blocked_on_compilation(true);
 
   methodHandle method(thread, task->method());
+  bool free_task;
+#if INCLUDE_JVMCI
+  if (compiler(task->comp_level())->is_jvmci()) {
+    MutexLocker waiter(task->lock(), thread);
+    // No need to check if compilation has completed - just
+    // rely on the time out. The JVMCI compiler thread will
+    // recycle the CompileTask.
+    task->lock()->wait(!Mutex::_no_safepoint_check_flag, BLOCKING_JVMCI_COMPILATION_TIMEOUT);
+    // If the compilation completes while has_waiter is true then
+    // this thread is responsible for freeing the task.  Otherwise
+    // the compiler thread will free the task.
+    task->clear_waiter();
+    free_task = task->is_complete();
+  } else
+#endif
   {
     MutexLocker waiter(task->lock(), thread);
-
+    free_task = true;
     while (!task->is_complete() && !is_compilation_disabled_forever()) {
       task->lock()->wait();
     }
   }
 
   thread->set_blocked_on_compilation(false);
-  if (is_compilation_disabled_forever()) {
+  if (free_task) {
+    if (is_compilation_disabled_forever()) {
+      CompileTask::free(task);
+      return;
+    }
+
+    // It is harmless to check this status without the lock, because
+    // completion is a stable property (until the task object is recycled).
+    assert(task->is_complete(), "Compilation should have completed");
+    assert(task->code_handle() == NULL, "must be reset");
+
+    // By convention, the waiter is responsible for recycling a
+    // blocking CompileTask. Since there is only one waiter ever
+    // waiting on a CompileTask, we know that no one else will
+    // be using this CompileTask; we can free it.
     CompileTask::free(task);
-    return;
   }
-
-  // It is harmless to check this status without the lock, because
-  // completion is a stable property (until the task object is recycled).
-  assert(task->is_complete(), "Compilation should have completed");
-  assert(task->code_handle() == NULL, "must be reset");
-
-  // By convention, the waiter is responsible for recycling a
-  // blocking CompileTask. Since there is only one waiter ever
-  // waiting on a CompileTask, we know that no one else will
-  // be using this CompileTask; we can free it.
-  CompileTask::free(task);
 }
 
 /**
--- a/hotspot/src/share/vm/compiler/compileTask.cpp	Wed Nov 18 03:03:43 2015 +0300
+++ b/hotspot/src/share/vm/compiler/compileTask.cpp	Wed Nov 18 09:43:31 2015 -1000
@@ -90,6 +90,7 @@
   _method_holder = JNIHandles::make_global(method->method_holder()->klass_holder());
   _osr_bci = osr_bci;
   _is_blocking = is_blocking;
+  JVMCI_ONLY(_has_waiter = CompileBroker::compiler(comp_level)->is_jvmci();)
   _comp_level = comp_level;
   _num_inlined_bytecodes = 0;
 
--- a/hotspot/src/share/vm/compiler/compileTask.hpp	Wed Nov 18 03:03:43 2015 +0300
+++ b/hotspot/src/share/vm/compiler/compileTask.hpp	Wed Nov 18 09:43:31 2015 -1000
@@ -53,6 +53,9 @@
   bool         _is_complete;
   bool         _is_success;
   bool         _is_blocking;
+#if INCLUDE_JVMCI
+  bool         _has_waiter;
+#endif
   int          _comp_level;
   int          _num_inlined_bytecodes;
   nmethodLocker* _code_handle;  // holder of eventual result
@@ -85,6 +88,10 @@
   bool         is_complete() const               { return _is_complete; }
   bool         is_blocking() const               { return _is_blocking; }
   bool         is_success() const                { return _is_success; }
+#if INCLUDE_JVMCI
+  bool         has_waiter() const                { return _has_waiter; }
+  void         clear_waiter()                    { _has_waiter = false; }
+#endif
 
   nmethodLocker* code_handle() const             { return _code_handle; }
   void         set_code_handle(nmethodLocker* l) { _code_handle = l; }