-
Notifications
You must be signed in to change notification settings - Fork 421
Make fuzz runtime seconds not iterations #4179
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
Make fuzz runtime seconds not iterations #4179
Conversation
|
I've assigned @wpaulino as a reviewer! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4179 +/- ##
=======================================
Coverage 89.33% 89.33%
=======================================
Files 180 180
Lines 138055 138055
Branches 138055 138055
=======================================
+ Hits 123326 123335 +9
+ Misses 12122 12121 -1
+ Partials 2607 2599 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cbb53b6 to
3a4387d
Compare
8bfffb2 to
1ba5def
Compare
fuzz/ci-fuzz.sh
Outdated
| # Because we're fuzzing relatively few iterations, the maximum possible | ||
| # compiler optimizations aren't necessary, so we turn off LTO | ||
| sed -i 's/lto = true//' Cargo.toml | ||
| sed -i 's/codegen-units = 1//' Cargo.toml |
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.
In #3916 we made some attempts of benchmarking before we removed this line. Do you have a vague number of how much the slowdown would be?
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.
I forgot about that, but its almost certainly a tiny difference in fuzzing performance, for a bit more parallelism while compiling some crates. 🤷♂️
1ba5def to
89671a5
Compare
|
Oh, weird, I guess bumping the debian version fixed the build even with 1.75, which I thought I tested....anyway, I dropped the MSRV bump for fuzzing cause it appears to be woring now. Now this just reduces the runtime to make it more consistent across jobs. |
It's failing in CI with a linking error though? |
89671a5 to
fbdb5c5
Compare
|
Weird, it seems to be inconsistent, sometimes its fine, sometimes it fails. |
|
🔔 1st Reminder Hey @wpaulino! This PR has been waiting for your review. |
|
The |
|
🔔 2nd Reminder Hey @wpaulino! This PR has been waiting for your review. |
|
🔔 3rd Reminder Hey @wpaulino! This PR has been waiting for your review. |
|
🔔 4th Reminder Hey @wpaulino! This PR has been waiting for your review. |
|
🔔 5th Reminder Hey @wpaulino! This PR has been waiting for your review. |
|
🔔 6th Reminder Hey @wpaulino! This PR has been waiting for your review. |
We have some complexity in `ci-fuzz.sh` to limit each fuzzer to a rough runtime, but `honggfuzz` has a `--run-time` argument that we can simply use instead, which we do here.
535d546 to
8015c01
Compare
|
Hum, I guess the codegen-units = 1 is required to make the test pass, so I dropped that commit (though it shouldn't be required...). |
|
This is now only making the fuzz iterations consistent, which we should do, but is much less important. |
No description provided.