-
Notifications
You must be signed in to change notification settings - Fork 70
CI benchmarking suite #533
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
base: less-getptr
Are you sure you want to change the base?
Conversation
I added @ericphanson's GC benchmarks here too via git cherry-pick |
Here's the new results, comparing #529 with main:
Does this make sense? A bit slower GC, but now with thread safety? |
Hey @cjdoris I updated to the proper workflow version of AirspeedVelocity.jl: https://discourse.julialang.org/t/easy-github-benchmarking-with-new-airspeedvelocity-jl/129327. It uses a job summary. You can see the benchmark results here. Using this, one can quickly check this table on a PR to validate if a change hasn't caused a slowdown or anything. It also now reports memory and allocations which has been useful! (We can also switch to the GitHub comment version if you prefer.) Does this sound good to you? |
This comment was marked as outdated.
This comment was marked as outdated.
I'm rebasing on #618 to see if that fixes the bus error in the benchmark. The benchmark is heavy on the GC so it makes sense it would hit this kinda stuff. Edit: yep! |
(Merge after #618)
This defines a simple BenchmarkTools.jl suite based on BENCHMARKS.md for the actual benchmark code. Feel free to add other things we should be keeping track of or maybe some integration benchmarks.
This also uses AirspeedVelocity.jl for automatic CI output which parses the output of the benchmark results. Basically every pull request will get a comment like SymbolicML/DynamicExpressions.jl#94 (comment) with all of the performance info. It also measures the change in load time which is quite useful.
I find it nice for catching performance regressions in PRs.