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

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

Open
ankon opened this issue Feb 21, 2024 · 3 comments
Open

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

ankon opened this issue Feb 21, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@ankon
Copy link

ankon commented Feb 21, 2024

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
Copy link
Member

ktrz commented Mar 11, 2024

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
Copy link
Author

ankon commented Mar 11, 2024

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
Copy link
Member

ktrz commented Mar 11, 2024

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)

@ktrz ktrz added the enhancement New feature or request label Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants