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

Less locking for metric vectors. #181

Merged
merged 4 commits into from
Nov 10, 2015
Merged

Less locking for metric vectors. #181

merged 4 commits into from
Nov 10, 2015

Conversation

robx
Copy link
Contributor

@robx robx commented Nov 9, 2015

Joint work with @alicebob. @beorn7, you seem to know the vector stuff well?

  • Inline the label hashing to remove the need to keep around and lock a shared buffer and hash.
  • Try looking up metrics with a read lock before falling back to a write-locked "get or create".

This should improve lock contention issues when using variable labels in a concurrent setting. (We're currently using the hacky https://github.com/realzeitmedia/promvec, which avoids locking altogether.)

The benchmark results look quite good. BenchmarkHandler varies a lot between runs, that delta is not always positive.

$ benchcmp old.txt new.txt 
benchmark                                       old ns/op     new ns/op     delta
BenchmarkCounterWithLabelValues-4               462           144           -68.83%
BenchmarkCounterWithLabelValuesConcurrent-4     561           91.7          -83.65%
BenchmarkCounterWithMappedLabels-4              2360          760           -67.80%
BenchmarkCounterWithPreparedMappedLabels-4      763           178           -76.67%
BenchmarkCounterNoLabels-4                      26.7          22.8          -14.61%
BenchmarkGaugeWithLabelValues-4                 407           129           -68.30%
BenchmarkGaugeNoLabels-4                        15.7          13.7          -12.74%
BenchmarkSummaryWithLabelValues-4               1672          1550          -7.30%
BenchmarkSummaryNoLabels-4                      1671          1408          -15.74%
BenchmarkHistogramWithLabelValues-4             550           191           -65.27%
BenchmarkHistogramNoLabels-4                    72.5          62.4          -13.93%
BenchmarkHistogramObserve1-4                    53.3          45.7          -14.26%
BenchmarkHistogramObserve2-4                    171           200           +16.96%
BenchmarkHistogramObserve4-4                    433           530           +22.40%
BenchmarkHistogramObserve8-4                    987           1070          +8.41%
BenchmarkHistogramWrite1-4                      5718          4103          -28.24%
BenchmarkHistogramWrite2-4                      6783          5710          -15.82%
BenchmarkHistogramWrite4-4                      12864         12178         -5.33%
BenchmarkHistogramWrite8-4                      25073         22742         -9.30%
BenchmarkHandler-4                              12246782      20103907      +64.16%
BenchmarkSummaryObserve1-4                      1917          1950          +1.72%
BenchmarkSummaryObserve2-4                      5591          4818          -13.83%
BenchmarkSummaryObserve4-4                      12727         12346         -2.99%
BenchmarkSummaryObserve8-4                      26542         25127         -5.33%
BenchmarkSummaryWrite1-4                        23767         20058         -15.61%
BenchmarkSummaryWrite2-4                        48698         46691         -4.12%
BenchmarkSummaryWrite4-4                        92121         92998         +0.95%
BenchmarkSummaryWrite8-4                        196476        182895        -6.91%

benchmark                                       old allocs     new allocs     delta
BenchmarkCounterWithLabelValues-4               0              0              +0.00%
BenchmarkCounterWithLabelValuesConcurrent-4     0              0              +0.00%
BenchmarkCounterWithMappedLabels-4              3              2              -33.33%
BenchmarkCounterWithPreparedMappedLabels-4      1              0              -100.00%
BenchmarkCounterNoLabels-4                      0              0              +0.00%
BenchmarkGaugeWithLabelValues-4                 0              0              +0.00%
BenchmarkGaugeNoLabels-4                        0              0              +0.00%
BenchmarkSummaryWithLabelValues-4               0              0              +0.00%
BenchmarkSummaryNoLabels-4                      0              0              +0.00%
BenchmarkHistogramWithLabelValues-4             0              0              +0.00%
BenchmarkHistogramNoLabels-4                    0              0              +0.00%

benchmark                                       old bytes     new bytes     delta
BenchmarkCounterWithLabelValues-4               0             0             +0.00%
BenchmarkCounterWithLabelValuesConcurrent-4     0             0             +0.00%
BenchmarkCounterWithMappedLabels-4              384           336           -12.50%
BenchmarkCounterWithPreparedMappedLabels-4      48            0             -100.00%
BenchmarkCounterNoLabels-4                      0             0             +0.00%
BenchmarkGaugeWithLabelValues-4                 0             0             +0.00%
BenchmarkGaugeNoLabels-4                        0             0             +0.00%
BenchmarkSummaryWithLabelValues-4               0             0             +0.00%
BenchmarkSummaryNoLabels-4                      0             0             +0.00%
BenchmarkHistogramWithLabelValues-4             0             0             +0.00%
BenchmarkHistogramNoLabels-4                    0             0             +0.00%

@beorn7
Copy link
Member

beorn7 commented Nov 9, 2015

Thanks for your contribution.

I like the general idea. I'll have a closer look ASAP.

@beorn7
Copy link
Member

beorn7 commented Nov 10, 2015

👍 Looks perfect to me. Merging...

And thanks again. I guess we can apply the same idea to a few other performance critical paths...

beorn7 added a commit that referenced this pull request Nov 10, 2015
Less locking for metric vectors.
@beorn7 beorn7 merged commit cf3f724 into prometheus:master Nov 10, 2015
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.

2 participants