FrankChen021 commented on code in PR #12481:
URL: https://github.com/apache/druid/pull/12481#discussion_r864474388
##########
core/src/main/java/org/apache/druid/java/util/metrics/JvmMonitor.java:
##########
@@ -167,118 +144,117 @@ private void emitGcMetrics(ServiceEmitter emitter)
}
}
- @Nullable
- private GcCounters tryCreateGcCounters()
- {
- try {
- return new GcCounters();
- }
- 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)
- {
- final IntSet retVal = new IntRBTreeSet();
-
- for (final String counterName : jStatCounterNames) {
- final Matcher m = PATTERN_GC_GENERATION.matcher(counterName);
- if (m.matches()) {
- retVal.add(Integer.parseInt(m.group(1)));
- }
- }
-
- return retVal;
- }
-
- @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<>();
+ private final List<GcCollectors> generations = new ArrayList<>();
GcCounters()
{
- // 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)));
+ List<GarbageCollectorMXBean> collectorMxBeans =
ManagementFactory.getGarbageCollectorMXBeans();
+ for (GarbageCollectorMXBean collectorMxBean : collectorMxBeans) {
+ generations.add(new GcCollectors(collectorMxBean));
}
+
}
void emit(ServiceEmitter emitter, Map<String, String[]> dimensions)
{
- for (GcGeneration generation : generations) {
+ for (GcCollectors generation : generations) {
generation.emit(emitter, dimensions);
}
}
}
- private class GcGeneration
+ private class GcCollectors
{
- 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 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";
+
+ GcCollectors(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));
+ List<MemoryPoolMXBean> memoryPoolMxBeans =
ManagementFactory.getMemoryPoolMXBeans();
Review Comment:
Putting the gc generation collector here will cause metrics for the same
generation be emitted for more than 1 times.
For example, If CMS is used, there will be two `GcCollectors` instances, one
for the young, one for the old. Obviously, Each `GcCollectors` will hold same
`GcGenerationSpace` by the code here.
I think we need to separate the space collector from the current
`GcCollectors`. My suggestion is:
1. GcCollector, a collector that collects the gc count and time metrics
2. GcSpaceCollector, a collector that collects gc memory space
3. GcCollectors, a facade class that holds collectors for gc count metrics
and space metrics collector
--
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]
