8357473: Compilation spike leaves many CompileTasks in free list

Reviewed-by: kvn, chagedorn

Author: shipilev

src/hotspot/share/compiler/compileBroker.cpp

@@ -236,7 +236,7 @@ CompileTaskWrapper::~CompileTaskWrapper() {
 #if INCLUDE_JVMCI
       if (CompileBroker::compiler(task->comp_level())->is_jvmci()) {
         if (!task->has_waiter()) {
-          // The waiting thread timed out and thus did not free the task.
+          // The waiting thread timed out and thus did not delete the task.
           free_task = true;
         }
         task->set_blocking_jvmci_compile_state(nullptr);
@@ -249,15 +249,15 @@ CompileTaskWrapper::~CompileTaskWrapper() {
       }
     }
     if (free_task) {
-      // The task can only be freed once the task lock is released.
-      CompileTask::free(task);
+      // The task can only be deleted once the task lock is released.
+      delete task;
     }
   } else {
     task->mark_complete();
 
-    // By convention, the compiling thread is responsible for
-    // recycling a non-blocking CompileTask.
-    CompileTask::free(task);
+    // By convention, the compiling thread is responsible for deleting
+    // a non-blocking CompileTask.
+    delete task;
   }
 }
 
@@ -360,12 +360,12 @@ void CompileQueue::add(CompileTask* task) {
 }
 
 /**
- * Empties compilation queue by putting all compilation tasks onto
- * a freelist. Furthermore, the method wakes up all threads that are
- * waiting on a compilation task to finish. This can happen if background
+ * Empties compilation queue by deleting all compilation tasks.
+ * Furthermore, the method wakes up all threads that are waiting
+ * on a compilation task to finish. This can happen if background
  * compilation is disabled.
  */
-void CompileQueue::free_all() {
+void CompileQueue::delete_all() {
   MutexLocker mu(MethodCompileQueue_lock);
   CompileTask* next = _first;
 
@@ -385,11 +385,10 @@ void CompileQueue::free_all() {
       }
     }
     if (!found_waiter) {
-      // If no one was waiting for this task, we need to free it ourselves. In this case, the task
-      // is also certainly unlocked, because, again, there is no waiter.
-      // Otherwise, by convention, it's the waiters responsibility to free the task.
-      // Put the task back on the freelist.
-      CompileTask::free(current);
+      // If no one was waiting for this task, we need to delete it ourselves.
+      // In this case, the task is also certainly unlocked, because, again, there is no waiter.
+      // Otherwise, by convention, it's the waiters responsibility to delete the task.
+      delete current;
     }
   }
   _first = nullptr;
@@ -1627,10 +1626,8 @@ CompileTask* CompileBroker::create_compile_task(CompileQueue*       queue,
                                                 int                 hot_count,
                                                 CompileTask::CompileReason compile_reason,
                                                 bool                blocking) {
-  CompileTask* new_task = CompileTask::allocate();
-  new_task->initialize(compile_id, method, osr_bci, comp_level,
-                       hot_count, compile_reason,
-                       blocking);
+  CompileTask* new_task = new CompileTask(compile_id, method, osr_bci, comp_level,
+                                          hot_count, compile_reason, blocking);
   queue->add(new_task);
   return new_task;
 }
@@ -1651,7 +1648,7 @@ static const int JVMCI_COMPILATION_PROGRESS_WAIT_ATTEMPTS = 10;
  * JVMCI_COMPILATION_PROGRESS_WAIT_TIMESLICE *
  * JVMCI_COMPILATION_PROGRESS_WAIT_ATTEMPTS.
  *
- * @return true if this thread needs to free/recycle the task
+ * @return true if this thread needs to delete the task
  */
 bool CompileBroker::wait_for_jvmci_completion(JVMCICompiler* jvmci, CompileTask* task, JavaThread* thread) {
   assert(UseJVMCICompiler, "sanity");
@@ -1734,19 +1731,19 @@ void CompileBroker::wait_for_completion(CompileTask* task) {
 
   if (free_task) {
     if (is_compilation_disabled_forever()) {
-      CompileTask::free(task);
+      delete task;
       return;
     }
 
     // It is harmless to check this status without the lock, because
-    // completion is a stable property (until the task object is recycled).
+    // completion is a stable property (until the task object is deleted).
     assert(task->is_complete(), "Compilation should have completed");
 
-    // By convention, the waiter is responsible for recycling a
+    // By convention, the waiter is responsible for deleting 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);
+    // be using this CompileTask; we can delete it.
+    delete task;
   }
 }
 
@@ -1827,11 +1824,11 @@ void CompileBroker::shutdown_compiler_runtime(AbstractCompiler* comp, CompilerTh
 
     // Delete all queued compilation tasks to make compiler threads exit faster.
     if (_c1_compile_queue != nullptr) {
-      _c1_compile_queue->free_all();
+      _c1_compile_queue->delete_all();
     }
 
     if (_c2_compile_queue != nullptr) {
-      _c2_compile_queue->free_all();
+      _c2_compile_queue->delete_all();
     }
 
     // Set flags so that we continue execution with using interpreter only.

src/hotspot/share/compiler/compileBroker.hpp

@@ -140,7 +140,7 @@ class CompileQueue : public CHeapObj<mtCompiler> {
 
   // Redefine Classes support
   void mark_on_stack();
-  void free_all();
+  void delete_all();
   void print_tty();
   void print(outputStream* st = tty);
 

src/hotspot/share/compiler/compileTask.cpp

@@ -36,72 +36,15 @@
 #include "runtime/jniHandles.hpp"
 #include "runtime/mutexLocker.hpp"
 
-CompileTask*  CompileTask::_task_free_list = nullptr;
 int CompileTask::_active_tasks = 0;
 
-/**
- * Allocate a CompileTask, from the free list if possible.
- */
-CompileTask* CompileTask::allocate() {
-  MonitorLocker locker(CompileTaskAlloc_lock);
-  CompileTask* task = nullptr;
-
-  if (_task_free_list != nullptr) {
-    task = _task_free_list;
-    _task_free_list = task->next();
-    task->set_next(nullptr);
-  } else {
-    task = new CompileTask();
-    task->set_next(nullptr);
-    task->set_is_free(true);
-  }
-  assert(task->is_free(), "Task must be free.");
-  task->set_is_free(false);
-  _active_tasks++;
-  return task;
-}
-
-/**
-* Add a task to the free list.
-*/
-void CompileTask::free(CompileTask* task) {
-  MonitorLocker locker(CompileTaskAlloc_lock);
-  if (!task->is_free()) {
-    if ((task->_method_holder != nullptr && JNIHandles::is_weak_global_handle(task->_method_holder))) {
-      JNIHandles::destroy_weak_global(task->_method_holder);
-    } else {
-      JNIHandles::destroy_global(task->_method_holder);
-    }
-    if (task->_failure_reason_on_C_heap && task->_failure_reason != nullptr) {
-      os::free((void*) task->_failure_reason);
-    }
-    task->_failure_reason = nullptr;
-    task->_failure_reason_on_C_heap = false;
-
-    task->set_is_free(true);
-    task->set_next(_task_free_list);
-    _task_free_list = task;
-    _active_tasks--;
-    if (_active_tasks == 0) {
-      locker.notify_all();
-    }
-  }
-}
-
-void CompileTask::wait_for_no_active_tasks() {
-  MonitorLocker locker(CompileTaskAlloc_lock);
-  while (_active_tasks > 0) {
-    locker.wait();
-  }
-}
-
-void CompileTask::initialize(int compile_id,
-                             const methodHandle& method,
-                             int osr_bci,
-                             int comp_level,
-                             int hot_count,
-                             CompileTask::CompileReason compile_reason,
-                             bool is_blocking) {
+CompileTask::CompileTask(int compile_id,
+                         const methodHandle& method,
+                         int osr_bci,
+                         int comp_level,
+                         int hot_count,
+                         CompileReason compile_reason,
+                         bool is_blocking) {
   Thread* thread = Thread::current();
   _compile_id = compile_id;
   _method = method();
@@ -133,6 +76,33 @@ void CompileTask::initialize(int compile_id,
   _arena_bytes = 0;
 
   _next = nullptr;
+
+  Atomic::add(&_active_tasks, 1, memory_order_relaxed);
+}
+
+CompileTask::~CompileTask() {
+  if (_method_holder != nullptr && JNIHandles::is_weak_global_handle(_method_holder)) {
+    JNIHandles::destroy_weak_global(_method_holder);
+  } else {
+    JNIHandles::destroy_global(_method_holder);
+  }
+  if (_failure_reason_on_C_heap && _failure_reason != nullptr) {
+    os::free((void*) _failure_reason);
+    _failure_reason = nullptr;
+    _failure_reason_on_C_heap = false;
+  }
+
+  if (Atomic::sub(&_active_tasks, 1, memory_order_relaxed) == 0) {
+    MonitorLocker wait_ml(CompileTaskWait_lock);
+    wait_ml.notify_all();
+  }
+}
+
+void CompileTask::wait_for_no_active_tasks() {
+  MonitorLocker locker(CompileTaskWait_lock);
+  while (Atomic::load(&_active_tasks) > 0) {
+    locker.wait();
+  }
 }
 
 /**

src/hotspot/share/compiler/compileTask.hpp

@@ -82,7 +82,6 @@ class CompileTask : public CHeapObj<mtCompiler> {
   }
 
  private:
-  static CompileTask*  _task_free_list;
   static int           _active_tasks;
   int                  _compile_id;
   Method*              _method;
@@ -104,7 +103,6 @@ class CompileTask : public CHeapObj<mtCompiler> {
   int                  _comp_level;
   int                  _num_inlined_bytecodes;
   CompileTask*         _next, *_prev;
-  bool                 _is_free;
   // Fields used for logging why the compilation was initiated:
   jlong                _time_queued;  // time when task was enqueued
   jlong                _time_started; // time when compilation started
@@ -117,14 +115,10 @@ class CompileTask : public CHeapObj<mtCompiler> {
   size_t               _arena_bytes;  // peak size of temporary memory during compilation (e.g. node arenas)
 
  public:
-  CompileTask() : _failure_reason(nullptr), _failure_reason_on_C_heap(false) {}
-  void initialize(int compile_id, const methodHandle& method, int osr_bci, int comp_level,
-                  int hot_count,
-                  CompileTask::CompileReason compile_reason, bool is_blocking);
-
-  static CompileTask* allocate();
-  static void         free(CompileTask* task);
-  static void         wait_for_no_active_tasks();
+  CompileTask(int compile_id, const methodHandle& method, int osr_bci, int comp_level,
+              int hot_count, CompileReason compile_reason, bool is_blocking);
+  ~CompileTask();
+  static void wait_for_no_active_tasks();
 
   int          compile_id() const                { return _compile_id; }
   Method*      method() const                    { return _method; }
@@ -206,8 +200,6 @@ class CompileTask : public CHeapObj<mtCompiler> {
   void         set_next(CompileTask* next)       { _next = next; }
   CompileTask* prev() const                      { return _prev; }
   void         set_prev(CompileTask* prev)       { _prev = prev; }
-  bool         is_free() const                   { return _is_free; }
-  void         set_is_free(bool val)             { _is_free = val; }
   bool         is_unloaded() const;
 
   CompileTrainingData* training_data() const      { return _training_data; }

src/hotspot/share/runtime/mutexLocker.cpp

@@ -84,7 +84,6 @@ Monitor* CompileTaskWait_lock         = nullptr;
 Monitor* MethodCompileQueue_lock      = nullptr;
 Monitor* CompileThread_lock           = nullptr;
 Monitor* Compilation_lock             = nullptr;
-Monitor* CompileTaskAlloc_lock        = nullptr;
 Mutex*   CompileStatistics_lock       = nullptr;
 Mutex*   DirectivesStack_lock         = nullptr;
 Monitor* Terminator_lock              = nullptr;
@@ -346,7 +345,6 @@ void mutex_init() {
     MUTEX_DEFL(G1RareEvent_lock             , PaddedMutex  , Threads_lock, true);
   }
 
-  MUTEX_DEFL(CompileTaskAlloc_lock          , PaddedMonitor, MethodCompileQueue_lock);
   MUTEX_DEFL(CompileTaskWait_lock           , PaddedMonitor, MethodCompileQueue_lock);
 
 #if INCLUDE_PARALLELGC

src/hotspot/share/runtime/mutexLocker.hpp

@@ -86,7 +86,6 @@ extern Monitor* CompileThread_lock;              // a lock held by compile threa
 extern Monitor* Compilation_lock;                // a lock used to pause compilation
 extern Mutex*   TrainingData_lock;               // a lock used when accessing training records
 extern Monitor* TrainingReplayQueue_lock;        // a lock held when class are added/removed to the training replay queue
-extern Monitor* CompileTaskAlloc_lock;           // a lock held when CompileTasks are allocated
 extern Monitor* CompileTaskWait_lock;            // a lock held when CompileTasks are waited/notified
 extern Mutex*   CompileStatistics_lock;          // a lock held when updating compilation statistics
 extern Mutex*   DirectivesStack_lock;            // a lock held when mutating the dirstack and ref counting directives