8302455: VM.classloader_stats memory size values are wrong

Reviewed-by: coleenp, dholmes

Author: tstuefe

src/hotspot/share/classfile/classLoaderStats.cpp

@@ -84,8 +84,10 @@ void ClassLoaderStatsClosure::do_cld(ClassLoaderData* cld) {
 
   ClassLoaderMetaspace* ms = cld->metaspace_or_null();
   if (ms != nullptr) {
-    size_t used_bytes, capacity_bytes;
-    ms->calculate_jfr_stats(&used_bytes, &capacity_bytes);
+    size_t used_words, capacity_words;
+    ms->usage_numbers(&used_words, nullptr, &capacity_words);
+    size_t used_bytes = used_words * BytesPerWord;
+    size_t capacity_bytes = capacity_words * BytesPerWord;
     if(cld->has_class_mirror_holder()) {
       cls->_hidden_chunk_sz += capacity_bytes;
       cls->_hidden_block_sz += used_bytes;
@@ -98,7 +100,6 @@ void ClassLoaderStatsClosure::do_cld(ClassLoaderData* cld) {
   }
 }
 
-
 // Handles the difference in pointer width on 32 and 64 bit platforms
 #ifdef _LP64
   #define SPACE "%8s"

src/hotspot/share/memory/classLoaderMetaspace.cpp

@@ -160,24 +160,30 @@ void ClassLoaderMetaspace::verify() const {
 }
 #endif // ASSERT
 
-// This only exists for JFR and jcmd VM.classloader_stats. We may want to
-//  change this. Capacity as a stat is of questionable use since it may
-//  contain committed and uncommitted areas. For now we do this to maintain
-//  backward compatibility with JFR.
-void ClassLoaderMetaspace::calculate_jfr_stats(size_t* p_used_bytes, size_t* p_capacity_bytes) const {
-  // Implement this using the standard statistics objects.
-  size_t used_c = 0, cap_c = 0, used_nc = 0, cap_nc = 0;
-  if (non_class_space_arena() != nullptr) {
-    non_class_space_arena()->usage_numbers(&used_nc, nullptr, &cap_nc);
+// Convenience method to get the most important usage statistics.
+void ClassLoaderMetaspace::usage_numbers(Metaspace::MetadataType mdType, size_t* p_used_words,
+                                         size_t* p_committed_words, size_t* p_capacity_words) const {
+  const MetaspaceArena* arena = (mdType == Metaspace::MetadataType::ClassType) ?
+      class_space_arena() : non_class_space_arena();
+  arena->usage_numbers(p_used_words, p_committed_words, p_capacity_words);
+}
+
+// Convenience method to get total usage numbers
+void ClassLoaderMetaspace::usage_numbers(size_t* p_used_words, size_t* p_committed_words,
+                                         size_t* p_capacity_words) const {
+  size_t used_nc, comm_nc, cap_nc;
+  usage_numbers(Metaspace::MetadataType::NonClassType, &used_nc, &comm_nc, &cap_nc);
+  size_t used_c = 0, comm_c = 0, cap_c = 0;
+  if (Metaspace::using_class_space()) {
+    usage_numbers(Metaspace::MetadataType::ClassType, &used_c, &comm_c, &cap_c);
   }
-  if (class_space_arena() != nullptr) {
-    class_space_arena()->usage_numbers(&used_c, nullptr, &cap_c);
+  if (p_used_words != nullptr) {
+    (*p_used_words) = used_nc + used_c;
   }
-  if (p_used_bytes != nullptr) {
-    *p_used_bytes = used_c + used_nc;
+  if (p_committed_words != nullptr) {
+    (*p_committed_words) = comm_nc + comm_c;
   }
-  if (p_capacity_bytes != nullptr) {
-    *p_capacity_bytes = cap_c + cap_nc;
+  if (p_capacity_words != nullptr) {
+    (*p_capacity_words) = cap_nc + cap_c;
   }
 }
-

src/hotspot/share/memory/classLoaderMetaspace.hpp

@@ -99,11 +99,15 @@ class ClassLoaderMetaspace : public CHeapObj<mtClass> {
 
   DEBUG_ONLY(void verify() const;)
 
-  // This only exists for JFR and jcmd VM.classloader_stats. We may want to
-  //  change this. Capacity as a stat is of questionable use since it may
-  //  contain committed and uncommitted areas. For now we do this to maintain
-  //  backward compatibility with JFR.
-  void calculate_jfr_stats(size_t* p_used_bytes, size_t* p_capacity_bytes) const;
+  // Convenience method to get the most important usage statistics for either class
+  // or non-class space. For more detailed statistics, use add_to_statistics().
+  void usage_numbers(Metaspace::MetadataType mdType, size_t* p_used_words,
+                     size_t* p_committed_words, size_t* p_capacity_words) const;
+
+  // Convenience method to get the most important usage statistics (totals; both class- and non-class spaces)
+  // For more detailed statistics, use add_to_statistics().
+  void usage_numbers(size_t* p_used_words, size_t* p_committed_words,
+                     size_t* p_capacity_words) const;
 
 }; // end: ClassLoaderMetaspace
 

test/hotspot/jtreg/serviceability/dcmd/vm/ClassLoaderStatsTest.java

@@ -32,6 +32,17 @@
  * @run testng/othervm --add-exports=java.base/jdk.internal.misc=ALL-UNNAMED --add-exports=jdk.internal.jvmstat/sun.jvmstat.monitor=ALL-UNNAMED ClassLoaderStatsTest
  */
 
+/*
+ * @test
+ * @summary Test of diagnostic command VM.classloader_stats (-UseCCP)
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ *          java.compiler
+ *          java.management
+ *          jdk.internal.jvmstat/sun.jvmstat.monitor
+ * @run testng/othervm -XX:-UseCompressedClassPointers --add-exports=java.base/jdk.internal.misc=ALL-UNNAMED --add-exports=jdk.internal.jvmstat/sun.jvmstat.monitor=ALL-UNNAMED ClassLoaderStatsTest
+ */
+
 import org.testng.annotations.Test;
 import org.testng.Assert;
 
@@ -80,6 +91,7 @@ public void run(CommandExecutor executor) throws ClassNotFoundException {
         }
 
         OutputAnalyzer output = executor.execute("VM.classloader_stats");
+        output.reportDiagnosticSummary();
         Iterator<String> lines = output.asLines().iterator();
         while (lines.hasNext()) {
             String line = lines.next();
@@ -91,8 +103,20 @@ public void run(CommandExecutor executor) throws ClassNotFoundException {
                     if (!m.group(1).equals("1")) {
                         Assert.fail("Should have loaded 1 class: " + line);
                     }
-                    checkPositiveInt(m.group(2));
-                    checkPositiveInt(m.group(3));
+
+                    long capacityBytes = Long.parseLong(m.group(2)); // aka "Chunksz"
+                    long usedBytes = Long.parseLong(m.group(3)); // aka "Blocksz"
+
+                    // Minimum expected sizes: initial capacity is governed by the chunk size of the first chunk, which
+                    // depends on the arena growth policy. Since this is a normal class loader, we expect as initial chunk
+                    // size at least 4k (if UseCompressedClassPointers is off).
+                    // Minimum used size is difficult to guess but should be at least 1k.
+                    // Maximum expected sizes: We just assume a reasonable maximum. We only loaded one class, so
+                    // we should not see values > 64k.
+                    long K = 1024;
+                    if (capacityBytes < (K * 4) || usedBytes < K || capacityBytes > (64 * K) || usedBytes > (64 * K)) {
+                        throw new RuntimeException("Sizes seem off. Chunksz: " + capacityBytes + ", Blocksz: " + usedBytes);
+                    }
 
                     String next = lines.next();
                     System.out.println("DummyClassLoader next: " + next);