8252043: Move inner class metaspace cleaning out of safepoint cleanup tasks

Clean up inner metaspaces from ServiceThread if cleanup is needed for concurrent GCs.

Reviewed-by: eosterlund, pchilanomate

src/hotspot/share/classfile/classLoaderDataGraph.cpp

@@ -40,6 +40,8 @@
 #include "runtime/mutex.hpp"
 #include "runtime/safepoint.hpp"
 #include "runtime/safepointVerifiers.hpp"
+#include "runtime/vmOperations.hpp"
+#include "runtime/vmThread.hpp"
 #include "utilities/growableArray.hpp"
 #include "utilities/macros.hpp"
 #include "utilities/ostream.hpp"
@@ -165,6 +167,13 @@ void ClassLoaderDataGraph::clean_deallocate_lists(bool walk_previous_versions) {
                                  loaders_processed, walk_previous_versions ? "walk_previous_versions" : "");
 }
 
+void ClassLoaderDataGraph::safepoint_and_clean_metaspaces() {
+  // Safepoint and mark all metadata with MetadataOnStackMark and then deallocate unused bits of metaspace.
+  // This needs to be exclusive to Redefinition, so needs to be a safepoint.
+  VM_CleanClassLoaderDataMetaspaces op;
+  VMThread::execute(&op);
+}
+
 void ClassLoaderDataGraph::walk_metadata_and_clean_metaspaces() {
   assert(SafepointSynchronize::is_at_safepoint(), "must only be called at safepoint");
 
@@ -497,9 +506,6 @@ bool ClassLoaderDataGraph::is_valid(ClassLoaderData* loader_data) {
 bool ClassLoaderDataGraph::do_unloading() {
   assert_locked_or_safepoint(ClassLoaderDataGraph_lock);
 
-  // Indicate whether safepoint cleanup is needed.
-  _safepoint_cleanup_needed = true;
-
   ClassLoaderData* data = _head;
   ClassLoaderData* prev = NULL;
   bool seen_dead_loader = false;
@@ -560,7 +566,7 @@ void ClassLoaderDataGraph::clean_module_and_package_info() {
   }
 }
 
-void ClassLoaderDataGraph::purge() {
+void ClassLoaderDataGraph::purge(bool at_safepoint) {
   ClassLoaderData* list = _unloading;
   _unloading = NULL;
   ClassLoaderData* next = list;
@@ -576,6 +582,20 @@ void ClassLoaderDataGraph::purge() {
     set_metaspace_oom(false);
   }
   DependencyContext::purge_dependency_contexts();
+
+  // If we're purging metadata at a safepoint, clean remaining
+  // metaspaces if we need to.
+  if (at_safepoint) {
+    if (_should_clean_deallocate_lists || InstanceKlass::has_previous_versions()) {
+      walk_metadata_and_clean_metaspaces();
+    }
+  } else {
+    // Tell service thread this is a good time to check to see if we should
+    // clean loaded CLDGs. This causes another safepoint.
+    MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
+    _safepoint_cleanup_needed = true;
+    Service_lock->notify_all();
+  }
 }
 
 int ClassLoaderDataGraph::resize_dictionaries() {

src/hotspot/share/classfile/classLoaderDataGraph.hpp

@@ -62,7 +62,7 @@ class ClassLoaderDataGraph : public AllStatic {
   static ClassLoaderData* find_or_create(Handle class_loader);
   static ClassLoaderData* add(Handle class_loader, bool has_class_mirror_holder);
   static void clean_module_and_package_info();
-  static void purge();
+  static void purge(bool at_safepoint);
   static void clear_claimed_marks();
   static void clear_claimed_marks(int claim);
   // Iteration through CLDG inside a safepoint; GC support
@@ -89,10 +89,12 @@ class ClassLoaderDataGraph : public AllStatic {
   static void classes_unloading_do(void f(Klass* const));
   static bool do_unloading();
 
-  // Expose state to avoid logging overhead in safepoint cleanup tasks.
   static inline bool should_clean_metaspaces_and_reset();
   static void set_should_clean_deallocate_lists() { _should_clean_deallocate_lists = true; }
   static void clean_deallocate_lists(bool purge_previous_versions);
+  // Called from ServiceThread
+  static void safepoint_and_clean_metaspaces();
+  // Called from VMOperation
   static void walk_metadata_and_clean_metaspaces();
 
   // dictionary do

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

@@ -1032,7 +1032,7 @@ void G1CollectedHeap::prepare_heap_for_mutators() {
   hrm()->prepare_for_full_collection_end();
 
   // Delete metaspaces for unloaded class loaders and clean up loader_data graph
-  ClassLoaderDataGraph::purge();
+  ClassLoaderDataGraph::purge(/*at_safepoint*/true);
   MetaspaceUtils::verify_metrics();
 
   // Prepare heap for normal collections.

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

@@ -1143,7 +1143,7 @@ void G1ConcurrentMark::remark() {
     // Clean out dead classes
     if (ClassUnloadingWithConcurrentMark) {
       GCTraceTime(Debug, gc, phases) debug("Purge Metaspace", _gc_timer_cm);
-      ClassLoaderDataGraph::purge();
+      ClassLoaderDataGraph::purge(/*at_safepoint*/true);
     }
 
     _g1h->resize_heap_if_necessary();

src/hotspot/share/gc/parallel/psParallelCompact.cpp

@@ -1056,7 +1056,7 @@ void PSParallelCompact::post_compact()
   }
 
   // Delete metaspaces for unloaded class loaders and clean up loader_data graph
-  ClassLoaderDataGraph::purge();
+  ClassLoaderDataGraph::purge(/*at_safepoint*/true);
   MetaspaceUtils::verify_metrics();
 
   heap->prune_scavengable_nmethods();

src/hotspot/share/gc/shared/genCollectedHeap.cpp

@@ -661,7 +661,7 @@ void GenCollectedHeap::do_collection(bool           full,
     _young_gen->compute_new_size();
 
     // Delete metaspaces for unloaded class loaders and clean up loader_data graph
-    ClassLoaderDataGraph::purge();
+    ClassLoaderDataGraph::purge(/*at_safepoint*/true);
     MetaspaceUtils::verify_metrics();
     // Resize the metaspace capacity after full collections
     MetaspaceGC::compute_new_size();

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp

@@ -2206,7 +2206,7 @@ void ShenandoahHeap::stw_unload_classes(bool full_gc) {
     ShenandoahGCPhase phase(full_gc ?
                             ShenandoahPhaseTimings::full_gc_purge_cldg :
                             ShenandoahPhaseTimings::purge_cldg);
-    ClassLoaderDataGraph::purge();
+    ClassLoaderDataGraph::purge(/*at_safepoint*/true);
   }
   // Resize and verify metaspace
   MetaspaceGC::compute_new_size();

src/hotspot/share/gc/shenandoah/shenandoahUnload.cpp

@@ -185,7 +185,7 @@ void ShenandoahUnload::unload() {
 
     {
       ShenandoahTimingsTracker t(ShenandoahPhaseTimings::conc_class_unload_purge_cldg);
-      ClassLoaderDataGraph::purge();
+      ClassLoaderDataGraph::purge(/*at_safepoint*/false);
     }
 
     {

src/hotspot/share/gc/z/zUnload.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -157,7 +157,7 @@ void ZUnload::purge() {
     ZNMethod::purge(_workers);
   }
 
-  ClassLoaderDataGraph::purge();
+  ClassLoaderDataGraph::purge(/*at_safepoint*/false);
   CodeCache::purge_exception_caches();
 }
 

src/hotspot/share/oops/instanceKlass.cpp

@@ -3956,8 +3956,9 @@ void InstanceKlass::purge_previous_version_list() {
       InstanceKlass* next = pv_node->previous_versions();
       pv_node->link_previous_versions(NULL);   // point next to NULL
       last->link_previous_versions(next);
-      // Add to the deallocate list after unlinking
-      loader_data->add_to_deallocate_list(pv_node);
+      // Delete this node directly. Nothing is referring to it and we don't
+      // want it to increase the counter for metadata to delete in CLDG.
+      MetadataFactory::free_metadata(loader_data, pv_node);
       pv_node = next;
       deleted_count++;
       version++;

src/hotspot/share/runtime/safepoint.cpp

@@ -23,7 +23,7 @@
  */
 
 #include "precompiled.hpp"
-#include "classfile/classLoaderDataGraph.inline.hpp"
+#include "classfile/classLoaderDataGraph.hpp"
 #include "classfile/dictionary.hpp"
 #include "classfile/stringTable.hpp"
 #include "classfile/symbolTable.hpp"
@@ -599,15 +599,6 @@ void SafepointSynchronize::do_cleanup_tasks() {
     cleanup.work(0);
   }
 
-  // Needs to be done single threaded by the VMThread.  This walks
-  // the thread stacks looking for references to metadata before
-  // deciding to remove it from the metaspaces.
-  if (ClassLoaderDataGraph::should_clean_metaspaces_and_reset()) {
-    const char* name = "cleanup live ClassLoaderData metaspaces";
-    TraceTime timer(name, TRACETIME_LOG(Info, safepoint, cleanup));
-    ClassLoaderDataGraph::walk_metadata_and_clean_metaspaces();
-  }
-
   assert(InlineCacheBuffer::is_empty(), "should have cleaned up ICBuffer");
 }
 

src/hotspot/share/runtime/serviceThread.cpp

@@ -23,6 +23,7 @@
  */
 
 #include "precompiled.hpp"
+#include "classfile/classLoaderDataGraph.inline.hpp"
 #include "classfile/javaClasses.hpp"
 #include "classfile/protectionDomainCache.hpp"
 #include "classfile/stringTable.hpp"
@@ -145,6 +146,7 @@ void ServiceThread::service_thread_entry(JavaThread* jt, TRAPS) {
     bool deflate_idle_monitors = false;
     JvmtiDeferredEvent jvmti_event;
     bool oop_handles_to_release = false;
+    bool cldg_cleanup_work = false;
     {
       // Need state transition ThreadBlockInVM so that this thread
       // will be handled by safepoint correctly when this thread is
@@ -172,6 +174,7 @@ void ServiceThread::service_thread_entry(JavaThread* jt, TRAPS) {
               (protection_domain_table_work = SystemDictionary::pd_cache_table()->has_work()) |
               (oopstorage_work = OopStorage::has_cleanup_work_and_reset()) |
               (oop_handles_to_release = (_oop_handle_list != NULL)) |
+              (cldg_cleanup_work = ClassLoaderDataGraph::should_clean_metaspaces_and_reset()) |
               (deflate_idle_monitors = ObjectSynchronizer::is_async_deflation_needed())
              ) == 0) {
         // Wait until notified that there is some work to do.
@@ -237,6 +240,10 @@ void ServiceThread::service_thread_entry(JavaThread* jt, TRAPS) {
     if (oop_handles_to_release) {
       release_oop_handles();
     }
+
+    if (cldg_cleanup_work) {
+      ClassLoaderDataGraph::safepoint_and_clean_metaspaces();
+    }
   }
 }
 

src/hotspot/share/runtime/vmOperations.cpp

@@ -23,7 +23,7 @@
  */
 
 #include "precompiled.hpp"
-#include "classfile/symbolTable.hpp"
+#include "classfile/classLoaderDataGraph.hpp"
 #include "classfile/vmSymbols.hpp"
 #include "code/codeCache.hpp"
 #include "compiler/compileBroker.hpp"
@@ -40,7 +40,6 @@
 #include "runtime/deoptimization.hpp"
 #include "runtime/frame.inline.hpp"
 #include "runtime/interfaceSupport.inline.hpp"
-#include "runtime/sweeper.hpp"
 #include "runtime/synchronizer.hpp"
 #include "runtime/thread.inline.hpp"
 #include "runtime/threadSMR.inline.hpp"
@@ -94,6 +93,10 @@ void VM_ClearICs::doit() {
   }
 }
 
+void VM_CleanClassLoaderDataMetaspaces::doit() {
+  ClassLoaderDataGraph::walk_metadata_and_clean_metaspaces();
+}
+
 VM_DeoptimizeFrame::VM_DeoptimizeFrame(JavaThread* thread, intptr_t* id, int reason) {
   _thread = thread;
   _id     = id;

src/hotspot/share/runtime/vmOperations.hpp

@@ -104,6 +104,7 @@
   template(ClassLoaderHierarchyOperation)         \
   template(DumpHashtable)                         \
   template(DumpTouchedMethods)                    \
+  template(CleanClassLoaderDataMetaspaces)        \
   template(PrintCompileQueue)                     \
   template(PrintClassHierarchy)                   \
   template(ThreadSuspend)                         \
@@ -236,6 +237,13 @@ class VM_GTestExecuteAtSafepoint: public VM_Operation {
   VM_GTestExecuteAtSafepoint() {}
 };
 
+class VM_CleanClassLoaderDataMetaspaces : public VM_Operation {
+ public:
+  VM_CleanClassLoaderDataMetaspaces() {}
+  VMOp_Type type() const                         { return VMOp_CleanClassLoaderDataMetaspaces; }
+  void doit();
+};
+
 // Deopt helper that can deoptimize frames in threads other than the
 // current thread.  Only used through Deoptimization::deoptimize_frame.
 class VM_DeoptimizeFrame: public VM_Operation {