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

MINOR: Fix a race and add JMH bench for HdrHistogram #17221

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from

Conversation

dimitarndimitrov
Copy link
Contributor

About

An encore of #17184, this addresses the race described #16949 (comment) and adds a JMH benchmark comparing HdrHistogram with Yammer's histogram. It also addresses some nits from that previous PR and rebases on top of the #17011 changes.

A sample result from a local run on my development machine:

Benchmark                                       Mode  Cnt     Score    Error  Units
HistogramBenchmark.runner                       avgt    5   422.372 ± 13.081  ns/op
HistogramBenchmark.runner:readHdrHistogram      avgt    5    50.952 ±  1.330  ns/op
HistogramBenchmark.runner:readYammerHistogram   avgt    5    51.249 ±  1.319  ns/op
HistogramBenchmark.runner:writeHdrHistogram     avgt    5   319.489 ± 52.221  ns/op
HistogramBenchmark.runner:writeYammerHistogram  avgt    5  1020.283 ± 18.695  ns/op

Testing

In addition to the JMH benchmark, and unlike the original PR, #17184, this also includes a fairly reliable test for the race that's being fixed.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant