-
Notifications
You must be signed in to change notification settings - Fork 514
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
perf: Run all benchmarks for shorter time #2870
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2870 +/- ##
=======================================
- Coverage 80.9% 80.8% -0.1%
=======================================
Files 124 124
Lines 23833 23833
=======================================
- Hits 19281 19280 -1
- Misses 4552 4553 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Ooh! I'd love for this to be quick enough we can run it more often. Let's see how this run goes. |
@@ -40,15 +40,5 @@ jobs: | |||
toolchain: stable | |||
- uses: boa-dev/criterion-compare-action@v3 | |||
with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to combine this all into one
Since this doesn't work in one go, I'm going to split it into two PRs. |
069e1e7
to
a23692a
Compare
Also, since the |
I just noticed that the github workflow is not switching back to the PR branch after the first invocation of |
a23692a
to
b114222
Compare
Yuck. Good catch. Fortunately we've only ever discussed changes from the first run - all the context stuff happens before the rest! |
8b7e4bf
to
84c09ee
Compare
This now runs the benchmarks in ~50 minutes instead of ~1:05. And that is with only the changed side running faster while |
93383fa
to
d38ac9b
Compare
d38ac9b
to
ce2b5a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just what we need, and all looks good to me. Thanks for the effort! Excited to see how fast it'll be once we have it in main too.
Changes
Shortens the warmup time to 1 second and measurement time to 2 seconds for all benchmarks, and prepares them to be run in one go from the workspace root. Most benchmarks have millions or billions of iterations with a 5 second measurement, which seems a bit excessive.
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes