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

(Local)IntHistogram #254

Open
breezewish opened this issue Jun 22, 2019 · 6 comments
Open

(Local)IntHistogram #254

breezewish opened this issue Jun 22, 2019 · 6 comments

Comments

@breezewish
Copy link
Member

We need to implement a int version for Histogram, like Counter and Gauge, since atomic float implementation (loop + lock cmpxchg) is much slower than native atomic integer instruction (lock add).

For example, according to benchmarks:

test bench_counter_no_labels                          ... bench:          14 ns/iter (+/- 0)
test bench_int_counter_no_labels                      ... bench:           7 ns/iter (+/- 1)

When there is no concurrent write, float atomic implemented via CAS is 1x slower than int atomic.

test bench_counter_no_labels_concurrent_write         ... bench:       5,263 ns/iter (+/- 1,204)
test bench_int_counter_no_labels_concurrent_write     ... bench:         892 ns/iter (+/- 100)

When there is concurrent write, float atomic is 5x slower than int atomic.

@siddontang
Copy link
Contributor

Mostly, we use the histogram to record operation duration in TiKV, using Int can help us, but we need to update Grafana.

@breezewish
Copy link
Member Author

Yes, currently we use seconds and we need to update to milliseconds in order to use IntHistogram.

@hdost
Copy link

hdost commented Dec 6, 2020

Is there a block of example code which could be used to benchmark this?

@mxinden
Copy link
Contributor

mxinden commented Dec 6, 2020

@hdost today histograms don't support anything other than AtomicF64, thus there is no benchmark comparing AtomicF64 with e.g. AtomicU64. Benchmark running AtomicF64 can be found here.

@hdost
Copy link

hdost commented Dec 7, 2020

Yes indeed I wasn't sure if the bench_counter_no_labels_concurrent_write would appear in the bench tests.

hdost added a commit to hdost/rust-prometheus that referenced this issue Feb 9, 2021
hdost added a commit to hdost/rust-prometheus that referenced this issue Feb 9, 2021
@hdost
Copy link

hdost commented Feb 9, 2021

I've started on a rather large draft here: #388 before going too much further down the rabbit hole I wouldn't mind a once over. It doesn't compile and I am well aware of hat but I am curious if this path is worth pursing

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

No branches or pull requests

4 participants