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

Add basic benchmarking using criterion #62

Merged
merged 2 commits into from
Oct 1, 2023

Conversation

emwalker
Copy link
Collaborator

@emwalker emwalker commented Sep 29, 2023

This PR adds a simple benchmarking harness as an example of the kind of thing that can be done using a library called criterion.

The criterion library is a well-known Rust library that is often used for this kind of thing. The benchmark in this PR is just an example; hopefully it will be straightforward to extend to more substantial cases. Its comparisons from run to run are a little noisy, but I suspect this can be dialed in. Examples of people using criterion here and here.

To run the suite, do:

$ make bench
$ cargo bench

I'll leave this PR in draft since I didn't discuss it first.

2023-09-29_17-59

2023-09-29_17-53

2023-09-29_17-56

@jaytaph
Copy link
Member

jaytaph commented Sep 30, 2023

Awesome. Looking forward to merge

@emwalker
Copy link
Collaborator Author

This PR should be ready to review and merge now. The PR is just a placeholder. Feel free to make significant changes, or refactor, or even revert the functionality if it doesn't prove useful.

I adjusted the settings in an attempt to make the results a little more consistent from one run to another. The lesson is that you have to run cargo bench several times in order to get a picture of where things are at.

2023-09-30_06-03

@emwalker emwalker marked this pull request as ready for review September 30, 2023 12:11
@jaytaph
Copy link
Member

jaytaph commented Oct 1, 2023

@emwalker Thanks for the PR.

Is this something we should run on each PR and deploy the results? (we have the *.developer.gosub.io domain for this for now)?

@jaytaph jaytaph merged commit b2a1994 into gosub-io:main Oct 1, 2023
4 checks passed
@emwalker
Copy link
Collaborator Author

emwalker commented Oct 1, 2023

I doubt the benchmark results are deterministic enough to block a PR on at this point. But I can imagine in the future failing a PR check if some time threshold is passed. I suspect the actual benchmark might need to be modified if it was going to be used for a PR check. So perhaps good to leave for a future investigation.

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.

2 participants