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

missleading benchmark results #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KingOfBaboon
Copy link

@KingOfBaboon KingOfBaboon commented Sep 18, 2023

Hi,
it's not a real PR but some sample code to demonstrate my idea.
First, thanks for this great project.

I'm trying to include this lib in my project and I was doing some benchmarking comparing native map vs bigcache get for bytes.
I was getting 11ns/op for native map and getting 101ns/op for bigcache on a 4,000,000 string keys. It's 10x slower than native map.

Which is pretty off from your benchmarking result:

BenchmarkMapGetForBytes-8                    	23130621	       207.2 ns/op	      23 B/op	       1 allocs/op
BenchmarkBigCacheGetForBytes-8               12868870	       346.8 ns/op	     151 B/op	       3 allocs/op

your results indicate 1.6x slower than native map.

looking into your benchmark code, I realize this line id := rand.Intn(maxEntryCount) actually takes most of the time in the benchmark loop. So the current results are misleading to some degree.

it's not easy to exclude certain code in the loop with timers, since the timer function is also slow compared to fast operations like map lookup. I included a baseline test to give us the time spent on the rand.Intn. There should be better ways to narrow down the timer to focus on our target code.

try to run the sample code and you'll know what I mean.

…ading

the rand.Intn use more time than the actual code we want to measure
@KingOfBaboon KingOfBaboon changed the title some sample code to show why current benchmark test results are misle… missleading benchmark results Sep 18, 2023
@@ -230,8 +238,8 @@ func BigCacheGet[T any](cs constructor[T], b *testing.B) {

hitCount := 0
for i := 0; i < b.N; i++ {
id := rand.Intn(maxEntryCount)
data, _ := cache.Get(key(id))
//id := rand.Intn(maxEntryCount)
Copy link
Contributor

Choose a reason for hiding this comment

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

how about generating ids upfront ant then just reading them as ids[i]? That should be much faster

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.

None yet

2 participants