Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic jmh overhead test #3672

Merged
merged 8 commits into from
Jul 26, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions buildSrc/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ dependencies {
implementation("org.gradle:test-retry-gradle-plugin:1.2.1")
// When updating, also update dependencyManagement/dependencyManagement.gradle.kts
implementation("net.bytebuddy:byte-buddy-gradle-plugin:1.11.2")
implementation("gradle.plugin.io.morethan.jmhreport:gradle-jmh-report:0.9.0")
implementation("me.champeau.jmh:jmh-gradle-plugin:0.6.5")
implementation("net.ltgt.gradle:gradle-errorprone-plugin:2.0.1")
implementation("net.ltgt.gradle:gradle-nullaway-plugin:1.1.0")

Expand Down
36 changes: 36 additions & 0 deletions buildSrc/src/main/kotlin/otel.jmh-conventions.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
plugins {
id("me.champeau.jmh")
id("io.morethan.jmhreport")
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file is identical copy from opentelemetry-java repo

dependencies {
jmh(platform(project(":dependencyManagement")))
jmh("org.openjdk.jmh:jmh-core")
jmh("org.openjdk.jmh:jmh-generator-bytecode")
}

// invoke jmh on a single benchmark class like so:
// ./gradlew -PjmhIncludeSingleClass=StatsTraceContextBenchmark clean :grpc-core:jmh
jmh {
failOnError.set(true)
resultFormat.set("JSON")
// Otherwise an error will happen:
// Could not expand ZIP 'byte-buddy-agent-1.9.7.jar'.
includeTests.set(false)
profilers.add("gc")
val jmhIncludeSingleClass: String? by project
if (jmhIncludeSingleClass != null) {
includes.add(jmhIncludeSingleClass as String)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
}

jmhReport {
jmhResultPath = file("${buildDir}/results/jmh/results.json").absolutePath
jmhReportOutput = file("${buildDir}/results/jmh").absolutePath
}

tasks {
named("jmh") {
finalizedBy(named("jmhReport"))
}
}
5 changes: 5 additions & 0 deletions dependencyManagement/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ val DEPENDENCY_SETS = listOf(
"1.11.2",
listOf("byte-buddy", "byte-buddy-agent", "byte-buddy-gradle-plugin")
),
DependencySet(
"org.openjdk.jmh",
"1.32",
listOf("jmh-core", "jmh-generator-bytecode")
),
DependencySet(
"org.mockito",
"3.11.1",
Expand Down
2 changes: 2 additions & 0 deletions settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ include(":testing-common")
include(":testing-common:integration-tests")
include(":testing-common:library-for-integration-tests")

include(":testing-overhead-jmh")

// smoke tests
include(":smoke-tests")

Expand Down
47 changes: 47 additions & 0 deletions testing-overhead-jmh/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import net.ltgt.gradle.errorprone.errorprone

plugins {
id("otel.java-conventions")
id("otel.jmh-conventions")
}

dependencies {
jmhImplementation("org.springframework.boot:spring-boot-starter-web:2.5.2")
jmhImplementation(project(":testing-common")) {
exclude("ch.qos.logback")
}

// this only exists to make Intellij happy since it doesn't (currently at least) understand our
// inclusion of this artifact inside of :testing-common
jmhCompileOnly(project(path = ":testing:armeria-shaded-for-testing", configuration = "shadow"))
}

tasks {

// TODO(trask) without disabling errorprone, jmh task fails with
// Task :testing-overhead-jmh:jmhCompileGeneratedClasses FAILED
// error: plug-in not found: ErrorProne
withType<JavaCompile>().configureEach {
options.errorprone {
isEnabled.set(false)
}
}

named<me.champeau.jmh.JMHTask>("jmh") {
trask marked this conversation as resolved.
Show resolved Hide resolved
val shadowTask = project(":javaagent").tasks.named<com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar>("shadowJar").get()
inputs.files(layout.files(shadowTask))

// note: without an exporter, toSpanData() won't even be called
// (which is good for benchmarking the instrumentation itself)
val args = listOf(
"-javaagent:${shadowTask.archiveFile.get()}",
"-Dotel.traces.exporter=none",
"-Dotel.metrics.exporter=none"
)
// see https://github.com/melix/jmh-gradle-plugin/issues/200
jvmArgsPrepend.add(args.joinToString(" "))

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.benchmark.servlet;

import io.opentelemetry.javaagent.benchmark.servlet.app.HelloWorldApplication;
import io.opentelemetry.testing.internal.armeria.client.WebClient;
import java.util.concurrent.TimeUnit;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.TearDown;

@BenchmarkMode(Mode.SampleTime)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
@State(Scope.Thread)
public class ServletBenchmark {

static {
HelloWorldApplication.main();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

// using shaded armeria http client from testing-common artifact since it won't be instrumented
private WebClient client;

@Setup
public void setup() {
client = WebClient.builder().build();
}

@TearDown
public void tearDown() {
HelloWorldApplication.stop();
}

@Benchmark
public Object execute() {
return client.get("http://localhost:8080").aggregate().join();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.benchmark.servlet;

import org.openjdk.jmh.annotations.Fork;

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added 👍

public class ServletWithAgentDisabledBenchmark extends ServletBenchmark {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.benchmark.servlet;

import org.openjdk.jmh.annotations.Fork;

@Fork(jvmArgsAppend = {"-Dotel.traces.sampler=traceidratio", "-Dotel.traces.sampler.arg=0.01"})
public class ServletWithOnePercentSamplingBenchmark extends ServletBenchmark {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.benchmark.servlet.app;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.context.ConfigurableApplicationContext;

@SpringBootApplication
public class HelloWorldApplication {

private static volatile ConfigurableApplicationContext context;

public static void main(String... args) {

context = SpringApplication.run(HelloWorldApplication.class, args);
}

public static void stop() {
context.stop();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.benchmark.servlet.app;

import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;

@RestController
public class HelloWorldController {

@GetMapping("/")
public String root() {
return "Hello world!";
}
}