Uh oh!
There was an error while loading. Please reload this page.
[improvement] add jmh benchmark - #179
Merged
Merged
Conversation
wenhoujx
commented
Jun 18, 2019
ContributorAuthor
|
| Tracer.fastCompleteSpan(); | ||
| } | ||
| }; | ||
| blackhole.consume(callable.call()); |
Contributor
There was a problem hiding this comment.
We don't need to blackhole the null result from our callable, however we should blackhole the output, which in this case is a series of Span objects:
Tracer.subscribe("jmh", blackhole::consume);ContributorAuthor
There was a problem hiding this comment.
@carterkozak thanks for CR, does it matter that every call of Tracer.startSpan("span"); starts a span with the same name?
Contributor
There was a problem hiding this comment.
Spans with the same name are totally fine, we have them in practice via recursive methods ;-)
ContributorAuthor
There was a problem hiding this comment.
good example. do you want each span subscribed by blackhole::consume? which makes the precomputed Callable<Object> becomes a Function<BlackHole, Object> or even Consumer<BlackHole>
Contributor
There was a problem hiding this comment.
We just need to set the subscriber at the beginning when we get the blackhole instance. Ideally we could configure that globally instead of for each benchmark method, but I’m not sure how. Note that tracing observer registration is a static per jvm, so concurrent measurements will reset the observer
ContributorAuthor
There was a problem hiding this comment.
@carterkozak can i use a UUID as the observer name? so that each invocation has different observer.
Contributor
There was a problem hiding this comment.
No, you'll end up writing each span to N observers instead of the case we want to benchmark with a single observer.
Contributor
There was a problem hiding this comment.
I'd start with something along these lines:
@Fork(1)
@Threads(1)
@State(Scope.Benchmark)
@Warmup(iterations = 3, time = 3)
@Measurement(iterations = 3, time = 3)
publicclassSimpleBenchmark {
@Setuppublicvoidsetup(Blackholeblackhole) {
Tracer.subscribe("jmh", blackhole::consume);
Tracer.setSampler(AlwaysSampler.INSTANCE);
}
@TearDownpublicvoidtearDown() {
Tracer.unsubscribe("jmh");
}
@BenchmarkpublicvoidnestedSpans() {
for (inti = 0; i < 100; i++) {
Tracer.startSpan("span");
}
for (inti = 0; i < 100; i++) {
Tracer.fastCompleteSpan();
}
}
}Contributor
There was a problem hiding this comment.
The sampler can be parameterized so that we can write one benchmark and get results for both the sampled and unsampled cases using parameterized values in state, see the internal class I linked in a DM.
| return NOOP_CALLABLE.call(); | ||
| } finally { | ||
| Tracer.fastCompleteSpan(); | ||
| } |
Contributor
There was a problem hiding this comment.
Should precompute the callable wrappers.
wenhoujx
commented
Jun 18, 2019
ContributorAuthor
the three benchmarks has similar throughput and gc? i must be doing something wrong: |
| dependencies { | ||
| annotationProcessor 'org.openjdk.jmh:jmh-generator-annprocess' | ||
| compileOnly 'org.openjdk.jmh:jmh-generator-annprocess' | ||
| compile "org.immutables:value::annotations" |
Contributor
There was a problem hiding this comment.
What about compileOnly + annotationProocessor?
ContributorAuthor
There was a problem hiding this comment.
> Task :tracing-benchmarks:compileJmhJava FAILED
warning: unknown enum constant ImplementationVisibility.PACKAGE
reason: class file for org.immutables.value.Value$Style$ImplementationVisibility not found
warning: unknown enum constant ImplementationVisibility.PACKAGE
error: warnings found and -Werror specified
1 error
2 warnings
the same
carterkozak
commented
Jun 18, 2019
Contributor
Will take a look in a while. |
wenhoujx
commented
Jun 19, 2019
ContributorAuthor
new benchmark with parameters and |
ContributorAuthor
new benchmark for commit 7d1d92f |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
add span benchmarks according to @carterkozak 's comments on #176
remove
Before this PR
After this PR