You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/04/27 05:28:01 UTC

[GitHub] [druid] gianm commented on a diff in pull request #12481: Use MXBeans to get GC metrics #12476

gianm commented on code in PR #12481:
URL: https://github.com/apache/druid/pull/12481#discussion_r859394873


##########
core/src/main/java/org/apache/druid/java/util/metrics/JvmMonitor.java:
##########
@@ -183,64 +169,110 @@ private GcCounters tryCreateGcCounters()
    */
   private class GcCounters
   {
-    private final List<GcGeneration> generations = new ArrayList<>();
+    private final List<GcCollector> generations = new ArrayList<>();
 
     GcCounters()
     {
-      // connect to itself
-      final JStatData jStatData = JStatData.connect(pid);
-      final Map<String, JStatData.Counter<?>> jStatCounters = jStatData.getAllCounters();
-
-      generations.add(new GcGeneration(jStatCounters, 0, "young"));
-      generations.add(new GcGeneration(jStatCounters, 1, "old"));
-      // Removed in Java 8 but still actual for previous Java versions
-      if (jStatCounters.containsKey("sun.gc.generation.2.name")) {
-        generations.add(new GcGeneration(jStatCounters, 2, "perm"));
+      List<GarbageCollectorMXBean> collectorMxBeans = ManagementFactory.getGarbageCollectorMXBeans();
+      for (GarbageCollectorMXBean collectorMxBean : collectorMxBeans) {
+        generations.add(new GcCollector(collectorMxBean));
       }
+
     }
 
     void emit(ServiceEmitter emitter, Map<String, String[]> dimensions)
     {
-      for (GcGeneration generation : generations) {
+      for (GcCollector generation : generations) {
         generation.emit(emitter, dimensions);
       }
     }
   }
 
-  private class GcGeneration
+  private class GcCollector
   {
-    private final String name;
+    private final String generation;
+    private final String collectorName;
     private final GcGenerationCollector collector;
     private final List<GcGenerationSpace> spaces = new ArrayList<>();
 
-    GcGeneration(Map<String, JStatData.Counter<?>> jStatCounters, long genIndex, String name)
-    {
-      this.name = StringUtils.toLowerCase(name);
+    private static final String GC_YOUNG_GENERATION_NAME = "young";
+    private static final String GC_OLD_GENERATION_NAME = "old";
+    private static final String GC_ZGC_GENERATION_NAME = "zgc";
+
+    private static final String GC_CMS_NAME = "cms";
+    private static final String GC_G1_NAME = "g1";
+    private static final String GC_PARALLEL_NAME = "parallel";
+    private static final String GC_SERIAL_NAME = "serial";
+    private static final String GC_ZGC_NAME = "zgc";
+    private static final String GC_SHENANDOAN_NAME = "shenandoah";
 
-      long spacesCount = ((JStatData.LongCounter) jStatCounters.get(
-          StringUtils.format("sun.gc.generation.%d.spaces", genIndex)
-      )).getLong();
-      for (long spaceIndex = 0; spaceIndex < spacesCount; spaceIndex++) {
-        spaces.add(new GcGenerationSpace(jStatCounters, genIndex, spaceIndex));
+    GcCollector(GarbageCollectorMXBean gcBean)
+    {
+      Pair<String, String> gcNamePair = getReadableName(gcBean.getName());
+      this.generation = gcNamePair.lhs;
+      this.collectorName = gcNamePair.rhs;
+
+      collector = new GcGenerationCollector(gcBean);
+
+      List<MemoryPoolMXBean> memoryPoolMxBeans = ManagementFactory.getMemoryPoolMXBeans();
+      for (MemoryPoolMXBean memoryPoolMxBean : memoryPoolMxBeans) {
+        MemoryUsage collectionUsage = memoryPoolMxBean.getCollectionUsage();
+        if (collectionUsage != null) {
+          spaces.add(new GcGenerationSpace(collectionUsage, memoryPoolMxBean.getName()));
+        }
       }
+    }
+
+    private Pair<String, String> getReadableName(String name)
+    {
+      switch (name) {
+        //CMS
+        case "ParNew":
+          return new Pair<>(GC_YOUNG_GENERATION_NAME, GC_CMS_NAME);
+        case "ConcurrentMarkSweep":
+          return new Pair<>(GC_OLD_GENERATION_NAME, GC_CMS_NAME);
+
+        // G1
+        case "G1 Young Generation":
+          return new Pair<>(GC_YOUNG_GENERATION_NAME, GC_G1_NAME);
+        case "G1 Old Generation":
+          return new Pair<>(GC_OLD_GENERATION_NAME, GC_G1_NAME);
+
+        // Parallel
+        case "PS Scavenge":
+          return new Pair<>(GC_YOUNG_GENERATION_NAME, GC_PARALLEL_NAME);
+        case "PS MarkSweep":
+          return new Pair<>(GC_OLD_GENERATION_NAME, GC_PARALLEL_NAME);
+
+        // Serial
+        case "Copy":
+          return new Pair<>(GC_YOUNG_GENERATION_NAME, GC_SERIAL_NAME);
+        case "MarkSweepCompact":
+          return new Pair<>(GC_OLD_GENERATION_NAME, GC_SERIAL_NAME);
+
+        //zgc
+        case "ZGC":
+          return new Pair<>(GC_ZGC_GENERATION_NAME, GC_ZGC_NAME);
+
+        //shenanoan

Review Comment:
   Shenandoah (spelling)



##########
core/src/main/java/org/apache/druid/java/util/metrics/JvmMonitor.java:
##########
@@ -46,7 +43,6 @@ public class JvmMonitor extends FeedDefiningMonitor
   private static final Logger log = new Logger(JvmMonitor.class);
 
   private final Map<String, String[]> dimensions;
-  private final long pid;

Review Comment:
   🎉 



##########
core/src/main/java/org/apache/druid/java/util/metrics/JvmMonitor.java:
##########
@@ -183,64 +169,110 @@ private GcCounters tryCreateGcCounters()
    */
   private class GcCounters
   {
-    private final List<GcGeneration> generations = new ArrayList<>();
+    private final List<GcCollector> generations = new ArrayList<>();
 
     GcCounters()

Review Comment:
   Couple questions about the new mechanism:
   
   1. Do we still need `--add-exports java.base/jdk.internal.perf=ALL-UNNAMED` on Java 11+ after these changes?
   2. Can `new GcCounters()` still fail on some JVM versions or configurations?
   
   If either of these is "no" then we should adjust the behavior of `tryCreateGcCounters`.



##########
core/src/main/java/org/apache/druid/java/util/metrics/JvmMonitor.java:
##########
@@ -183,64 +169,110 @@ private GcCounters tryCreateGcCounters()
    */
   private class GcCounters
   {
-    private final List<GcGeneration> generations = new ArrayList<>();
+    private final List<GcCollector> generations = new ArrayList<>();

Review Comment:
   Rename as `collectors`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org