8206423: Use locking for cleaning ResolvedMethodTable

ServiceThread is now in charge of cleaning ResolvedMethodTable entries

Reviewed-by: gziemski, dholmes, coleenp

src/hotspot/share/classfile/systemDictionary.cpp

@@ -1883,7 +1883,7 @@ bool SystemDictionary::do_unloading(GCTimer* gc_timer,
 
   if (do_cleaning) {
     GCTraceTime(Debug, gc, phases) t("ResolvedMethodTable", gc_timer);
-    ResolvedMethodTable::unlink();
+    ResolvedMethodTable::trigger_cleanup();
   }
 
   return unloading_occurred;

src/hotspot/share/gc/g1/g1CollectedHeap.cpp

@@ -83,7 +83,6 @@
 #include "oops/access.inline.hpp"
 #include "oops/compressedOops.inline.hpp"
 #include "oops/oop.inline.hpp"
-#include "prims/resolvedMethodTable.hpp"
 #include "runtime/atomic.hpp"
 #include "runtime/flags/flagSetting.hpp"
 #include "runtime/handles.inline.hpp"
@@ -3532,28 +3531,6 @@ public:
   }
 };
 
-class G1ResolvedMethodCleaningTask : public StackObj {
-  volatile int       _resolved_method_task_claimed;
-public:
-  G1ResolvedMethodCleaningTask() :
-      _resolved_method_task_claimed(0) {}
-
-  bool claim_resolved_method_task() {
-    if (_resolved_method_task_claimed) {
-      return false;
-    }
-    return Atomic::cmpxchg(1, &_resolved_method_task_claimed, 0) == 0;
-  }
-
-  // These aren't big, one thread can do it all.
-  void work() {
-    if (claim_resolved_method_task()) {
-      ResolvedMethodTable::unlink();
-    }
-  }
-};
-
-
 // To minimize the remark pause times, the tasks below are done in parallel.
 class G1ParallelCleaningTask : public AbstractGangTask {
 private:
@@ -3561,7 +3538,6 @@ private:
   G1StringCleaningTask          _string_task;
   G1CodeCacheUnloadingTask      _code_cache_task;
   G1KlassCleaningTask           _klass_cleaning_task;
-  G1ResolvedMethodCleaningTask  _resolved_method_cleaning_task;
 
 public:
   // The constructor is run in the VMThread.
@@ -3570,8 +3546,7 @@ public:
       _unloading_occurred(unloading_occurred),
       _string_task(is_alive, true, G1StringDedup::is_enabled()),
       _code_cache_task(num_workers, is_alive, unloading_occurred),
-      _klass_cleaning_task(),
+      _klass_cleaning_task() {
-      _resolved_method_cleaning_task() {
   }
 
   // The parallel work done by all worker threads.
@@ -3585,9 +3560,6 @@ public:
     // Clean the Strings.
     _string_task.work(worker_id);
 
-    // Clean unreferenced things in the ResolvedMethodTable
-    _resolved_method_cleaning_task.work();
-
     // Wait for all workers to finish the first code cache cleaning pass.
     _code_cache_task.barrier_wait(worker_id);
 

src/hotspot/share/prims/resolvedMethodTable.cpp

@@ -57,6 +57,7 @@ ResolvedMethodTable::ResolvedMethodTable()
   : Hashtable<ClassLoaderWeakHandle, mtClass>(_table_size, sizeof(ResolvedMethodEntry)) { }
 
 oop ResolvedMethodTable::lookup(int index, unsigned int hash, Method* method) {
+  assert_locked_or_safepoint(ResolvedMethodTable_lock);
   for (ResolvedMethodEntry* p = bucket(index); p != NULL; p = p->next()) {
     if (p->hash() == hash) {
 
@@ -114,6 +115,7 @@ oop ResolvedMethodTable::basic_add(Method* method, Handle rmethod_name) {
 ResolvedMethodTable* ResolvedMethodTable::_the_table = NULL;
 
 oop ResolvedMethodTable::find_method(Method* method) {
+  MutexLocker ml(ResolvedMethodTable_lock);
   oop entry = _the_table->lookup(method);
   return entry;
 }
@@ -144,9 +146,19 @@ oop ResolvedMethodTable::add_method(Handle resolved_method_name) {
 int ResolvedMethodTable::_oops_removed = 0;
 int ResolvedMethodTable::_oops_counted = 0;
 
+// There are no dead entries at start
+bool ResolvedMethodTable::_dead_entries = false;
+
+void ResolvedMethodTable::trigger_cleanup() {
+  MutexLockerEx ml(Service_lock, Mutex::_no_safepoint_check_flag);
+  _dead_entries = true;
+  Service_lock->notify_all();
+}
+
 // Serially invoke removed unused oops from the table.
-// This is done late during GC.
+// This is done by the ServiceThread after being notified on class unloading
 void ResolvedMethodTable::unlink() {
+  MutexLocker ml(ResolvedMethodTable_lock);
   _oops_removed = 0;
   _oops_counted = 0;
   for (int i = 0; i < _the_table->table_size(); ++i) {
@@ -173,10 +185,12 @@ void ResolvedMethodTable::unlink() {
   }
   log_debug(membername, table) ("ResolvedMethod entries counted %d removed %d",
                                 _oops_counted, _oops_removed);
+  _dead_entries = false;
 }
 
 #ifndef PRODUCT
 void ResolvedMethodTable::print() {
+  MutexLocker ml(ResolvedMethodTable_lock);
   for (int i = 0; i < table_size(); ++i) {
     ResolvedMethodEntry* entry = bucket(i);
     while (entry != NULL) {

src/hotspot/share/prims/resolvedMethodTable.hpp

@@ -59,6 +59,8 @@ class ResolvedMethodTable : public Hashtable<ClassLoaderWeakHandle, mtClass> {
   static int _oops_removed;
   static int _oops_counted;
 
+  static bool _dead_entries;
+
   static ResolvedMethodTable* _the_table;
 private:
   ResolvedMethodEntry* bucket(int i) {
@@ -90,6 +92,9 @@ public:
   static oop find_method(Method* method);
   static oop add_method(Handle rmethod_name);
 
+  static bool has_work() { return _dead_entries; }
+  static void trigger_cleanup();
+
 #if INCLUDE_JVMTI
   // It is called at safepoint only for RedefineClasses
   static void adjust_method_entries(bool * trace_name_printed);

src/hotspot/share/runtime/serviceThread.cpp

@@ -31,6 +31,7 @@
 #include "runtime/mutexLocker.hpp"
 #include "runtime/os.hpp"
 #include "prims/jvmtiImpl.hpp"
+#include "prims/resolvedMethodTable.hpp"
 #include "services/diagnosticArgument.hpp"
 #include "services/diagnosticFramework.hpp"
 #include "services/gcNotifier.hpp"
@@ -86,6 +87,7 @@ void ServiceThread::service_thread_entry(JavaThread* jt, TRAPS) {
     bool acs_notify = false;
     bool stringtable_work = false;
     bool symboltable_work = false;
+    bool resolved_method_table_work = false;
     JvmtiDeferredEvent jvmti_event;
     {
       // Need state transition ThreadBlockInVM so that this thread
@@ -104,7 +106,8 @@ void ServiceThread::service_thread_entry(JavaThread* jt, TRAPS) {
               !(has_gc_notification_event = GCNotifier::has_event()) &&
               !(has_dcmd_notification_event = DCmdFactory::has_pending_jmx_notification()) &&
               !(stringtable_work = StringTable::has_work()) &&
-              !(symboltable_work = SymbolTable::has_work())) {
+              !(symboltable_work = SymbolTable::has_work()) &&
+              !(resolved_method_table_work = ResolvedMethodTable::has_work())) {
         // wait until one of the sensors has pending requests, or there is a
         // pending JVMTI event or JMX GC notification to post
         Service_lock->wait(Mutex::_no_safepoint_check_flag);
@@ -132,12 +135,16 @@ void ServiceThread::service_thread_entry(JavaThread* jt, TRAPS) {
     }
 
     if(has_gc_notification_event) {
-        GCNotifier::sendNotification(CHECK);
+      GCNotifier::sendNotification(CHECK);
     }
 
     if(has_dcmd_notification_event) {
       DCmdFactory::send_notification(CHECK);
     }
+
+    if (resolved_method_table_work) {
+      ResolvedMethodTable::unlink();
+    }
   }
 }