read performance #42
read performance #42
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #42 +/- ##
==========================================
- Coverage 88.72% 88.20% -0.53%
==========================================
Files 24 25 +1
Lines 2564 2637 +73
==========================================
+ Hits 2275 2326 +51
- Misses 199 217 +18
- Partials 90 94 +4 ☔ View full report in Codecov by Sentry. |
|
In general, it looks good, but I couldn't repeat the distribution of your benchmarks. |
|
I used benchmarks based on go-cache-benchmark-plus with a few edits.
I think the current version of go-cache-benchmark has too many bugs, so the benchmarks are fixed too. func BenchmarkGetParallel(b *testing.B) {
keys := []string{}
for i := 0; i < 100000; i++ {
keys = append(keys, fmt.Sprintf("%d", i))
}
for _, client := range benchClients {
client.Init(100000)
// The filling has been added
for _, key := range keys {
client.Set(key, key)
}
b.ResetTimer()
b.Run(client.Name(), func(b *testing.B) {
b.RunParallel(func(p *testing.PB) {
// It seems better not to read one key out of all the goroutines,
// but the results of this almost do not change in any way.
counter := rand.Int() % 100000
for p.Next() {
client.Get(keys[counter%100000])
counter++
}
})
})
client.Close()
}
}I got the following on M1 Max. Everything is a bit different on Set benchmarks. func BenchmarkSetParallel(b *testing.B) {
keys := []string{}
for i := 0; i < 1000000; i++ {
keys = append(keys, fmt.Sprintf("%d", i))
}
for _, client := range benchClients {
client.Init(100000)
b.ResetTimer()
b.Run(client.Name(), func(b *testing.B) {
b.RunParallel(func(p *testing.PB) {
// 1 insert and inf updates seem to be too cheating for theine.
// Since it has a fastpath, which will only help for this case.
// So let's add a random selection.
counter := rand.Int() % 1000000
for p.Next() {
client.Set(keys[counter%1000000], "bar")
counter++
}
})
})
client.Close()
}
}And we get the following. |
|
@maypok86 I actually prefill the cache in the Get method, but the code hasn't been pushed yet. I've now updated the repo. I'm considering also fill the cache for set and make it an updating benchmark, but let me foucs on read performance first. This is the updated result: I can't reproduce your Get result(71.68 ns/op), which theine is still much slower than ristretto, is that |
Yeah, that was the first thing I checked. This also proves the change in results since the latest release. For example, here are the results of my benchmarks. With theine from the perf branch. ( With theine from the latest release. ( |
|
@maypok86 I figured out the issue. The atomic add operation is significantly slower on ARM64 compared to AMD64 (Intel). I’ve switched to using the striped counter from xsync instead, which should improve performance on ARM64 and I’ve already tested it on Aliyun Yitian ECS. |
|
Yes, theine has become much faster. |
related golang/go#60905 (comment) |
|
@vmg This PR improves read performance and scalability, you might consider incorporating some of the changes into Vitess Theine. |
|
Thank you for the heads up @Yiling-J! I'm back from my holiday and will try to backport this in the upcoming weeks. :) |
time.Unix(0, ts).UTC()->time.Unix(0, ts),UTC()will strip monotonic clock, and makeSinceslow)before:
after: