8290731: Clean up CDS handling of LambdaForm Species classes

Reviewed-by: ccheung

Author: iklam

src/hotspot/share/cds/archiveBuilder.cpp

@@ -727,6 +727,7 @@ void ArchiveBuilder::make_klasses_shareable() {
     const char* type;
     const char* unlinked = "";
     const char* hidden = "";
+    const char* generated = "";
     Klass* k = klasses()->at(i);
     k->remove_java_mirror();
     if (k->is_objArray_klass()) {
@@ -771,13 +772,18 @@ void ArchiveBuilder::make_klasses_shareable() {
         hidden = " ** hidden";
       }
 
+      if (ik->is_generated_shared_class()) {
+        generated = " ** generated";
+      }
       MetaspaceShared::rewrite_nofast_bytecodes_and_calculate_fingerprints(Thread::current(), ik);
       ik->remove_unshareable_info();
     }
 
     if (log_is_enabled(Debug, cds, class)) {
       ResourceMark rm;
-      log_debug(cds, class)("klasses[%5d] = " PTR_FORMAT " %-5s %s%s%s", i, p2i(to_requested(k)), type, k->external_name(), hidden, unlinked);
+      log_debug(cds, class)("klasses[%5d] = " PTR_FORMAT " %-5s %s%s%s%s", i,
+                            p2i(to_requested(k)), type, k->external_name(),
+                            hidden, unlinked, generated);
     }
   }
 

src/hotspot/share/cds/lambdaFormInvokers.cpp

@@ -165,46 +165,43 @@ void LambdaFormInvokers::regenerate_holder_classes(TRAPS) {
     assert(h_bytes != NULL, "Class bytes is NULL");
 
     char *class_name = java_lang_String::as_utf8_string(h_name());
-    int len = h_bytes->length();
-    // make a copy of class bytes so GC will not affect us.
-    char *buf = NEW_RESOURCE_ARRAY(char, len);
-    memcpy(buf, (char*)h_bytes->byte_at_addr(0), len);
-    ClassFileStream st((u1*)buf, len, NULL, ClassFileStream::verify);
-    regenerate_class(class_name, st, CHECK);
-  }
-}
-
-// check if a class name is a species
-bool is_a_species(const char* species_name) {
-  log_info(cds)("Checking class %s", species_name);
-  if (strstr(species_name, "java/lang/invoke/BoundMethodHandle$Species_") != nullptr) {
-    return true;
+    if (strstr(class_name, "java/lang/invoke/BoundMethodHandle$Species_") != nullptr) {
+      // The species classes are already loaded into the system dictionary
+      // during the execution of CDS.generateLambdaFormHolderClasses(). No
+      // need to regenerate.
+      TempNewSymbol class_name_sym = SymbolTable::new_symbol(class_name);
+      Klass* klass = SystemDictionary::resolve_or_null(class_name_sym, THREAD);
+      assert(klass != NULL, "must already be loaded");
+      if (!klass->is_shared() && klass->shared_classpath_index() < 0) {
+        // Fake it, so that it will be included into the archive.
+        klass->set_shared_classpath_index(0);
+        // Set the "generated" bit, so it won't interfere with JVMTI.
+        // See SystemDictionaryShared::find_builtin_class().
+        klass->set_is_generated_shared_class();
+      }
+    } else {
+      int len = h_bytes->length();
+      // make a copy of class bytes so GC will not affect us.
+      char *buf = NEW_RESOURCE_ARRAY(char, len);
+      memcpy(buf, (char*)h_bytes->byte_at_addr(0), len);
+      ClassFileStream st((u1*)buf, len, NULL, ClassFileStream::verify);
+      regenerate_class(class_name, st, CHECK);
+    }
   }
-  return false;
 }
 
-void LambdaFormInvokers::regenerate_class(char* name, ClassFileStream& st, TRAPS) {
-  Symbol* class_name = SymbolTable::new_symbol((const char*)name);
-  // the class must exist
-  Klass* klass = SystemDictionary::resolve_or_null(class_name, THREAD);
-  if (klass == NULL) {
-    log_info(cds)("Class %s not present, skip", name);
-    return;
-  }
-  // the species is shared in base archive, skip it.
-  if (klass->is_regenerated() && is_a_species(name)) {
-    log_info(cds)("Skip regenerating for shared  %s", name);
-    return;
-  }
-
+void LambdaFormInvokers::regenerate_class(char* class_name, ClassFileStream& st, TRAPS) {
+  TempNewSymbol class_name_sym = SymbolTable::new_symbol(class_name);
+  Klass* klass = SystemDictionary::resolve_or_null(class_name_sym, THREAD);
+  assert(klass != NULL, "must exist");
   assert(klass->is_instance_klass(), "Should be");
 
   ClassLoaderData* cld = ClassLoaderData::the_null_class_loader_data();
   Handle protection_domain;
   ClassLoadInfo cl_info(protection_domain);
 
   InstanceKlass* result = KlassFactory::create_from_stream(&st,
-                                                   class_name,
+                                                   class_name_sym,
                                                    cld,
                                                    cl_info,
                                                    CHECK);
@@ -220,11 +217,11 @@ void LambdaFormInvokers::regenerate_class(char* name, ClassFileStream& st, TRAPS
   MetaspaceShared::try_link_class(THREAD, result);
   assert(!HAS_PENDING_EXCEPTION, "Invariant");
 
-  result->set_regenerated();  // mark for regenerated
+  result->set_is_generated_shared_class();
   SystemDictionaryShared::set_excluded(InstanceKlass::cast(klass)); // exclude the existing class from dump
   SystemDictionaryShared::init_dumptime_info(result);
   log_info(cds, lambda)("Regenerated class %s, old: " INTPTR_FORMAT " new: " INTPTR_FORMAT,
-                 name, p2i(klass), p2i(result));
+                 class_name, p2i(klass), p2i(result));
 }
 
 void LambdaFormInvokers::dump_static_archive_invokers() {

src/hotspot/share/classfile/systemDictionaryShared.cpp

@@ -1341,7 +1341,8 @@ SystemDictionaryShared::find_record(RunTimeSharedDictionary* static_dict, RunTim
   unsigned int hash = SystemDictionaryShared::hash_for_shared_dictionary_quick(name);
   const RunTimeClassInfo* record = NULL;
   if (DynamicArchive::is_mapped()) {
-    // Those regenerated holder classes are in dynamic archive
+    // Use the regenerated holder classes in the dynamic archive as they
+    // have more methods than those in the base archive.
     if (name == vmSymbols::java_lang_invoke_Invokers_Holder() ||
         name == vmSymbols::java_lang_invoke_DirectMethodHandle_Holder() ||
         name == vmSymbols::java_lang_invoke_LambdaForm_Holder() ||
@@ -1373,9 +1374,9 @@ InstanceKlass* SystemDictionaryShared::find_builtin_class(Symbol* name) {
   if (record != NULL) {
     assert(!record->_klass->is_hidden(), "hidden class cannot be looked up by name");
     assert(check_alignment(record->_klass), "Address not aligned");
-    // We did not save the classfile data of the regenerated LambdaForm invoker classes,
+    // We did not save the classfile data of the generated LambdaForm invoker classes,
     // so we cannot support CLFH for such classes.
-    if (record->_klass->is_regenerated() && JvmtiExport::should_post_class_file_load_hook()) {
+    if (record->_klass->is_generated_shared_class() && JvmtiExport::should_post_class_file_load_hook()) {
        return NULL;
     }
     return record->_klass;

src/hotspot/share/oops/klass.hpp

@@ -173,14 +173,16 @@ class Klass : public Metadata {
   jshort _shared_class_path_index;
 
 #if INCLUDE_CDS
-  // Flags of the current shared class.
+  // Various attributes for shared classes. Should be zero for a non-shared class.
   u2     _shared_class_flags;
-  enum {
+  enum CDSSharedClassFlags {
     _archived_lambda_proxy_is_available    = 1 << 1,
     _has_value_based_class_annotation      = 1 << 2,
     _verified_at_dump_time                 = 1 << 3,
     _has_archived_enum_objs                = 1 << 4,
-    _regenerated                           = 1 << 5
+    // This class was not loaded from a classfile in the module image
+    // or classpath.
+    _is_generated_shared_class             = 1 << 5
   };
 #endif
 
@@ -352,11 +354,11 @@ class Klass : public Metadata {
     NOT_CDS(return false;)
   }
 
-  void set_regenerated() {
-    CDS_ONLY(_shared_class_flags |= _regenerated;)
+  void set_is_generated_shared_class() {
+    CDS_ONLY(_shared_class_flags |= _is_generated_shared_class;)
   }
-  bool is_regenerated() const {
-    CDS_ONLY(return (_shared_class_flags & _regenerated) != 0;)
+  bool is_generated_shared_class() const {
+    CDS_ONLY(return (_shared_class_flags & _is_generated_shared_class) != 0;)
     NOT_CDS(return false;)
   }
 

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/CDSLambdaInvoker.java

@@ -28,9 +28,15 @@
 
 public class CDSLambdaInvoker {
     public static void main(String args[]) throws Throwable {
+        // The following calls trigger the generation of new Species classes
+        // that are not included in the base archive (or the default modules image).
+        // - java.lang.invoke.BoundMethodHandle$Species_F
+        // - java.lang.invoke.BoundMethodHandle$Species_FL
+        // - java.lang.invoke.BoundMethodHandle$Species_J
+        // - java.lang.invoke.BoundMethodHandle$Species_JL
         invoke(MethodHandles.identity(double.class), 1.0);
         invoke(MethodHandles.identity(long.class), 1);
-        invoke(MethodHandles.identity(int.class), 1);
+        invoke(MethodHandles.identity(int.class), 1); // Note: Species_IL is in default modules image.
         invoke(MethodHandles.identity(float.class), 1.0f);
 
         MethodHandles.Lookup lookup = MethodHandles.lookup();

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestLambdaInvokers.java

@@ -42,12 +42,18 @@ public class TestLambdaInvokers extends DynamicArchiveTestBase {
     private static void doTest(String topArchiveName) throws Exception {
         dump(topArchiveName,
              "-Xlog:cds",
+             "-Xlog:cds+class=debug",
              "-Xlog:cds+dynamic=debug",
              "-cp",
              jarFile,
              mainClass)
              .assertNormalExit(output -> {
-                 output.shouldContain("Skip regenerating for shared");
+                 // This should be not be generated from the dynamic dump, as it should be included
+                 // by the base archive.
+                 output.shouldNotMatch("cds,class.*=.*java.lang.invoke.BoundMethodHandle.Species_IL");
+
+                 // This should be generated from the dynamic dump
+                 output.shouldMatch("cds,class.*=.*java.lang.invoke.BoundMethodHandle.Species_JL");
              });
         run(topArchiveName,
              "-Xlog:cds",
@@ -57,7 +63,11 @@ private static void doTest(String topArchiveName) throws Exception {
              jarFile,
              mainClass)
              .assertNormalExit(output -> {
-                 // java.lang.invoke.BoundMethodHandle$Species_JL is generated from CDSLambdaInvoker
+                 // java.lang.invoke.BoundMethodHandle$Species_IL is loaded from base archive
+                 output.shouldContain("java.lang.invoke.BoundMethodHandle$Species_IL source: shared objects file");
+
+                 // java.lang.invoke.BoundMethodHandle$Species_JL is generated from CDSLambdaInvoker and
+                 // stored in the dynamic archive
                  output.shouldContain("java.lang.invoke.BoundMethodHandle$Species_JL source: shared objects file (top)");
              });
     }