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

perf(blooms): mempool no longer zeroes out buffers unnecessarily #13282

Merged
merged 3 commits into from
Jun 21, 2024

Conversation

owen-d
Copy link
Member

@owen-d owen-d commented Jun 21, 2024

Dramatically improves the speed of mempool usage. I noticed some code in the mempool which iterated over buffers on every get() call. Guessing this to be expensive, I added a few benchmarks -- this is one of those rare -100% speed improvements. The reason this is safe because only the length of the provided []byte is important -- NOT it's contents. I verified this everywhere it was used (I knew this was the expected contract already), added some benchmarks, and we saw a great improvement.

goos: darwin
goarch: arm64
pkg: github.com/grafana/loki/v3/pkg/util/mempool
              │   /tmp/old.txt   │             /tmp/new.txt             │
              │      sec/op      │   sec/op     vs base                 │
Slab/1KB-10         121.35n ± 1%   33.80n ± 1%   -72.15% (p=0.000 n=10)
Slab/1MB-10       11222.00n ± 2%   33.82n ± 0%   -99.70% (p=0.000 n=10)
Slab/128MB-10   1420680.00n ± 2%   33.75n ± 0%  -100.00% (p=0.000 n=10)
geomean              12.46µ        33.79n        -99.73%

              │ /tmp/old.txt │            /tmp/new.txt             │
              │     B/op     │    B/op     vs base                 │
Slab/1KB-10     0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Slab/1MB-10     0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Slab/128MB-10   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                    ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

              │ /tmp/old.txt │            /tmp/new.txt             │
              │  allocs/op   │ allocs/op   vs base                 │
Slab/1KB-10     0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Slab/1MB-10     0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Slab/128MB-10   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                    ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

adds benchmarks
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d owen-d requested a review from a team as a code owner June 21, 2024 17:33
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d owen-d merged commit eb1cd4c into grafana:main Jun 21, 2024
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants