Add basic jmh overhead test by trask · Pull Request #3672 · open-telemetry/opentelemetry-java-instrumentation

trask · 2021-07-25T21:30:56Z

I did a bit of profiling, and planning to submit a couple of (micro) optimizations, so want to first submit the test I'm using for profiling/measurement.

Benchmark                                                               Score      Error   Units
ServletBenchmark.execute                                              139.343 ±    0.138   us/op
ServletBenchmark.execute:execute·p0.50                                125.696              us/op
ServletBenchmark.execute:·gc.alloc.rate.norm                        29111.284 ± 7057.135    B/op
ServletWithAgentDisabledBenchmark.execute                             119.724 ±    0.112   us/op
ServletWithAgentDisabledBenchmark.execute:execute·p0.50               110.464              us/op
ServletWithAgentDisabledBenchmark.execute:·gc.alloc.rate.norm       25580.861 ± 5803.009    B/op
ServletWithOnePercentSamplingBenchmark.execute                        138.331 ±    0.135   us/op
ServletWithOnePercentSamplingBenchmark.execute:execute·p0.50          124.928              us/op
ServletWithOnePercentSamplingBenchmark.execute:·gc.alloc.rate.norm  28508.222 ± 6843.209    B/op

trask · 2021-07-26T02:41:28Z

buildSrc/src/main/kotlin/otel.jmh-conventions.gradle.kts

+  id("me.champeau.jmh")
+  id("io.morethan.jmhreport")
+}
+


this file is identical copy from opentelemetry-java repo

trask · 2021-07-26T02:43:53Z

testing-overhead-jmh/build.gradle.kts

+    // TODO(trask) is this ok? if it's ok, move to otel.jmh-conventions?
+    outputs.upToDateWhen { false }


gradle experts: is this ok? each time I run jmh task I actually want it to run. using --rerun-tasks takes a long time because it doesn't only rerun this one task

trask · 2021-07-26T02:50:57Z

testing-overhead-jmh/build.gradle.kts

+  val copyAgent by registering(Copy::class) {
+    into(file("$buildDir/agent"))
+    from(configurations.named("agentJar"))
+    rename { file: String ->
+      file.replace("-$version", "")
+    }
+
+    dependsOn(":javaagent:shadowJar")
+  }


gradle experts: let me know if there is a more preferred way to get the opentelemetry-javaagent-all into a location that the jmh benchmark can depend on

...verhead-jmh/src/jmh/java/io/opentelemetry/javaagent/benchmark/ServletWithAgentBenchmark.java

iNikem · 2021-07-26T07:15:26Z

buildSrc/src/main/kotlin/otel.jmh-conventions.gradle.kts

+  profilers.add("gc")
+  val jmhIncludeSingleClass: String? by project
+  if (jmhIncludeSingleClass != null) {
+    includes.add(jmhIncludeSingleClass as String)


Just curious: why this as String is needed? It is already clear that this is not null.

IIRC, the by syntax seems to result in something "dynamic". Seems like it might be a Gradle bug, but it makes kotlin not treat the null check as a type cast.

testing-overhead-jmh/build.gradle.kts

anuraaga · 2021-07-26T07:35:42Z

buildSrc/src/main/kotlin/otel.jmh-conventions.gradle.kts

+  profilers.add("gc")
+  val jmhIncludeSingleClass: String? by project
+  if (jmhIncludeSingleClass != null) {
+    includes.add(jmhIncludeSingleClass as String)


IIRC, the by syntax seems to result in something "dynamic". Seems like it might be a Gradle bug, but it makes kotlin not treat the null check as a type cast.

testing-overhead-jmh/build.gradle.kts

mateuszrzeszutek · 2021-07-26T10:19:13Z

...jmh/java/io/opentelemetry/javaagent/benchmark/servlet/ServletWithAgentDisabledBenchmark.java

+
+import org.openjdk.jmh.annotations.Fork;
+
+@Fork(jvmArgsAppend = "-Dotel.javaagent.enabled=false")


Just an idea: we could also run a benchmark with -Dotel.javaagent.experimental.use-noop-api=true to see how much overhead is added when no SDK at all is used.

mateuszrzeszutek · 2021-07-26T10:21:40Z

...overhead-jmh/src/jmh/java/io/opentelemetry/javaagent/benchmark/servlet/ServletBenchmark.java

+public class ServletBenchmark {
+
+  static {
+    HelloWorldApplication.main();


Just wondering: why do we initialize the app in static {} and not @Setup?

added comment to explain

// using static initializer instead of @Setup since only want to initialize the app under test // once regardless of @State and @Threads

mateuszrzeszutek · 2021-07-26T10:24:14Z

Just thought about it now, but should we name the dir/module benchmark-overhead-jmh? Usually our testing... modules contain code that tests for correctness, we could keep everything that tests for performance in benchmark... modules.

trask · 2021-07-26T17:34:58Z

Just thought about it now, but should we name the dir/module benchmark-overhead-jmh? Usually our testing... modules contain code that tests for correctness, we could keep everything that tests for performance in benchmark... modules.

ya, I like this, updated

trask · 2021-07-26T18:31:53Z

here's a re-run, including the no-op api test:

Benchmark                                                               Score      Error   Units
ServletBenchmark.execute                                              138.941 ±    0.137   us/op
ServletBenchmark.execute:execute·p0.50                                125.824              us/op
ServletBenchmark.execute:·gc.alloc.rate.norm                        29090.156 ± 7065.042    B/op
ServletWithAgentDisabledBenchmark.execute                             122.255 ±    0.107   us/op
ServletWithAgentDisabledBenchmark.execute:execute·p0.50               112.768              us/op
ServletWithAgentDisabledBenchmark.execute:·gc.alloc.rate.norm       25585.117 ± 5811.971    B/op
ServletWithNoopApiBenchmark.execute                                   134.359 ±    0.127   us/op
ServletWithNoopApiBenchmark.execute:execute·p0.50                     121.344              us/op
ServletWithNoopApiBenchmark.execute:·gc.alloc.rate.norm             27834.075 ± 6578.200    B/op
ServletWithOnePercentSamplingBenchmark.execute                        136.299 ±    0.130   us/op
ServletWithOnePercentSamplingBenchmark.execute:execute·p0.50          123.264              us/op
ServletWithOnePercentSamplingBenchmark.execute:·gc.alloc.rate.norm  28484.639 ± 6846.715    B/op

Add basic jmh overhead test

132b7a5

trask commented Jul 26, 2021

View reviewed changes

trask marked this pull request as ready for review July 26, 2021 02:51

trask requested review from anuraaga, iNikem, jkwatson, laurit, mateuszrzeszutek, pavolloffay and tylerbenson as code owners July 26, 2021 02:51

anuraaga reviewed Jul 26, 2021

View reviewed changes

...verhead-jmh/src/jmh/java/io/opentelemetry/javaagent/benchmark/ServletWithAgentBenchmark.java Outdated Show resolved Hide resolved

trask added 2 commits July 25, 2021 20:37

Merge remote-tracking branch 'upstream/main' into overhead-testing-jmh

3ef6f63

Move javaagent arg to gradle file

653cf59

iNikem approved these changes Jul 26, 2021

View reviewed changes

anuraaga approved these changes Jul 26, 2021

View reviewed changes

mateuszrzeszutek approved these changes Jul 26, 2021

View reviewed changes

trask added 5 commits July 26, 2021 10:20

Merge remote-tracking branch 'upstream/main' into overhead-testing-jmh

cdefb51

Rename module to benchmark-overhead-jmh

a508afd

Add imports

6c42d16

Add comment

e146a54

Add benchmark for noop api

30f757a

trask merged commit 59e4647 into open-telemetry:main Jul 26, 2021

trask deleted the overhead-testing-jmh branch July 26, 2021 19:55

trask mentioned this pull request Jul 26, 2021

Add JFR profile analyzer #3678

Merged

breedx-splk mentioned this pull request Jul 27, 2021

Overhead testing foundations #3627

Merged

		// TODO(trask) is this ok? if it's ok, move to otel.jmh-conventions?
		outputs.upToDateWhen { false }


		import org.openjdk.jmh.annotations.Fork;

		@Fork(jvmArgsAppend = "-Dotel.javaagent.enabled=false")

Conversation

trask commented Jul 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateuszrzeszutek commented Jul 26, 2021

Uh oh!

trask commented Jul 26, 2021

Uh oh!

trask commented Jul 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

trask commented Jul 25, 2021 •

edited

Loading