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

Golang Benchmarks don't take Package Name into consideration #264

Open
gaby opened this issue Sep 15, 2024 · 3 comments
Open

Golang Benchmarks don't take Package Name into consideration #264

gaby opened this issue Sep 15, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@gaby
Copy link

gaby commented Sep 15, 2024

When running benchmarks in a repo with a lot of packages, if two benchmarks have the same name this github-action doesn't take into account the package name cause the comparison to fail randomly.

This can be seen on this run: https://github.com/gofiber/fiber/actions/runs/10873073504

Where BenchmarkAppendMsgitem shows up multiple times with different values even though they are in different packages.

@ktrz
Copy link
Member

ktrz commented Sep 15, 2024

Hey @gaby
Thanks for reporting this! I wonder how we could deal with this. I think it will create a similar issue of backward compatibility as we faced with PR #177

We could append the package name to the name of the benchmark so it would be eg

BenchmarkAppendMsgitem (github.com/gofiber/fiber/v3/middleware/cache)

This should create a unique name to compare
As a remedy to not break existing metrics we could treat benchmarks with only one package as special cases and not append the package name in those cases. Or add both cases: old (without suffix) and new (with suffix) so that it can be then dropped in the next release. WDYT?

@ktrz ktrz added the bug Something isn't working label Sep 15, 2024
@gaby
Copy link
Author

gaby commented Sep 15, 2024

@ktrz Adding both cases is probably the way to go now. Other option I can think is having a config option to specify if you want benchmarks with suffix or not.

@gaby
Copy link
Author

gaby commented Oct 8, 2024

@ktrz Any progress on this issue? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

No branches or pull requests

2 participants