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

Run benchmarks on pull requests #976

Open
tommy-waltmann opened this issue Jun 8, 2022 · 2 comments
Open

Run benchmarks on pull requests #976

tommy-waltmann opened this issue Jun 8, 2022 · 2 comments

Comments

@tommy-waltmann
Copy link
Collaborator

Is your feature request related to a problem? Please describe.

In the past, we have accidentally created performance regressions (#694, fixed in #843) when adding features to freud. Our 2.4 release added a custom build type to get around a documentation issue with scikit-build, which accidentally removed some important compiler optimization flags.

Describe the solution you'd like

Have the benchmarks run on PRs before merging them to make sure we haven't accidentally added any performance regressions. This can be done be adding the benchmark label to a pull request.

Describe alternatives you've considered

We could maybe run the benchmarks manually, but that is a huge hassle and I doubt that would be done reliably.

Additional context

Using the extra CI time is now feasible because our CI is in the process of being entirely converted to github actions (#951), so we have more time and the ability to host our own CI runners.

@vyasr
Copy link
Collaborator

vyasr commented Jun 8, 2022

I would be happy to see benchmarks run this way, but I would caution against doing so unless we host our own dedicated runners. Our previous experiences (across multiple packages, including freud but also e.g. signac) is that running benchmarks on shared CI machines is so noisy as to be basically useless for these purposes.

@joaander
Copy link
Member

joaander commented Jun 9, 2022

Yes, we will use self-hosted runners on dedicated VMs running on Jetstream2.

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

No branches or pull requests

3 participants