8180325: Use ClassLoaderData::classes_do for CDS classes

Use closures and ClassLoaderData::classes_do instead of SystemDictionary::classes_do

Reviewed-by: iklam, jiangli

hotspot/src/share/vm/classfile/classLoaderData.cpp

@@ -270,9 +270,8 @@ void ClassLoaderData::methods_do(void f(Method*)) {
 }
 
 void ClassLoaderData::loaded_classes_do(KlassClosure* klass_closure) {
-  // Lock to avoid classes being modified/added/removed during iteration
+  // Lock-free access requires load_ptr_acquire
-  MutexLockerEx ml(metaspace_lock(),  Mutex::_no_safepoint_check_flag);
+  for (Klass* k = load_ptr_acquire(&_klasses); k != NULL; k = k->next_link()) {
-  for (Klass* k = _klasses; k != NULL; k = k->next_link()) {
     // Do not filter ArrayKlass oops here...
     if (k->is_array_klass() || (k->is_instance_klass() && InstanceKlass::cast(k)->is_loaded())) {
       klass_closure->do_klass(k);

hotspot/src/share/vm/memory/metaspaceShared.cpp

@@ -61,8 +61,6 @@ ReservedSpace* MetaspaceShared::_shared_rs = NULL;
 
 MetaspaceSharedStats MetaspaceShared::_stats;
 
-bool MetaspaceShared::_link_classes_made_progress;
-bool MetaspaceShared::_check_classes_made_progress;
 bool MetaspaceShared::_has_error_classes;
 bool MetaspaceShared::_archive_loading_failed = false;
 bool MetaspaceShared::_remapped_readwrite = false;
@@ -174,18 +172,11 @@ address MetaspaceShared::cds_i2i_entry_code_buffers(size_t total_size) {
 // Global object for holding classes that have been loaded.  Since this
 // is run at a safepoint just before exit, this is the entire set of classes.
 static GrowableArray<Klass*>* _global_klass_objects;
-static void collect_classes(Klass* k) {
+class CollectClassesClosure : public KlassClosure {
-  _global_klass_objects->append_if_missing(k);
+  void do_klass(Klass* k) {
-  if (k->is_instance_klass()) {
+    _global_klass_objects->append_if_missing(k);
-    // Add in the array classes too
-    InstanceKlass* ik = InstanceKlass::cast(k);
-    ik->array_klasses_do(collect_classes);
   }
-}
+};
-
-static void collect_classes2(Klass* k, ClassLoaderData* class_data) {
-  collect_classes(k);
-}
 
 static void remove_unshareable_in_classes() {
   for (int i = 0; i < _global_klass_objects->length(); i++) {
@@ -731,12 +722,8 @@ void VM_PopulateDumpSharedSpace::doit() {
   // Gather systemDictionary classes in a global array and do everything to
   // that so we don't have to walk the SystemDictionary again.
   _global_klass_objects = new GrowableArray<Klass*>(1000);
-  Universe::basic_type_classes_do(collect_classes);
+  CollectClassesClosure collect_classes;
-
+  ClassLoaderDataGraph::loaded_classes_do(&collect_classes);
-  // Need to call SystemDictionary::classes_do(void f(Klass*, ClassLoaderData*))
-  // as we may have some classes with NULL ClassLoaderData* in the dictionary. Other
-  // variants of SystemDictionary::classes_do will skip those classes.
-  SystemDictionary::classes_do(collect_classes2);
 
   tty->print_cr("Number of classes %d", _global_klass_objects->length());
   {
@@ -920,23 +907,40 @@ void VM_PopulateDumpSharedSpace::doit() {
 #undef fmt_space
 }
 
+class LinkSharedClassesClosure : public KlassClosure {
+  Thread* THREAD;
+  bool    _made_progress;
+ public:
+  LinkSharedClassesClosure(Thread* thread) : THREAD(thread), _made_progress(false) {}
 
-void MetaspaceShared::link_one_shared_class(Klass* k, TRAPS) {
+  void reset()               { _made_progress = false; }
-  if (k->is_instance_klass()) {
+  bool made_progress() const { return _made_progress; }
-    InstanceKlass* ik = InstanceKlass::cast(k);
-    // Link the class to cause the bytecodes to be rewritten and the
-    // cpcache to be created. Class verification is done according
-    // to -Xverify setting.
-    _link_classes_made_progress |= try_link_class(ik, THREAD);
-    guarantee(!HAS_PENDING_EXCEPTION, "exception in link_class");
-  }
-}
 
-void MetaspaceShared::check_one_shared_class(Klass* k) {
+  void do_klass(Klass* k) {
-  if (k->is_instance_klass() && InstanceKlass::cast(k)->check_sharing_error_state()) {
+    if (k->is_instance_klass()) {
-    _check_classes_made_progress = true;
+      InstanceKlass* ik = InstanceKlass::cast(k);
+      // Link the class to cause the bytecodes to be rewritten and the
+      // cpcache to be created. Class verification is done according
+      // to -Xverify setting.
+      _made_progress |= MetaspaceShared::try_link_class(ik, THREAD);
+      guarantee(!HAS_PENDING_EXCEPTION, "exception in link_class");
+    }
   }
-}
+};
+
+class CheckSharedClassesClosure : public KlassClosure {
+  bool    _made_progress;
+ public:
+  CheckSharedClassesClosure() : _made_progress(false) {}
+
+  void reset()               { _made_progress = false; }
+  bool made_progress() const { return _made_progress; }
+  void do_klass(Klass* k) {
+    if (k->is_instance_klass() && InstanceKlass::cast(k)->check_sharing_error_state()) {
+      _made_progress = true;
+    }
+  }
+};
 
 void MetaspaceShared::check_shared_class_loader_type(Klass* k) {
   if (k->is_instance_klass()) {
@@ -951,21 +955,23 @@ void MetaspaceShared::check_shared_class_loader_type(Klass* k) {
 void MetaspaceShared::link_and_cleanup_shared_classes(TRAPS) {
   // We need to iterate because verification may cause additional classes
   // to be loaded.
+  LinkSharedClassesClosure link_closure(THREAD);
   do {
-    _link_classes_made_progress = false;
+    link_closure.reset();
-    SystemDictionary::classes_do(link_one_shared_class, THREAD);
+    ClassLoaderDataGraph::loaded_classes_do(&link_closure);
     guarantee(!HAS_PENDING_EXCEPTION, "exception in link_class");
-  } while (_link_classes_made_progress);
+  } while (link_closure.made_progress());
 
   if (_has_error_classes) {
     // Mark all classes whose super class or interfaces failed verification.
+    CheckSharedClassesClosure check_closure;
     do {
       // Not completely sure if we need to do this iteratively. Anyway,
       // we should come here only if there are unverifiable classes, which
       // shouldn't happen in normal cases. So better safe than sorry.
-      _check_classes_made_progress = false;
+      check_closure.reset();
-      SystemDictionary::classes_do(check_one_shared_class);
+      ClassLoaderDataGraph::loaded_classes_do(&check_closure);
-    } while (_check_classes_made_progress);
+    } while (check_closure.made_progress());
 
     if (IgnoreUnverifiableClassesDuringDump) {
       // This is useful when running JCK or SQE tests. You should not
@@ -1074,6 +1080,11 @@ void MetaspaceShared::preload_and_dump(TRAPS) {
 
     VMThread::execute(&op);
   }
+
+  if (PrintSystemDictionaryAtExit) {
+    SystemDictionary::print();
+  }
+
   // Since various initialization steps have been undone by this process,
   // it is not reasonable to continue running a java process.
   exit(0);

hotspot/src/share/vm/memory/metaspaceShared.hpp

@@ -110,8 +110,6 @@ class MetaspaceShared : AllStatic {
   static ReservedSpace* _shared_rs;
   static int _max_alignment;
   static MetaspaceSharedStats _stats;
-  static bool _link_classes_made_progress;
-  static bool _check_classes_made_progress;
   static bool _has_error_classes;
   static bool _archive_loading_failed;
   static bool _remapped_readwrite;
@@ -202,10 +200,8 @@ class MetaspaceShared : AllStatic {
   static void print_shared_spaces();
 
   static bool try_link_class(InstanceKlass* ik, TRAPS);
-  static void link_one_shared_class(Klass* obj, TRAPS);
-  static void check_one_shared_class(Klass* obj);
-  static void check_shared_class_loader_type(Klass* obj);
   static void link_and_cleanup_shared_classes(TRAPS);
+  static void check_shared_class_loader_type(Klass* obj);
 
   static int count_class(const char* classlist_file);
   static void estimate_regions_size() NOT_CDS_RETURN;

hotspot/src/share/vm/oops/instanceKlass.cpp

@@ -1995,13 +1995,18 @@ void InstanceKlass::store_fingerprint(uint64_t fingerprint) {
   }
 }
 
-static void remove_unshareable_in_class(Klass* k) {
-  // remove klass's unshareable info
-  k->remove_unshareable_info();
-}
-
 void InstanceKlass::remove_unshareable_info() {
   Klass::remove_unshareable_info();
+
+  if (is_in_error_state()) {
+    // Classes are attempted to link during dumping and may fail,
+    // but these classes are still in the dictionary and class list in CLD.
+    // Check in_error state first because in_error is > linked state, so
+    // is_linked() is true.
+    // If there's a linking error, there is nothing else to remove.
+    return;
+  }
+
   // Unlink the class
   if (is_linked()) {
     unlink_class();
@@ -2016,9 +2021,6 @@ void InstanceKlass::remove_unshareable_info() {
     Method* m = methods()->at(i);
     m->remove_unshareable_info();
   }
-
-  // do array classes also.
-  array_klasses_do(remove_unshareable_in_class);
 }
 
 static void restore_unshareable_in_class(Klass* k, TRAPS) {