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

(chore) Bloom gateway: Improve fingerprint partitioning in client #11971

Merged
merged 4 commits into from
Feb 16, 2024

Conversation

chaudum
Copy link
Contributor

@chaudum chaudum commented Feb 15, 2024

What this PR does / why we need it:

This PR re-implements the fingerprint partitioning on the bloom gateway client using slices.BinarySearchFunc.

Benchmark results from sort.Search (old) vs. slices.BinarySearchFunc (new):

$ benchstat old.txt new.txt
goos: linux
goarch: amd64
pkg: github.com/grafana/loki/pkg/bloomgateway
cpu: 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz
                                   │   old.txt    │              new.txt               │
                                   │    sec/op    │   sec/op     vs base               │
PartitionFingerprintsByAddresses-8   18.75µ ± 70%   16.94µ ± 2%  -9.68% (p=0.000 n=10)

                                   │   old.txt    │             new.txt              │
                                   │     B/op     │     B/op      vs base            │
PartitionFingerprintsByAddresses-8   18.69Ki ± 0%   18.69Ki ± 0%  ~ (p=1.000 n=10) ¹
¹ all samples are equal

                                   │  old.txt   │            new.txt             │
                                   │ allocs/op  │ allocs/op   vs base            │
PartitionFingerprintsByAddresses-8   8.000 ± 0%   8.000 ± 0%  ~ (p=1.000 n=10) ¹
¹ all samples are equal

Signed-off-by: Christian Haudum <[email protected]>
Benchmark results from `sort.Search` (old) vs. `slices.BinarySearchFunc` (new):

```
$ benchstat old.txt new.txt
goos: linux
goarch: amd64
pkg: github.com/grafana/loki/pkg/bloomgateway
cpu: 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz
                                   │   old.txt    │              new.txt               │
                                   │    sec/op    │   sec/op     vs base               │
PartitionFingerprintsByAddresses-8   18.75µ ± 70%   16.94µ ± 2%  -9.68% (p=0.000 n=10)

                                   │   old.txt    │             new.txt              │
                                   │     B/op     │     B/op      vs base            │
PartitionFingerprintsByAddresses-8   18.69Ki ± 0%   18.69Ki ± 0%  ~ (p=1.000 n=10) ¹
¹ all samples are equal

                                   │  old.txt   │            new.txt             │
                                   │ allocs/op  │ allocs/op   vs base            │
PartitionFingerprintsByAddresses-8   8.000 ± 0%   8.000 ± 0%  ~ (p=1.000 n=10) ¹
¹ all samples are equal
```

Signed-off-by: Christian Haudum <[email protected]>
@chaudum chaudum changed the title Chaudum/bloomgateway client partition fingerprints (chore) Bloom gateway: Improve fingerprint partitioning in client Feb 15, 2024
@chaudum chaudum requested a review from salvacorts February 16, 2024 07:26
@chaudum chaudum marked this pull request as ready for review February 16, 2024 07:26
@chaudum chaudum requested a review from a team as a code owner February 16, 2024 07:26
@chaudum chaudum merged commit 31711d5 into main Feb 16, 2024
10 checks passed
@chaudum chaudum deleted the chaudum/bloomgateway-client-partition-fingerprints branch February 16, 2024 11:01
rhnasc pushed a commit to inloco/loki that referenced this pull request Apr 12, 2024
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