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

Allocate map for results to hold expected number of keys #17

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

56quarters
Copy link

@56quarters 56quarters commented Feb 29, 2024

Pre-allocate the map used for results based on the number of keys being fetched. Not needing to grow the map as items are added results in a 5-10% performance improvement in local benchmarking. Tested with a local instance of memcached 1.6.14

$ go test -cpu=1,2,4 -run=XXX -benchmem -bench=BenchmarkGetMulti ./... | tee old.txt
goos: linux
goarch: amd64
pkg: github.com/grafana/gomemcache/memcache
cpu: Intel(R) Core(TM) i9-10885H CPU @ 2.40GHz
BenchmarkGetMulti          12399             95828 ns/op           47040 B/op        426 allocs/op
BenchmarkGetMulti-2         9954            101181 ns/op           47053 B/op        426 allocs/op
BenchmarkGetMulti-4        10000            101635 ns/op           47060 B/op        426 allocs/op
PASS
ok      github.com/grafana/gomemcache/memcache  4.245s
$ go test -cpu=1,2,4 -run=XXX -benchmem -bench=BenchmarkGetMulti ./... | tee new.txt
goos: linux
goarch: amd64
pkg: github.com/grafana/gomemcache/memcache
cpu: Intel(R) Core(TM) i9-10885H CPU @ 2.40GHz
BenchmarkGetMulti          13216             90229 ns/op           39241 B/op        420 allocs/op
BenchmarkGetMulti-2        12792             93448 ns/op           39247 B/op        420 allocs/op
BenchmarkGetMulti-4        12559             94620 ns/op           39258 B/op        420 allocs/op
PASS
ok      github.com/grafana/gomemcache/memcache  6.436s
benchstat -delta-test none old.txt new.txt 
name        old time/op    new time/op    delta
GetMulti      95.8µs ± 0%    90.2µs ± 0%   -5.84%
GetMulti-2     101µs ± 0%      93µs ± 0%   -7.64%
GetMulti-4     102µs ± 0%      95µs ± 0%   -6.90%

name        old alloc/op   new alloc/op   delta
GetMulti      47.0kB ± 0%    39.2kB ± 0%  -16.58%
GetMulti-2    47.1kB ± 0%    39.2kB ± 0%  -16.59%
GetMulti-4    47.1kB ± 0%    39.3kB ± 0%  -16.58%

name        old allocs/op  new allocs/op  delta
GetMulti         426 ± 0%       420 ± 0%   -1.41%
GetMulti-2       426 ± 0%       420 ± 0%   -1.41%
GetMulti-4       426 ± 0%       420 ± 0%   -1.41%

Pre-allocate the map used for results based on the number of keys
being fetched. Not needing to grow the map as results are added
results in a 5-10% performance improvement in local benchmarking.
@56quarters 56quarters marked this pull request as ready for review February 29, 2024 19:04
Copy link

@leizor leizor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, LGTM!

@56quarters 56quarters merged commit cd6a66d into master Feb 29, 2024
3 checks passed
56quarters added a commit to grafana/dskit that referenced this pull request Feb 29, 2024
Specifically, pulls in grafana/gomemcache#17

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
@56quarters 56quarters deleted the 56quarters/pre-alloc branch February 29, 2024 21:09
56quarters added a commit to grafana/dskit that referenced this pull request Feb 29, 2024
Specifically, pulls in grafana/gomemcache#17

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
56quarters added a commit to grafana/mimir that referenced this pull request Feb 29, 2024
Specifically, pulls in:

grafana/dskit#500
grafana/gomemcache#17

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
56quarters added a commit to grafana/mimir that referenced this pull request Feb 29, 2024
Specifically, pulls in:

grafana/dskit#500
grafana/gomemcache#17

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
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