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

fix: actually limit the time taken to shrink #202

Merged
merged 1 commit into from
Jan 13, 2024

Conversation

Ekleog
Copy link
Contributor

@Ekleog Ekleog commented Jan 12, 2024

I have a case where there is a 4kiB input that needs shrinking. Bolero then wants to run 4000 times the apply_transforms function before checking for shrink time.

Unfortunately, each apply_transforms seems to run my test closure 2000 times, and each test closure takes 50-100ms to run.

All together, bolero would need 9 days before checking for the shrink time limit for the first time.

With this change, bolero now checks more often the shrink time, which actually leads to me getting an error message and not what looks like a deadlock :)


I'll also ask, do you think you could release 0.10.1 with the fixes? It's probably not the first time I've hit the bug, but until now I've been ignoring it due to it looking like a deadlock.

I have a case where there is a 4kiB input that needs shrinking. Bolero
then wants to run 4000 times the apply_transforms function before
checking for shrink time.

Unfortunately, each apply_transforms seems to run my test closure 2000
times, and each test closure takes 50-100ms to run.

All together, bolero would need 9 days before checking for the shrink
time limit for the first time.

With this change, bolero now checks more often the shrink time, which
actually leads to me getting an error message and not what looks like a
deadlock :)
@camshaft camshaft merged commit c5edc8e into camshaft:master Jan 13, 2024
9 checks passed
@Ekleog
Copy link
Contributor Author

Ekleog commented Jan 16, 2024

@camshaft Would it be possible for you to release 0.10.1 of bolero-engine with this change? I've hit the issue again, and again it took me quite a while to understand that it's not a deadlock — I don't always develop with path dependencies on as I'm trying to not commit them 😅

@camshaft
Copy link
Owner

Yes, sorry, I will do a release tomorrow morning (~12 hrs from now). I meant to when I merged this but had a few things come up.

@camshaft
Copy link
Owner

@Ekleog
Copy link
Contributor Author

Ekleog commented Jan 16, 2024

Awesome, thank you! ❤️

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