Skip to content

slightly decrease thread contention by synchronizing once rather than…#481

Merged
brian-brazil merged 1 commit intoprometheus:masterfrom
t3hnar:master
Jun 17, 2019
Merged

slightly decrease thread contention by synchronizing once rather than…#481
brian-brazil merged 1 commit intoprometheus:masterfrom
t3hnar:master

Conversation

@t3hnar
Copy link
Contributor

@t3hnar t3hnar commented Jun 2, 2019

Improve Summary: slightly decrease thread contention by synchronizing once rather than "number quantiles + 1" times

@t3hnar
slightly decrease thread contention by synchronizing once rather than…
c726b1a 
… "number quantiles + 1" times

Signed-off-by: Yaroslav Klymko <t3hnar@gmail.com>
@ghost
Copy link

ghost commented Jun 3, 2019

It looks like this doesn't make a significant difference. 🙁

Benchmark:

# JMH version: 1.21
# VM version: JDK 11.0.2, OpenJDK 64-Bit Server VM, 11.0.2+9
# VM invoker: /Library/Java/JavaVirtualMachines/openjdk-11.0.2.jdk/Contents/Home/bin/java
# VM options: -javaagent:/Applications/IntelliJ IDEA CE.app/Contents/lib/idea_rt.jar=49590:/Applications/IntelliJ IDEA CE.app/Contents/bin -Dfile.encoding=UTF-8
# Warmup: 5 iterations, 10 s each
# Measurement: 4 iterations, 10 s each
# Timeout: 10 min per iteration
# Threads: 24 threads, will synchronize iterations

Please take the results with a grain of salt.

Before - 4e0e752:

Benchmark                                                           Mode  Cnt       Score        Error  Units
SummaryBenchmark.codahaleHistogramBenchmark                         avgt    4    5449.211 ±    157.085  ns/op
SummaryBenchmark.prometheusSimpleHistogramBenchmark                 avgt    4     219.256 ±     23.811  ns/op
SummaryBenchmark.prometheusSimpleHistogramChildBenchmark            avgt    4      88.585 ±      1.734  ns/op
SummaryBenchmark.prometheusSimpleHistogramNoLabelsBenchmark         avgt    4      83.989 ±      2.093  ns/op
SummaryBenchmark.prometheusSimpleSummaryBenchmark                   avgt    4     186.796 ±     15.029  ns/op
SummaryBenchmark.prometheusSimpleSummaryChildBenchmark              avgt    4      61.782 ±      5.032  ns/op
SummaryBenchmark.prometheusSimpleSummaryNoLabelsBenchmark           avgt    4      59.388 ±     12.457  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesBenchmark          avgt    4  530692.958 ± 627614.926  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesChildBenchmark     avgt    4  563807.323 ± 389413.917  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesNoLabelsBenchmark  avgt    4  559719.005 ± 631676.260  ns/op

After - c726b1a:

Benchmark                                                           Mode  Cnt       Score        Error  Units
SummaryBenchmark.codahaleHistogramBenchmark                         avgt    4    5651.711 ±    487.476  ns/op
SummaryBenchmark.prometheusSimpleHistogramBenchmark                 avgt    4     220.865 ±     11.554  ns/op
SummaryBenchmark.prometheusSimpleHistogramChildBenchmark            avgt    4      95.237 ±      6.059  ns/op
SummaryBenchmark.prometheusSimpleHistogramNoLabelsBenchmark         avgt    4      87.088 ±     15.551  ns/op
SummaryBenchmark.prometheusSimpleSummaryBenchmark                   avgt    4     196.351 ±     33.766  ns/op
SummaryBenchmark.prometheusSimpleSummaryChildBenchmark              avgt    4      60.853 ±      9.388  ns/op
SummaryBenchmark.prometheusSimpleSummaryNoLabelsBenchmark           avgt    4      64.209 ±      2.437  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesBenchmark          avgt    4  525567.096 ± 599616.355  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesChildBenchmark     avgt    4  724240.526 ± 864857.588  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesNoLabelsBenchmark  avgt    4  703944.682 ± 906458.910  ns/op

I picked the result with the smallest error from multiple benchmark runs.

Quantiles:

  .quantile(0.5, 0.05).quantile(0.9, 0.05).quantile(0.95, 0.01).quantile(0.99, 0.005)

@brian-brazil
Copy link
Contributor

brian-brazil commented Jun 3, 2019

It likely needs a different benchmark design for this to show up.

@t3hnar
Copy link
Contributor Author

t3hnar commented Jun 3, 2019

@brian-brazil is there something which stops you from accepting this PR ? how can I improve this?

@brian-brazil
Copy link
Contributor

brian-brazil commented Jun 3, 2019 via email

@ghost

This comment has been minimized.

Sign in to view
This was referenced Jun 5, 2019
Closed
Closed
@ghost
Copy link

ghost commented Jun 15, 2019

Benchmarks with unrealistically frequent rotations by hardcoding TimeWindowQuantiles. durationBetweenRotates.

The results were consistent across many runs over the last 2 days.

Before - 4e0e752 - durationBetweenRotates=1ms:

Benchmark                                                           Mode  Cnt      Score      Error  Units
SummaryBenchmark.prometheusSimpleSummaryBenchmark                   avgt    4    180.660 ±   31.025  ns/op
SummaryBenchmark.prometheusSimpleSummaryChildBenchmark              avgt    4     59.794 ±    2.183  ns/op
SummaryBenchmark.prometheusSimpleSummaryNoLabelsBenchmark           avgt    4     62.603 ±    2.560  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesBenchmark          avgt    4  16668.278 ±  899.351  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesChildBenchmark     avgt    4  16473.560 ± 1271.724  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesNoLabelsBenchmark  avgt    4  16279.621 ± 1513.338  ns/op

After - c726b1a - durationBetweenRotates=1ms:

Benchmark                                                           Mode  Cnt      Score      Error  Units
SummaryBenchmark.prometheusSimpleSummaryBenchmark                   avgt    4    188.946 ±   23.181  ns/op
SummaryBenchmark.prometheusSimpleSummaryChildBenchmark              avgt    4     59.379 ±    2.877  ns/op
SummaryBenchmark.prometheusSimpleSummaryNoLabelsBenchmark           avgt    4     59.461 ±    1.214  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesBenchmark          avgt    4  14943.023 ± 1479.789  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesChildBenchmark     avgt    4   9085.545 ±  778.160  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesNoLabelsBenchmark  avgt    4   9181.785 ±  976.102  ns/op

Before - 4e0e752 - durationBetweenRotates=100us:

Benchmark                                                           Mode  Cnt      Score     Error  Units
SummaryBenchmark.prometheusSimpleSummaryBenchmark                   avgt    4    190.128 ±  37.522  ns/op
SummaryBenchmark.prometheusSimpleSummaryChildBenchmark              avgt    4     59.393 ±   1.586  ns/op
SummaryBenchmark.prometheusSimpleSummaryNoLabelsBenchmark           avgt    4     62.860 ±   7.727  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesBenchmark          avgt    4  12261.290 ± 206.572  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesChildBenchmark     avgt    4  12279.661 ± 117.961  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesNoLabelsBenchmark  avgt    4  12317.630 ± 270.580  ns/op

After - c726b1a - durationBetweenRotates=100us:

Benchmark                                                           Mode  Cnt      Score     Error  Units
SummaryBenchmark.prometheusSimpleSummaryBenchmark                   avgt    4    184.783 ±  29.702  ns/op
SummaryBenchmark.prometheusSimpleSummaryChildBenchmark              avgt    4     59.636 ±   1.406  ns/op
SummaryBenchmark.prometheusSimpleSummaryNoLabelsBenchmark           avgt    4     61.560 ±   2.485  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesBenchmark          avgt    4  12279.452 ±  29.655  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesChildBenchmark     avgt    4   7940.067 ± 590.255  ns/op
SummaryBenchmark.prometheusSimpleSummaryQuantilesNoLabelsBenchmark  avgt    4   7320.339 ± 260.343  ns/op

@brian-brazil brian-brazil merged commit 6200135 into prometheus:master Jun 17, 2019
@brian-brazil
Copy link
Contributor

brian-brazil commented Jun 17, 2019

Thanks!

@ghost ghost mentioned this pull request Jun 18, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@t3hnar @brian-brazil