FrankChen021 commented on code in PR #12481:
URL: https://github.com/apache/druid/pull/12481#discussion_r908592680
##########
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
{
- private final String name;
- @Nullable
+ 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)
+ 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 CMS_COLLECTOR_NAME = "cms";
+ private static final String G1_COLLECTOR_NAME = "g1";
+ private static final String PARALLEL_COLLECTOR_NAME = "parallel";
+ private static final String SERIAL_COLLECTOR_NAME = "serial";
+ private static final String ZGC_COLLECTOR_NAME = "zgc";
+ private static final String SHENANDOAN_COLLECTOR_NAME = "shenandoah";
+
+ GcCollector(GarbageCollectorMXBean gcBean)
{
- this.name = StringUtils.toLowerCase(name);
+ Pair<String, String> gcNamePair = getReadableName(gcBean.getName());
+ this.generation = gcNamePair.lhs;
+ this.collectorName = gcNamePair.rhs;
- final String spacesCountKey =
StringUtils.format("sun.gc.generation.%d.spaces", genIndex);
+ collector = new GcGenerationCollector(gcBean);
+ }
- if (jStatCounters.containsKey(spacesCountKey)) {
- final long spacesCount = ((JStatData.LongCounter)
jStatCounters.get(spacesCountKey)).getLong();
- for (long spaceIndex = 0; spaceIndex < spacesCount; spaceIndex++) {
- spaces.add(new GcGenerationSpace(jStatCounters, genIndex,
spaceIndex));
- }
- }
+ private Pair<String, String> getReadableName(String name)
+ {
+ switch (name) {
+ //CMS
+ case "ParNew":
+ return new Pair<>(GC_YOUNG_GENERATION_NAME, CMS_COLLECTOR_NAME);
+ case "ConcurrentMarkSweep":
+ return new Pair<>(GC_OLD_GENERATION_NAME, CMS_COLLECTOR_NAME);
+
+ // G1
+ case "G1 Young Generation":
+ return new Pair<>(GC_YOUNG_GENERATION_NAME, G1_COLLECTOR_NAME);
+ case "G1 Old Generation":
+ return new Pair<>(GC_OLD_GENERATION_NAME, G1_COLLECTOR_NAME);
+
+ // Parallel
+ case "PS Scavenge":
+ return new Pair<>(GC_YOUNG_GENERATION_NAME, PARALLEL_COLLECTOR_NAME);
+ case "PS MarkSweep":
+ return new Pair<>(GC_OLD_GENERATION_NAME, PARALLEL_COLLECTOR_NAME);
+
+ // Serial
+ case "Copy":
+ return new Pair<>(GC_YOUNG_GENERATION_NAME, SERIAL_COLLECTOR_NAME);
+ case "MarkSweepCompact":
+ return new Pair<>(GC_OLD_GENERATION_NAME, SERIAL_COLLECTOR_NAME);
- if
(jStatCounters.containsKey(StringUtils.format("sun.gc.collector.%d.name",
genIndex))) {
- collector = new GcGenerationCollector(jStatCounters, genIndex);
- } else {
- collector = null;
+ //zgc
+ case "ZGC":
+ return new Pair<>(GC_ZGC_GENERATION_NAME, ZGC_COLLECTOR_NAME);
+
+ //Shenandoah
+ case "Shenandoah Cycles":
+ return new Pair<>(GC_YOUNG_GENERATION_NAME,
SHENANDOAN_COLLECTOR_NAME);
+ case "Shenandoah Pauses":
+ return new Pair<>(GC_OLD_GENERATION_NAME, SHENANDOAN_COLLECTOR_NAME);
+
+ default:
+ return new Pair<>(name, name);
}
}
void emit(ServiceEmitter emitter, Map<String, String[]> dimensions)
{
ImmutableMap.Builder<String, String[]> dimensionsCopyBuilder =
ImmutableMap
- .<String, String[]>builder()
- .putAll(dimensions)
- .put("gcGen", new String[]{name});
+ .<String, String[]>builder()
+ .putAll(dimensions)
+ .put("gcGen", new String[]{generation});
- if (collector != null) {
- dimensionsCopyBuilder.put("gcName", new String[]{collector.name});
- }
+ dimensionsCopyBuilder.put("gcName", new String[]{collectorName});
Map<String, String[]> dimensionsCopy = dimensionsCopyBuilder.build();
if (collector != null) {
collector.emit(emitter, dimensionsCopy);
}
- for (GcGenerationSpace space : spaces) {
- space.emit(emitter, dimensionsCopy);
- }
}
}
private class GcGenerationCollector
{
- private final String name;
- private final LongCounter invocationsCounter;
- private final TickCounter cpuCounter;
private long lastInvocations = 0;
private long lastCpuNanos = 0;
+ private final GarbageCollectorMXBean gcBean;
- GcGenerationCollector(Map<String, JStatData.Counter<?>> jStatCounters,
long genIndex)
+ GcGenerationCollector(GarbageCollectorMXBean gcBean)
{
- String collectorKeyPrefix = StringUtils.format("sun.gc.collector.%d",
genIndex);
-
- String nameKey = StringUtils.format("%s.name", collectorKeyPrefix);
- StringCounter nameCounter = (StringCounter) jStatCounters.get(nameKey);
- name = getReadableName(nameCounter.getString());
-
- invocationsCounter = (LongCounter)
jStatCounters.get(StringUtils.format("%s.invocations", collectorKeyPrefix));
- cpuCounter = (TickCounter)
jStatCounters.get(StringUtils.format("%s.time", collectorKeyPrefix));
+ this.gcBean = gcBean;
}
void emit(ServiceEmitter emitter, Map<String, String[]> dimensions)
{
final ServiceMetricEvent.Builder builder = builder();
MonitorUtils.addDimensionsToBuilder(builder, dimensions);
- long newInvocations = invocationsCounter.getLong();
+ long newInvocations = gcBean.getCollectionCount();
emitter.emit(builder.build("jvm/gc/count", newInvocations -
lastInvocations));
lastInvocations = newInvocations;
- long newCpuNanos = cpuCounter.getNanos();
+ long newCpuNanos = gcBean.getCollectionTime();
emitter.emit(builder.build("jvm/gc/cpu", newCpuNanos - lastCpuNanos));
lastCpuNanos = newCpuNanos;
}
+ }
- private String getReadableName(String name)
+ private class GcSpaceCollector
+ {
+
+ private final List<GcGenerationSpace> spaces = new ArrayList<>();
+
+ public GcSpaceCollector(MemoryPoolMXBean memoryPoolMxBean)
{
- switch (name) {
- // Young gen
- case "Copy":
- return "serial";
- case "PSScavenge":
- return "parallel";
- case "PCopy":
- return "cms";
- case "G1 incremental collections":
- return "g1";
- case "Shenandoah partial":
- return "shenandoah";
-
- // Old gen
- case "MCS":
- return "serial";
- case "PSParallelCompact":
- return "parallel";
- case "CMS":
- return "cms";
- case "G1 stop-the-world full collections":
- return "g1";
- case "Shenandoah full":
- return "shenandoah";
+ MemoryUsage collectionUsage = memoryPoolMxBean.getCollectionUsage();
+ if (collectionUsage != null) {
Review Comment:
I think we can get `MemoryUsage` object out of this ctor and check it if is
null before constructing this `GcSpaceCollector` object. In this way, if the
`collectionUsage` is not null, there is no `GcSpaceCollector` object creation.
--
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]
