changeset 59088:d78d27b7009b

8243572: Multiple tests fail with assert(cld->klasses() != 0LL) failed: unexpected NULL for cld->klasses() Summary: Merge unsafe anonymous class stats with hidden classes, avoiding having to call cld->klasses() Reviewed-by: lfoltan, mchung, mgronlun
author hseigel
date Thu, 30 Apr 2020 13:10:09 +0000
parents 8b9c4a988ab6
children b2ee876d83c8
files src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp src/hotspot/share/classfile/classLoaderStats.cpp src/hotspot/share/classfile/classLoaderStats.hpp src/hotspot/share/jfr/metadata/metadata.xml src/hotspot/share/jfr/periodic/jfrPeriodic.cpp src/hotspot/share/memory/metaspaceTracer.cpp test/hotspot/jtreg/serviceability/dcmd/vm/ClassLoaderStatsTest.java test/jdk/jdk/jfr/event/runtime/TestClassLoaderStatsEvent.java
diffstat 8 files changed, 21 insertions(+), 116 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp	Thu Apr 30 06:15:34 2020 -0400
+++ b/src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp	Thu Apr 30 13:10:09 2020 +0000
@@ -146,9 +146,6 @@
   LoadedClassInfo* _classes;
   int _num_classes;
 
-  LoadedClassInfo* _anon_classes;
-  int _num_anon_classes;
-
   LoadedClassInfo* _hidden_classes;
   int _num_hidden_classes;
 
@@ -224,7 +221,8 @@
       if (print_classes) {
         if (_classes != NULL) {
           for (LoadedClassInfo* lci = _classes; lci; lci = lci->_next) {
-            // non-strong hidden and unsafe anonymous classes should not live in the primary CLD of their loaders.
+            // non-strong hidden and unsafe anonymous classes should not live in
+            // the primary CLD of their loaders.
             assert(lci->_cld == _cld, "must be");
 
             branchtracker.print(st);
@@ -253,33 +251,6 @@
           st->cr();
         }
 
-        if (_anon_classes != NULL) {
-          for (LoadedClassInfo* lci = _anon_classes; lci; lci = lci->_next) {
-            branchtracker.print(st);
-            if (lci == _anon_classes) { // first iteration
-              st->print("%*s ", indentation, "Unsafe Anonymous Classes:");
-            } else {
-              st->print("%*s ", indentation, "");
-            }
-            st->print("%s", lci->_klass->external_name());
-            // For unsafe anonymous classes, also print CLD if verbose. Should
-            // be a different one than the primary CLD.
-            assert(lci->_cld != _cld, "must be");
-            if (verbose) {
-              st->print("  (Loader Data: " PTR_FORMAT ")", p2i(lci->_cld));
-            }
-            st->cr();
-          }
-          branchtracker.print(st);
-          st->print("%*s ", indentation, "");
-          st->print_cr("(%u unsafe anonymous class%s)", _num_anon_classes,
-                       (_num_anon_classes == 1) ? "" : "es");
-
-          // Empty line
-          branchtracker.print(st);
-          st->cr();
-        }
-
         if (_hidden_classes != NULL) {
           for (LoadedClassInfo* lci = _hidden_classes; lci; lci = lci->_next) {
             branchtracker.print(st);
@@ -333,9 +304,8 @@
 
   LoaderTreeNode(const oop loader_oop)
     : _loader_oop(loader_oop), _cld(NULL), _child(NULL), _next(NULL),
-      _classes(NULL), _num_classes(0), _anon_classes(NULL), _num_anon_classes(0),
-      _hidden_classes(NULL), _num_hidden_classes(0),
-      _num_folded(0)
+      _classes(NULL), _num_classes(0), _hidden_classes(NULL),
+      _num_hidden_classes(0), _num_folded(0)
     {}
 
   void set_cld(const ClassLoaderData* cld) {
@@ -357,7 +327,7 @@
     LoadedClassInfo** p_list_to_add_to;
     bool is_hidden = first_class->_klass->is_hidden();
     if (has_class_mirror_holder) {
-      p_list_to_add_to = is_hidden ? &_hidden_classes : &_anon_classes;
+      p_list_to_add_to = &_hidden_classes;
     } else {
       p_list_to_add_to = &_classes;
     }
@@ -367,11 +337,7 @@
     }
     *p_list_to_add_to = first_class;
     if (has_class_mirror_holder) {
-      if (is_hidden) {
-        _num_hidden_classes += num_classes;
-      } else {
-        _num_anon_classes += num_classes;
-      }
+      _num_hidden_classes += num_classes;
     } else {
       _num_classes += num_classes;
     }
--- a/src/hotspot/share/classfile/classLoaderStats.cpp	Thu Apr 30 06:15:34 2020 -0400
+++ b/src/hotspot/share/classfile/classLoaderStats.cpp	Thu Apr 30 13:10:09 2020 +0000
@@ -74,18 +74,9 @@
   cld->classes_do(&csc);
   bool is_hidden = false;
   if(cld->has_class_mirror_holder()) {
-    // if cld has a class holder then it must be either hidden or unsafe anonymous.
-    Klass* k = cld->klasses();
-    // if it's an array class then need to see if bottom class is hidden.
-    if (k->is_array_klass()) {
-      k = ObjArrayKlass::cast(k)->bottom_klass();
-    }
-    is_hidden = k->is_hidden();
-    if (is_hidden) {
-      cls->_hidden_classes_count += csc._num_classes;
-    } else {
-      cls->_anon_classes_count += csc._num_classes;
-    }
+    // If cld has a class holder then it must be either hidden or unsafe anonymous.
+    // Either way, count it as a hidden class.
+    cls->_hidden_classes_count += csc._num_classes;
   } else {
     cls->_classes_count = csc._num_classes;
   }
@@ -94,13 +85,8 @@
   ClassLoaderMetaspace* ms = cld->metaspace_or_null();
   if (ms != NULL) {
     if(cld->has_class_mirror_holder()) {
-      if (is_hidden) {
-        cls->_hidden_chunk_sz += ms->allocated_chunks_bytes();
-        cls->_hidden_block_sz += ms->allocated_blocks_bytes();
-      } else {
-        cls->_anon_chunk_sz += ms->allocated_chunks_bytes();
-        cls->_anon_block_sz += ms->allocated_blocks_bytes();
-      }
+      cls->_hidden_chunk_sz += ms->allocated_chunks_bytes();
+      cls->_hidden_block_sz += ms->allocated_blocks_bytes();
     } else {
       cls->_chunk_sz = ms->allocated_chunks_bytes();
       cls->_block_sz = ms->allocated_blocks_bytes();
@@ -133,12 +119,6 @@
     _out->print("<boot class loader>");
   }
   _out->cr();
-  if (cls->_anon_classes_count > 0) {
-    _out->print_cr(SPACE SPACE SPACE "                                    " UINTX_FORMAT_W(6) "  " SIZE_FORMAT_W(8) "  " SIZE_FORMAT_W(8) "   + unsafe anonymous classes",
-        "", "", "",
-        cls->_anon_classes_count,
-        cls->_anon_chunk_sz, cls->_anon_block_sz);
-  }
   if (cls->_hidden_classes_count > 0) {
     _out->print_cr(SPACE SPACE SPACE "                                    " UINTX_FORMAT_W(6) "  " SIZE_FORMAT_W(8) "  " SIZE_FORMAT_W(8) "   + hidden classes",
         "", "", "",
--- a/src/hotspot/share/classfile/classLoaderStats.hpp	Thu Apr 30 06:15:34 2020 -0400
+++ b/src/hotspot/share/classfile/classLoaderStats.hpp	Thu Apr 30 13:10:09 2020 +0000
@@ -77,10 +77,6 @@
   size_t            _block_sz;
   uintx             _classes_count;
 
-  size_t            _anon_chunk_sz;
-  size_t            _anon_block_sz;
-  uintx             _anon_classes_count;
-
   size_t            _hidden_chunk_sz;
   size_t            _hidden_block_sz;
   uintx             _hidden_classes_count;
@@ -92,9 +88,6 @@
     _chunk_sz(0),
     _block_sz(0),
     _classes_count(0),
-    _anon_chunk_sz(0),
-    _anon_block_sz(0),
-    _anon_classes_count(0),
     _hidden_chunk_sz(0),
     _hidden_block_sz(0),
     _hidden_classes_count(0) {
--- a/src/hotspot/share/jfr/metadata/metadata.xml	Thu Apr 30 06:15:34 2020 -0400
+++ b/src/hotspot/share/jfr/metadata/metadata.xml	Thu Apr 30 13:10:09 2020 +0000
@@ -218,7 +218,6 @@
   <Event name="MetaspaceAllocationFailure" category="Java Virtual Machine, GC, Metaspace" label="Metaspace Allocation Failure" startTime="false"
     stackTrace="true">
     <Field type="ClassLoader" name="classLoader" label="Class Loader" />
-    <Field type="boolean" name="unsafeAnonymousClassLoader" label="Unsafe Anonymous Class Loader" />
     <Field type="boolean" name="hiddenClassLoader" label="Hidden Class Loader" />
     <Field type="ulong" contentType="bytes" name="size" label="Size" />
     <Field type="MetadataType" name="metadataType" label="Metadata Type" />
@@ -227,7 +226,6 @@
 
   <Event name="MetaspaceOOM" category="Java Virtual Machine, GC, Metaspace" label="Metaspace Out of Memory" startTime="false" stackTrace="true">
     <Field type="ClassLoader" name="classLoader" label="Class Loader" />
-    <Field type="boolean" name="unsafeAnonymousClassLoader" label="Unsafe Anonymous Class Loader" />
     <Field type="boolean" name="hiddenClassLoader" label="Hidden Class Loader" />
     <Field type="ulong" contentType="bytes" name="size" label="Size" />
     <Field type="MetadataType" name="metadataType" label="Metadata Type" />
@@ -726,11 +724,6 @@
     <Field type="long" name="classCount" label="Classes" description="Number of loaded classes" />
     <Field type="ulong" contentType="bytes" name="chunkSize" label="Total Chunk Size" description="Total size of all allocated metaspace chunks (each chunk has several blocks)" />
     <Field type="ulong" contentType="bytes" name="blockSize" label="Total Block Size" description="Total size of all allocated metaspace blocks (each chunk has several blocks)" />
-    <Field type="long" name="unsafeAnonymousClassCount" label="Unsafe Anonymous Classes" description="Number of loaded classes to support invokedynamic" />
-    <Field type="ulong" contentType="bytes" name="unsafeAnonymousChunkSize" label="Total Unsafe Anonymous Classes Chunk Size"
-      description="Total size of all allocated metaspace chunks for unsafe anonymous classes (each chunk has several blocks)" />
-    <Field type="ulong" contentType="bytes" name="unsafeAnonymousBlockSize" label="Total Unsafe Anonymous Classes Block Size"
-      description="Total size of all allocated metaspace blocks for unsafe anonymous classes (each chunk has several blocks)" />
     <Field type="long" name="hiddenClassCount" label="Hidden Classes" description="Number of hidden classes" />
     <Field type="ulong" contentType="bytes" name="hiddenChunkSize" label="Total Hidden Classes Chunk Size"
       description="Total size of all allocated metaspace chunks for hidden classes (each chunk has several blocks)" />
--- a/src/hotspot/share/jfr/periodic/jfrPeriodic.cpp	Thu Apr 30 06:15:34 2020 -0400
+++ b/src/hotspot/share/jfr/periodic/jfrPeriodic.cpp	Thu Apr 30 13:10:09 2020 +0000
@@ -480,9 +480,6 @@
     event.set_classCount(cls->_classes_count);
     event.set_chunkSize(cls->_chunk_sz);
     event.set_blockSize(cls->_block_sz);
-    event.set_unsafeAnonymousClassCount(cls->_anon_classes_count);
-    event.set_unsafeAnonymousChunkSize(cls->_anon_chunk_sz);
-    event.set_unsafeAnonymousBlockSize(cls->_anon_block_sz);
     event.set_hiddenClassCount(cls->_hidden_classes_count);
     event.set_hiddenChunkSize(cls->_hidden_chunk_sz);
     event.set_hiddenBlockSize(cls->_hidden_block_sz);
--- a/src/hotspot/share/memory/metaspaceTracer.cpp	Thu Apr 30 06:15:34 2020 -0400
+++ b/src/hotspot/share/memory/metaspaceTracer.cpp	Thu Apr 30 13:10:09 2020 +0000
@@ -62,16 +62,7 @@
   E event;
   if (event.should_commit()) {
     event.set_classLoader(cld);
-    event.set_unsafeAnonymousClassLoader(false); // initialize these
-    event.set_hiddenClassLoader(false);
-    if (cld->has_class_mirror_holder()) {
-      assert(cld->klasses() != NULL, "unexpected NULL for cld->klasses()");
-      if (cld->klasses()->is_non_strong_hidden()) {
-        event.set_hiddenClassLoader(true);
-      } else {
-        event.set_unsafeAnonymousClassLoader(true);
-      }
-    }
+    event.set_hiddenClassLoader(cld->has_class_mirror_holder());
     event.set_size(word_size * BytesPerWord);
     event.set_metadataType((u1) mdtype);
     event.set_metaspaceObjectType((u1) objtype);
--- a/test/hotspot/jtreg/serviceability/dcmd/vm/ClassLoaderStatsTest.java	Thu Apr 30 06:15:34 2020 -0400
+++ b/test/hotspot/jtreg/serviceability/dcmd/vm/ClassLoaderStatsTest.java	Thu Apr 30 13:10:09 2020 +0000
@@ -61,8 +61,7 @@
   // Expected output from VM.classloader_stats:
     // ClassLoader         Parent              CLD*               Classes   ChunkSz   BlockSz  Type
     // 0x0000000800bd3830  0x000000080037f468  0x00007f001c2ea170       1     10240      4672  ClassLoaderStatsTest$DummyClassLoader
-    //                                                                  1      2048      1080   + unsafe anonymous classes
-    //                                                                  1      2048      1088   + hidden classes
+    //                                                                  2      2048      1088   + hidden classes
     // 0x0000000000000000  0x0000000000000000  0x00007f00e852d190    1607   4628480   3931216  <boot class loader>
     //                                                                 38    124928     85856   + hidden classes
     // 0x00000008003b5508  0x0000000000000000  0x00007f001c2d4760       1      6144      4040  jdk.internal.reflect.DelegatingClassLoader
@@ -70,7 +69,7 @@
     // ...
 
     static Pattern clLine = Pattern.compile("0x\\p{XDigit}*\\s*0x\\p{XDigit}*\\s*0x\\p{XDigit}*\\s*(\\d*)\\s*(\\d*)\\s*(\\d*)\\s*(.*)");
-    static Pattern anonLine = Pattern.compile("\\s*(\\d*)\\s*(\\d*)\\s*(\\d*)\\s*.*");
+    static Pattern hiddenLine = Pattern.compile("\\s*(\\d*)\\s*(\\d*)\\s*(\\d*)\\s*.*");
 
     public static DummyClassLoader dummyloader;
 
@@ -89,7 +88,7 @@
             String line = lines.next();
             Matcher m = clLine.matcher(line);
             if (m.matches()) {
-                // verify that DummyClassLoader has loaded 1 class, 1 anonymous class, and 1 hidden class
+                // verify that DummyClassLoader has loaded 1 regular class and 2 hidden classes
                 if (m.group(4).equals("ClassLoaderStatsTest$DummyClassLoader")) {
                     System.out.println("DummyClassLoader line: " + line);
                     if (!m.group(1).equals("1")) {
@@ -100,26 +99,14 @@
 
                     String next = lines.next();
                     System.out.println("DummyClassLoader next: " + next);
-                    if (!next.contains("unsafe anonymous classes")) {
-                        Assert.fail("Should have an anonymous class");
-                    }
-                    Matcher m1 = anonLine.matcher(next);
-                    m1.matches();
-                    if (!m1.group(1).equals("1")) {
-                        Assert.fail("Should have loaded 1 anonymous class, but found : " + m1.group(1));
-                    }
-                    checkPositiveInt(m1.group(2));
-                    checkPositiveInt(m1.group(3));
-
-                    next = lines.next();
-                    System.out.println("DummyClassLoader next: " + next);
                     if (!next.contains("hidden classes")) {
                         Assert.fail("Should have a hidden class");
                     }
-                    Matcher m2 = anonLine.matcher(next);
+                    Matcher m2 = hiddenLine.matcher(next);
                     m2.matches();
-                    if (!m2.group(1).equals("1")) {
-                        Assert.fail("Should have loaded 1 hidden class, but found : " + m2.group(1));
+                    if (!m2.group(1).equals("2")) {
+                        // anonymous classes are included in the hidden classes count.
+                        Assert.fail("Should have loaded 2 hidden classes, but found : " + m2.group(1));
                     }
                     checkPositiveInt(m2.group(2));
                     checkPositiveInt(m2.group(3));
--- a/test/jdk/jdk/jfr/event/runtime/TestClassLoaderStatsEvent.java	Thu Apr 30 06:15:34 2020 -0400
+++ b/test/jdk/jdk/jfr/event/runtime/TestClassLoaderStatsEvent.java	Thu Apr 30 13:10:09 2020 +0000
@@ -82,10 +82,8 @@
                 Events.assertField(event, "classCount").equal(2L);
                 Events.assertField(event, "chunkSize").above(1L);
                 Events.assertField(event, "blockSize").above(1L);
-                Events.assertField(event, "unsafeAnonymousClassCount").equal(2L);
-                Events.assertField(event, "unsafeAnonymousChunkSize").above(0L);
-                Events.assertField(event, "unsafeAnonymousBlockSize").above(0L);
-                Events.assertField(event, "hiddenClassCount").equal(2L);
+                // Hidden classes stats include both hidden and unsafe anonymous classes.
+                Events.assertField(event, "hiddenClassCount").equal(4L);
                 Events.assertField(event, "hiddenChunkSize").above(0L);
                 Events.assertField(event, "hiddenBlockSize").above(0L);
                 isAnyFound = true;