FrankChen021 commented on code in PR #12481:
URL: https://github.com/apache/druid/pull/12481#discussion_r908598571
##########
core/src/main/java/org/apache/druid/java/util/metrics/JvmMonitor.java:
##########
@@ -162,224 +143,184 @@ private void emitDirectMemMetrics(ServiceEmitter
emitter)
private void emitGcMetrics(ServiceEmitter emitter)
{
- if (gcCounters != null) {
- gcCounters.emit(emitter, dimensions);
- }
- }
-
- @Nullable
- private GcCounters tryCreateGcCounters()
- {
- try {
- return new GcCounters();
+ if (gcCollectors != null) {
+ gcCollectors.emit(emitter, dimensions);
}
- catch (RuntimeException e) {
- // in JDK11 jdk.internal.perf.Perf is not accessible, unless
- // --add-exports java.base/jdk.internal.perf=ALL-UNNAMED is set
- log.warn("Cannot initialize GC counters. If running JDK11 and above,"
- + " add --add-exports=java.base/jdk.internal.perf=ALL-UNNAMED"
- + " to the JVM arguments to enable GC counters.");
- }
- return null;
}
- @VisibleForTesting
- static IntSet getGcGenerations(final Set<String> jStatCounterNames)
+ private class GcCollectors
{
- final IntSet retVal = new IntRBTreeSet();
+ private final List<GcCollector> collectors = new ArrayList<>();
+ private final List<GcSpaceCollector> spaceCollectors = new ArrayList<>();
- for (final String counterName : jStatCounterNames) {
- final Matcher m = PATTERN_GC_GENERATION.matcher(counterName);
- if (m.matches()) {
- retVal.add(Integer.parseInt(m.group(1)));
+ GcCollectors()
+ {
+ List<GarbageCollectorMXBean> collectorMxBeans =
ManagementFactory.getGarbageCollectorMXBeans();
+ for (GarbageCollectorMXBean collectorMxBean : collectorMxBeans) {
+ collectors.add(new GcCollector(collectorMxBean));
}
- }
- return retVal;
- }
+ List<MemoryPoolMXBean> memoryPoolMXBeans =
ManagementFactory.getMemoryPoolMXBeans();
+ for (MemoryPoolMXBean memoryPoolMXBean : memoryPoolMXBeans) {
+ spaceCollectors.add(new GcSpaceCollector(memoryPoolMXBean));
+ }
- @VisibleForTesting
- static String getGcGenerationName(final int genIndex)
- {
- switch (genIndex) {
- case 0:
- return "young";
- case 1:
- return "old";
- case 2:
- // Removed in Java 8 but still actual for previous Java versions
- return "perm";
- default:
- return String.valueOf(genIndex);
}
- }
-
- /**
- * The following GC-related code is partially based on
- *
https://github.com/aragozin/jvm-tools/blob/e0e37692648951440aa1a4ea5046261cb360df70/
- *
sjk-core/src/main/java/org/gridkit/jvmtool/PerfCounterGcCpuUsageMonitor.java
- */
- private class GcCounters
- {
- private final List<GcGeneration> generations = new ArrayList<>();
- GcCounters()
+ void emit(ServiceEmitter emitter, Map<String, String[]> dimensions)
{
- // connect to itself
- final JStatData jStatData = JStatData.connect(pid);
- final Map<String, JStatData.Counter<?>> jStatCounters =
jStatData.getAllCounters();
-
- for (int genIndex : getGcGenerations(jStatCounters.keySet())) {
- generations.add(new GcGeneration(jStatCounters, genIndex,
getGcGenerationName(genIndex)));
+ for (GcCollector collector : collectors) {
+ collector.emit(emitter, dimensions);
}
- }
- void emit(ServiceEmitter emitter, Map<String, String[]> dimensions)
- {
- for (GcGeneration generation : generations) {
- generation.emit(emitter, dimensions);
+ for (GcSpaceCollector spaceCollector : spaceCollectors) {
+ spaceCollector.emit(emitter, dimensions);
}
}
}
- private class GcGeneration
+ private class GcCollector
Review Comment:
This `GcCollector` and its member `GcGenerationCollector collector` is a
little bit confusing. I think we can blend the `GcGenerationCollector` into
this class, and call current `GcCollector` as `GcGenerationCollector`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
