add some new thread metric and class metric to JVMMetric #52

Switch-vov · 2021-07-02T06:09:18Z

Switch-vov · 2021-07-02T06:10:30Z

@wu-sheng please review it.

language-agent/JVMMetric.proto

Ax1an · 2021-07-02T09:08:55Z

language-agent/JVMMetric.proto

+    int64 newStateThreadCount = 6;
+    int64 runnableStateThreadCount = 7;
+    int64 blockedStateThreadCount = 8;
+    int64 waitingStateThreadCount = 9;
+    int64 timedWaitingStateThreadCount = 10;
+    int64 terminatedStateThreadCount = 11;


In order to get this information, we need to get all active threads regularly and make statistics. I'm not sure our agent will accept this.

My implemention.

public enum ThreadProvider { INSTANCE; private final ThreadMXBean threadMXBean; ThreadProvider() { this.threadMXBean = ManagementFactory.getThreadMXBean(); } public Thread getThreadMetrics() { int newThreadCount = 0; int runnableThreadCount = 0; int blockedThreadCount = 0; int waitThreadCount = 0; int timeWaitThreadCount = 0; int terminatedThreadCount = 0; ThreadInfo[] threadInfos = threadMXBean.getThreadInfo(threadMXBean.getAllThreadIds(), 0); if (threadInfos != null) { for (ThreadInfo threadInfo : threadInfos) { if (threadInfo != null) { switch (threadInfo.getThreadState()) { case NEW: newThreadCount++; break; case RUNNABLE: runnableThreadCount++; break; case BLOCKED: blockedThreadCount++; break; case WAITING: waitThreadCount++; break; case TIMED_WAITING: timeWaitThreadCount++; break; case TERMINATED: terminatedThreadCount++; break; default: break; } } else { terminatedThreadCount++; } } } int threadCount = threadMXBean.getThreadCount(); int daemonThreadCount = threadMXBean.getDaemonThreadCount(); int peakThreadCount = threadMXBean.getPeakThreadCount(); int deadlocked = threadMXBean.findDeadlockedThreads() != null ? threadMXBean.findDeadlockedThreads().length : 0; int monitorDeadlocked = threadMXBean.findMonitorDeadlockedThreads() != null ? threadMXBean.findMonitorDeadlockedThreads().length : 0; return Thread.newBuilder().setLiveCount(threadCount) .setDaemonCount(daemonThreadCount) .setPeakCount(peakThreadCount) .setDeadlocked(deadlocked) .setMonitorDeadlocked(monitorDeadlocked) .setNewThreadCount(newThreadCount) .setRunnableThreadCount(runnableThreadCount) .setBlockedThreadCount(blockedThreadCount) .setWaitThreadCount(waitThreadCount) .setTimeWaitThreadCount(timeWaitThreadCount) .setTerminatedThreadCount(terminatedThreadCount) .build(); } }

We may need a benchmark about this way, could you use jmh to run this? I want to see whether there is a performance impact.

Please don't resolve it. There is an comment.

ThreadProviderBenchmark

I will compare with prometheus/client_java ThreadExports.java .

ClassProvider

package org.apache.skywalking.apm.agent.core.jvm.clazz; import java.lang.management.ClassLoadingMXBean; import java.lang.management.ManagementFactory; import org.apache.skywalking.apm.network.language.agent.v3.Class; public enum ClassProvider { INSTANCE; private final ClassLoadingMXBean classLoadingMXBean; ClassProvider() { this.classLoadingMXBean = ManagementFactory.getClassLoadingMXBean(); } public Class getClassMetrics() { int loadedClassCount = classLoadingMXBean.getLoadedClassCount(); long unloadedClassCount = classLoadingMXBean.getUnloadedClassCount(); long totalLoadedClassCount = classLoadingMXBean.getTotalLoadedClassCount(); return Class.newBuilder().setLoadedClassCount(loadedClassCount) .setUnloadedClassCount(unloadedClassCount) .setTotalLoadedClassCount(totalLoadedClassCount) .build(); } }

ClassProviderBenchmark

package org.apache.skywalking.apm.agent.core.jvm.clazz; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; import org.openjdk.jmh.annotations.Mode; import org.openjdk.jmh.annotations.OutputTimeUnit; import org.openjdk.jmh.runner.Runner; import org.openjdk.jmh.runner.options.Options; import org.openjdk.jmh.runner.options.OptionsBuilder; import java.util.concurrent.TimeUnit; public class ClassProviderBenchmark { @Benchmark @Fork(value = 1, warmups = 1) @OutputTimeUnit(TimeUnit.SECONDS) @BenchmarkMode(Mode.Throughput) public void getThreadMetrics() { ClassProvider.INSTANCE.getClassMetrics(); } public static void main(String[] args) throws Exception { Options opt = new OptionsBuilder().include(ClassProviderBenchmark.class.getSimpleName()) .build(); new Runner(opt).run(); } /** * # JMH version: 1.21 * # VM version: JDK 1.8.0_231, Java HotSpot(TM) 64-Bit Server VM, 25.231-b11 * # VM invoker: /Library/Java/JavaVirtualMachines/jdk1.8.0_231.jdk/Contents/Home/jre/bin/java * # VM options: -javaagent:/Users/switch/Library/Application Support/JetBrains/Toolbox/apps/IDEA-U/ch-0/211.7442.40/IntelliJ IDEA.app/Contents/lib/idea_rt.jar=65164:/Users/switch/Library/Application Support/JetBrains/Toolbox/apps/IDEA-U/ch-0/211.7442.40/IntelliJ IDEA.app/Contents/bin -Dfile.encoding=UTF-8 * # Warmup: 5 iterations, 10 s each * # Measurement: 5 iterations, 10 s each * # Timeout: 10 min per iteration * # Threads: 1 thread, will synchronize iterations * # Benchmark mode: Throughput, ops/time * * Benchmark Mode Cnt Score Error Units * ClassProviderBenchmark.getThreadMetrics thrpt 5 6461016.100 ± 1453485.840 ops/s */ }

There are some issues in the benchmark:

the fork value=1 and warmups=1 are too small IMO.

The result of the tested method ClassProvider.INSTANCE.getClassMetrics() and ThreadProvider.INSTANCE.getThreadMetrics() are not consumed, which might be optimized into NOOP by JVM.

If this is a critical path and the benchmark result really matters, we should redo with 👆 fixed, otherwise this PR also looks good to me.

There are some issues in the benchmark:

the fork value=1 and warmups=1 are too small IMO.

The result of the tested method ClassProvider.INSTANCE.getClassMetrics() and ThreadProvider.INSTANCE.getThreadMetrics() are not consumed, which might be optimized into NOOP by JVM.

If this is a critical path and the benchmark result really matters, we should redo with 👆 fixed, otherwise this PR also looks good to me.

ok, I will test again.

There are some issues in the benchmark:

the fork value=1 and warmups=1 are too small IMO.

The result of the tested method ClassProvider.INSTANCE.getClassMetrics() and ThreadProvider.INSTANCE.getThreadMetrics() are not consumed, which might be optimized into NOOP by JVM.

If this is a critical path and the benchmark result really matters, we should redo with 👆 fixed, otherwise this PR also looks good to me.

Increase fork value to 5 and warmups to 3.
Use Blackhole consume the result.

@kezhenxu94 please review again.

ThreadProviderBenchmark

package org.apache.skywalking.apm.agent.core.jvm.thread; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; import org.openjdk.jmh.annotations.Mode; import org.openjdk.jmh.annotations.OutputTimeUnit; import org.openjdk.jmh.infra.Blackhole; import org.openjdk.jmh.runner.Runner; import org.openjdk.jmh.runner.options.Options; import org.openjdk.jmh.runner.options.OptionsBuilder; import java.util.concurrent.TimeUnit; public class ThreadProviderBenchmark { @Benchmark @Fork(value = 5, warmups = 3) @OutputTimeUnit(TimeUnit.SECONDS) @BenchmarkMode(Mode.Throughput) public void getThreadMetrics(Blackhole bh) { bh.consume(ThreadProvider.INSTANCE.getThreadMetrics()); } public static void main(String[] args) throws Exception { Options opt = new OptionsBuilder().include(ThreadProviderBenchmark.class.getSimpleName()) .build(); new Runner(opt).run(); } /** * # JMH version: 1.21 * # VM version: JDK 1.8.0_231, Java HotSpot(TM) 64-Bit Server VM, 25.231-b11 * # VM invoker: /Library/Java/JavaVirtualMachines/jdk1.8.0_231.jdk/Contents/Home/jre/bin/java * # VM options: -javaagent:/Users/switch/Library/Application Support/JetBrains/Toolbox/apps/IDEA-U/ch-0/211.7442.40/IntelliJ IDEA.app/Contents/lib/idea_rt.jar=52623:/Users/switch/Library/Application Support/JetBrains/Toolbox/apps/IDEA-U/ch-0/211.7442.40/IntelliJ IDEA.app/Contents/bin -Dfile.encoding=UTF-8 * # Warmup: 5 iterations, 10 s each * # Measurement: 5 iterations, 10 s each * # Timeout: 10 min per iteration * # Threads: 1 thread, will synchronize iterations * # Benchmark mode: Throughput, ops/time * # Benchmark: org.apache.skywalking.apm.agent.core.jvm.thread.ThreadProviderBenchmark.getThreadMetrics * * Benchmark Mode Cnt Score Error Units * ThreadProviderBenchmark.getThreadMetrics thrpt 25 247393.607 ± 2493.640 ops/s */ }

ClassProviderBenchmark

package org.apache.skywalking.apm.agent.core.jvm.clazz; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; import org.openjdk.jmh.annotations.Mode; import org.openjdk.jmh.annotations.OutputTimeUnit; import org.openjdk.jmh.infra.Blackhole; import org.openjdk.jmh.runner.Runner; import org.openjdk.jmh.runner.options.Options; import org.openjdk.jmh.runner.options.OptionsBuilder; import java.util.concurrent.TimeUnit; public class ClassProviderBenchmark { @Benchmark @Fork(value = 5, warmups = 3) @OutputTimeUnit(TimeUnit.SECONDS) @BenchmarkMode(Mode.Throughput) public void getThreadMetrics(Blackhole bh) { bh.consume(ClassProvider.INSTANCE.getClassMetrics()); } public static void main(String[] args) throws Exception { Options opt = new OptionsBuilder().include(ClassProviderBenchmark.class.getSimpleName()) .build(); new Runner(opt).run(); } /** # JMH version: 1.21 # VM version: JDK 1.8.0_231, Java HotSpot(TM) 64-Bit Server VM, 25.231-b11 # VM invoker: /Library/Java/JavaVirtualMachines/jdk1.8.0_231.jdk/Contents/Home/jre/bin/java # VM options: -javaagent:/Users/switch/Library/Application Support/JetBrains/Toolbox/apps/IDEA-U/ch-0/211.7442.40/IntelliJ IDEA.app/Contents/lib/idea_rt.jar=52931:/Users/switch/Library/Application Support/JetBrains/Toolbox/apps/IDEA-U/ch-0/211.7442.40/IntelliJ IDEA.app/Contents/bin -Dfile.encoding=UTF-8 # Warmup: 5 iterations, 10 s each # Measurement: 5 iterations, 10 s each # Timeout: 10 min per iteration # Threads: 1 thread, will synchronize iterations # Benchmark mode: Throughput, ops/time * * Benchmark Mode Cnt Score Error Units * ClassProviderBenchmark.getThreadMetrics thrpt 25 6542809.978 ± 8794.520 ops/s */ }

language-agent/JVMMetric.proto

Ax1an · 2021-07-02T11:59:51Z

language-agent/JVMMetric.proto

+    int64 deadlockedCount = 4;
+    int64 monitorDeadlockedCount = 5;


I found that this place may have relatively large performance loss.

This operation may be more suitable for manual triggering when needed.

This operation may be more suitable for manual triggering when needed.

@Ax1an So, do i delete it or add a cache?

It seems deadlock metrics should not be added to a period reporter, this impacts performance a lot.
OAP could support a dynamic configuration to create a deadlock check task and report separately through other APIs

ThreadExports.java use it.

void addThreadMetrics(List<MetricFamilySamples> sampleFamilies) { // etc... sampleFamilies.add( new GaugeMetricFamily( "jvm_threads_deadlocked", "Cycles of JVM-threads that are in deadlock waiting to acquire object monitors or ownable synchronizers", nullSafeArrayLength(threadBean.findDeadlockedThreads()))); sampleFamilies.add( new GaugeMetricFamily( "jvm_threads_deadlocked_monitor", "Cycles of JVM-threads that are in deadlock waiting to acquire object monitors", nullSafeArrayLength(threadBean.findMonitorDeadlockedThreads()))); // etc... } public List<MetricFamilySamples> collect() { List<MetricFamilySamples> mfs = new ArrayList<MetricFamilySamples>(); addThreadMetrics(mfs); return mfs; }

Benchmark

package io.prometheus.client.hotspot; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; import org.openjdk.jmh.annotations.Mode; import org.openjdk.jmh.annotations.OutputTimeUnit; import org.openjdk.jmh.profile.GCProfiler; import org.openjdk.jmh.runner.Runner; import org.openjdk.jmh.runner.options.Options; import org.openjdk.jmh.runner.options.OptionsBuilder; import java.util.concurrent.TimeUnit; public class ThreadExportsBenchmark { @Benchmark @Fork(value = 1, warmups = 1) @OutputTimeUnit(TimeUnit.SECONDS) @BenchmarkMode(Mode.Throughput) public void getThreadMetrics() { new ThreadExports().collect(); } public static void main(String[] args) throws Exception { Options opt = new OptionsBuilder().include(ThreadExportsBenchmark.class.getSimpleName()) .build(); new Runner(opt).run(); } /** * # JMH version: 1.21 * # VM version: JDK 1.8.0_231, Java HotSpot(TM) 64-Bit Server VM, 25.231-b11 * # VM invoker: /Library/Java/JavaVirtualMachines/jdk1.8.0_231.jdk/Contents/Home/jre/bin/java * # VM options: -javaagent:/Users/switch/Library/Application Support/JetBrains/Toolbox/apps/IDEA-U/ch-0/211.7442.40/IntelliJ IDEA.app/Contents/lib/idea_rt.jar=50050:/Users/switch/Library/Application Support/JetBrains/Toolbox/apps/IDEA-U/ch-0/211.7442.40/IntelliJ IDEA.app/Contents/bin -Dfile.encoding=UTF-8 * # Warmup: 5 iterations, 10 s each * # Measurement: 5 iterations, 10 s each * # Timeout: 10 min per iteration * # Threads: 1 thread, will synchronize iterations * # Benchmark mode: Throughput, ops/time * * Benchmark Mode Cnt Score Error Units * ThreadExportsBenchmark.getThreadMetrics thrpt 5 14423.128 ± 2422.329 ops/s */ }

It seems deadlock metrics should not be added to a period reporter, this impacts performance a lot.
OAP could support a dynamic configuration to create a deadlock check task and report separately through other APIs

Ok. I remove it.

Before

Benchmark Mode Cnt Score Error Units ThreadProviderBenchmark.getThreadMetrics thrpt 5 14948.924 ± 1942.711 ops/s

After

This is beachmark result after remove deadlock metrics.

Benchmark Mode Cnt Score Error Units ThreadProviderBenchmark.getThreadMetrics thrpt 5 196253.890 ± 74866.883 ops/s

Same problem with benchmark as #52 (comment)

Ax1an · 2021-07-02T13:55:12Z

language-agent/JVMMetric.proto

@@ -84,9 +85,23 @@ enum GCPhrase {
    OLD = 1;
 }

+// see: https://docs.oracle.com/javase/8/docs/api/java/lang/management/ThreadMXBean.html


Suggested change

// see: https://docs.oracle.com/javase/8/docs/api/java/lang/management/ThreadMXBean.html

// See: https://docs.oracle.com/javase/8/docs/api/java/lang/management/ThreadMXBean.html

Capitalized

Ax1an · 2021-07-02T13:55:24Z

language-agent/JVMMetric.proto

+    int64 terminatedStateThreadCount = 9;
+}
+
+// see: https://docs.oracle.com/javase/8/docs/api/java/lang/management/ClassLoadingMXBean.html


Suggested change

// see: https://docs.oracle.com/javase/8/docs/api/java/lang/management/ClassLoadingMXBean.html

// See: https://docs.oracle.com/javase/8/docs/api/java/lang/management/ClassLoadingMXBean.html

Capitalized

Ax1an

LGTM. @Switch-vov Thank you so much for your good polish, patient, and keeping testing.

wu-sheng

LGTM, let's wait for 2 days as this is a protocol level upgrade, in case we miss anything.

mrproliu

LGTM

dmsolr

LGTM

wu-sheng · 2021-07-04T05:50:55Z

@Switch-vov Please continue your works on apache/skywalking#7230

add some new thread metric and class metric to JVMMetric(#7230)

7d83a05

wu-sheng requested review from wu-sheng, mrproliu, dmsolr, kezhenxu94, zhaoyuguang and Ax1an July 2, 2021 07:12

wu-sheng added the enhancement label Jul 2, 2021

Ax1an reviewed Jul 2, 2021

View reviewed changes

language-agent/JVMMetric.proto Outdated Show resolved Hide resolved

Ax1an reviewed Jul 2, 2021

View reviewed changes

language-agent/JVMMetric.proto Outdated Show resolved Hide resolved

Switch-vov added 2 commits July 2, 2021 15:58

change the name of jvm thread metrics (#7230)

3bcd627

change the name of jvm class metrics (#7230)

f840162

Ax1an reviewed Jul 2, 2021

View reviewed changes

wu-sheng reviewed Jul 2, 2021

View reviewed changes

language-agent/JVMMetric.proto Show resolved Hide resolved

wu-sheng reviewed Jul 2, 2021

View reviewed changes

language-agent/JVMMetric.proto Show resolved Hide resolved

add comment link to ClassLoadingMXBean and ThreadMXBean doc (#7230)

25dabe1

Ax1an reviewed Jul 2, 2021

View reviewed changes

remove deadlock metrics (#7230)

1d96f44

Ax1an reviewed Jul 2, 2021

View reviewed changes

capitalize the first letter (#7230)

1e367ae

Ax1an approved these changes Jul 2, 2021

View reviewed changes

wu-sheng approved these changes Jul 2, 2021

View reviewed changes

kezhenxu94 approved these changes Jul 3, 2021

View reviewed changes

mrproliu approved these changes Jul 4, 2021

View reviewed changes

dmsolr approved these changes Jul 4, 2021

View reviewed changes

wu-sheng merged commit bdc7af0 into apache:master Jul 4, 2021

Switch-vov mentioned this pull request Jul 5, 2021

Add some new thread metric and class metric to JVMMetric (#7230) apache/skywalking#7243

Merged

3 tasks

Switch-vov mentioned this pull request Jul 7, 2021

Remove Terminated State and New State in JVMMetric (#7230) #53

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add some new thread metric and class metric to JVMMetric #52

add some new thread metric and class metric to JVMMetric #52

Switch-vov commented Jul 2, 2021

Switch-vov commented Jul 2, 2021

Ax1an Jul 2, 2021

Switch-vov Jul 2, 2021 •

edited

Loading

Switch-vov Jul 2, 2021

wu-sheng Jul 2, 2021

wu-sheng Jul 2, 2021

Switch-vov Jul 2, 2021

Switch-vov Jul 2, 2021

kezhenxu94 Jul 3, 2021

Switch-vov Jul 3, 2021

Switch-vov Jul 3, 2021

Ax1an Jul 2, 2021

Ax1an Jul 2, 2021

Switch-vov Jul 2, 2021

wu-sheng Jul 2, 2021

Switch-vov Jul 2, 2021

Switch-vov Jul 2, 2021

kezhenxu94 Jul 3, 2021

Ax1an Jul 2, 2021

Switch-vov Jul 2, 2021

Ax1an Jul 2, 2021

Switch-vov Jul 2, 2021

Ax1an left a comment

wu-sheng left a comment

mrproliu left a comment

dmsolr left a comment

wu-sheng commented Jul 4, 2021

	// see: https://docs.oracle.com/javase/8/docs/api/java/lang/management/ThreadMXBean.html
	// See: https://docs.oracle.com/javase/8/docs/api/java/lang/management/ThreadMXBean.html

add some new thread metric and class metric to JVMMetric #52

add some new thread metric and class metric to JVMMetric #52

Conversation

Switch-vov commented Jul 2, 2021

Switch-vov commented Jul 2, 2021

Choose a reason for hiding this comment

Switch-vov Jul 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ClassProvider

ClassProviderBenchmark

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThreadProviderBenchmark

ClassProviderBenchmark

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Benchmark

Choose a reason for hiding this comment

Before

After

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ax1an left a comment

Choose a reason for hiding this comment

wu-sheng left a comment

Choose a reason for hiding this comment

mrproliu left a comment

Choose a reason for hiding this comment

dmsolr left a comment

Choose a reason for hiding this comment

wu-sheng commented Jul 4, 2021

Switch-vov Jul 2, 2021 •

edited

Loading