Optimized hdr_value_at_percentile#10606
Conversation
af88237 to
51d9c93
Compare
Co-authored-by: Oran Agra <[email protected]>
oranagra
left a comment
There was a problem hiding this comment.
i didn't review the actual changes to the hdr_histogram code itself, but then again, i didn't review these when they where introduced into redis either.
leaving this PR open for a while to see if anyone else wants to comment.
@oranagra the changes from @yoav-steinberg are fully safe and have been prooveed to be working as expected also on the go version of the hdrhistogram. IMHO we can merge this improvement without waiting for the merge of HdrHistogram/HdrHistogram_c#107. excellent work @yoav-steinberg and thank you for pushing this forward! Impressive to see the bump in the |
|
@yoav-steinberg https://github.com/redis/redis/blob/unstable/deps/hdr_histogram/hdr_histogram.c#L21 Need to change to |
|
@yangbodong22011 we don't.. see |
oh, I didn't check the history, it turns out this is already part of the library itself. IMHO the correct way to define an allocator should be through an interface, not by modifying files: https://github.com/RoaringBitmap/CRoaring/pull/358/files |
…#10606. The code is based on upstream https://github.com/HdrHistogram/HdrHistogram_c master branch latest commit (e4448cf6d1cd08fff519812d3b1e58bd5a94ac42). The reason to pull this in now is that their final version of our optimization is even faster. See: HdrHistogram/HdrHistogram_c#107.
… (#10755) The code is based on upstream https://github.com/HdrHistogram/HdrHistogram_c master branch latest commit (e4448cf6d1cd08fff519812d3b1e58bd5a94ac42). The reason to pull this in now is that their final version of our optimization is even faster. See: HdrHistogram/HdrHistogram_c#107.
`hdr_value_at_percentile()` is part of the Hdr_Histogram library used when generating `latencystats` report. There's a pending optimization for this function which greatly affects the performance of `info latencystats`. HdrHistogram/HdrHistogram_c#107 This PR: 1. Upgrades the sources in _deps/hdr_histogram_ to the latest Hdr_Histogram version 0.11.5 2. Applies the referenced optimization. 3. Adds minor documentation about the hdr_histogram dependency which was missing under _deps/README.md_. benchmark on my machine: running: `redis-benchmark -n 100000 info latencystats` on a clean build with no data. | benchmark | RPS | | ---- | ---- | | before upgrade to v0.11.05 | 7,681 | | before optimization | 12,474 | | after optimization | 52,606 | Co-authored-by: filipe oliveira <[email protected]>
…#10606. (redis#10755) The code is based on upstream https://github.com/HdrHistogram/HdrHistogram_c master branch latest commit (e4448cf6d1cd08fff519812d3b1e58bd5a94ac42). The reason to pull this in now is that their final version of our optimization is even faster. See: HdrHistogram/HdrHistogram_c#107.
hdr_value_at_percentile()is part of the Hdr_Histogram library used when generatinglatencystatsreport.There's a pending optimization for this function which greatly affects the performance of
info latencystats.This PR:
benchmark on my machine:
running:
redis-benchmark -n 100000 info latencystatson a clean build with no data.