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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid pointless discards during the reuse and target phases #3862

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

Zac-HD
Copy link
Member

@Zac-HD Zac-HD commented Jan 26, 2024

This problem was jointly identified in discussion of #3827 with @tybug and @hgoldstein95. Finding this kind of performance issue without knowing to look for it is a great illustration of the value of observability!

It turns out that we're spuriously aborting many test cases with status=Overrun, due to ConjectureData.from_buffer() limiting the max length to the length of the provided bytestring unless extend=... is passed. This makes sense in the shrink phase - any longer buffer cannot be a successful shrink, so we may as well abort early - but not in the reuse or target phases.

  • The reuse phase is easy to fix: simply add the relevant argument to allow extending buffers replayed from the database - we'll get different behaviour, but get a performance gain (and maybe avoid HealthCheck.filter_too_much) by finishing this test case rather than retrying in the generate phase.
  • The target phase is a little more complicated, and ... it turns out that by the time I worked out how we'd need to thread state through to the shrinker I had 80% of a fix, so this is now a PR rather than an issue 馃槄

Tests tbd when I have some more time.

@Zac-HD Zac-HD added performance go faster! use less memory! legibility make errors helpful and Hypothesis grokable labels Jan 26, 2024
@Zac-HD Zac-HD mentioned this pull request Jan 26, 2024
4 tasks
@jobh
Copy link
Contributor

jobh commented Jan 29, 2024

Nice! Probably closes #3446, too.

@Zac-HD Zac-HD force-pushed the uncap-replay-buffers branch 2 times, most recently from d2323e5 to 91c63bb Compare January 30, 2024 06:15
@Zac-HD Zac-HD merged commit 8a9279c into HypothesisWorks:master Jan 30, 2024
47 checks passed
@Zac-HD Zac-HD deleted the uncap-replay-buffers branch January 30, 2024 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legibility make errors helpful and Hypothesis grokable performance go faster! use less memory!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants