benchmark-action / github-action-benchmark

GitHub Action for continuous benchmarking to keep performance
https://benchmark-action.github.io/github-action-benchmark/dev/bench/
MIT License
1.02k stars 152 forks source link

Running go benchmarks: Use `-count` or not? #229

Open ankon opened 9 months ago

ankon commented 9 months ago

We're setting up this action for our performance-critical go applications, and notice some variance in the results. In one situation it got so bad (or good) that PR branches now report performance regressions because the master branch by chance recorded a really good value.

I was wondering whether we can "smooth" that a bit by using -count when running the benchmarks, but am not sure how/if this action will handle that? If it "should just work", do you think it's a good idea?

ktrz commented 8 months ago

Hi @ankon

I'm not sure what do you mean by "smooth" in this context. But as long as the output of the benchmark run is in the same format it should be fine.

ankon commented 8 months ago

In the case of running using -count=10 we get this output: Basically one line identifying the benchmark, and then it repeats once for each requested count. The question is whether the action will parse this format, and understand to aggregate the data in some way (average it?)

goos: linux
goarch: amd64
pkg: github.com/.../tmp
cpu: AMD Ryzen 9 7940HS w/ Radeon 780M Graphics     
BenchmarkBranchless
BenchmarkBranchless/toUpper1
BenchmarkBranchless/toUpper1-16             88331236           13.18 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper1-16             99448578           12.76 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper1-16             96488515           12.92 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper1-16             84861313           12.38 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper1-16             82347253           12.23 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper1-16             92781146           13.04 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper1-16             97147456           12.94 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper1-16             93339778           12.69 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper1-16             85405495           13.66 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper1-16             83599527           12.25 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper2
BenchmarkBranchless/toUpper2-16             51006874           21.01 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper2-16             51493095           20.49 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper2-16             52621446           20.44 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper2-16             51095416           20.32 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper2-16             51120625           20.61 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper2-16             53014428           20.54 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper2-16             52577970           20.73 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper2-16             53823154           20.41 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper2-16             50077378           20.84 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper2-16             51951420           20.87 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper3
BenchmarkBranchless/toUpper3-16             73775959           15.37 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper3-16             77838273           15.27 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper3-16             69781880           15.46 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper3-16             65597184           15.25 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper3-16             68476039           15.30 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper3-16             71289954           15.36 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper3-16             69229252           15.00 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper3-16             80217844           15.47 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper3-16             70269200           15.31 ns/op        0 B/op          0 allocs/op
BenchmarkBranchless/toUpper3-16             71143648           15.36 ns/op        0 B/op          0 allocs/op
PASS
ktrz commented 8 months ago

I don't think the action will support that output right now. I'm not sure whether it would fail or only use the last value for each benchmark name. If you have some time to submit a fix for that then I would greatly appreciate it. Otherwise, I will try to pick it up when I manage to get some time.

Just to clarify what would be the expected result here. I suppose the solution should combine all the metrics using a weighted average (based on number of iterations per run)