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

Add shrink pass for pairs of bytes #2483

Closed
wants to merge 3 commits into from
Closed

Conversation

DRMacIver
Copy link
Member

@DRMacIver DRMacIver commented Jul 10, 2020

A thing I've been noticing in recent work is that we don't do very well at lexicographic shrinks that are obvious if we ignore the block structure but cross block boundaries. In particular there's a common pattern where we have a pair of adjacent bytes that would be trivial to shrink but we never try.

In particular the reason #2482 is currently failing is because of a failure to apply such a reduction.

This PR adds a shrink pass that applies lexicographic shrinks to pairs of bytes, which will get us unstuck in those cases. We're mainly interested in adjacent pairs of bytes, but it turns out that there are a number of special cases to worry about here where the shrinker will just take forever and a day to complete if you only focus on adjacent pairs of bytes, so we have some additional logic to skip over sequences of zeroes.

@DRMacIver
Copy link
Member Author

Ha. I remember why we don't do this now - it's really easy for this sort of transformation to get into the death-by-a-thousand-cuts performance where it makes progress and then has to loop all the way back around again before it gets to undo the damage caused by that progress. Anyway I have managed to fiddle with it to get it to the point where it avoids doing that.

@DRMacIver DRMacIver force-pushed the DRMacIver/byte-pair branch from 4b814f5 to c9940c5 Compare July 10, 2020 14:20
result = condition(x)
if result and not runtime:
runtime.append(0.0)
if timeout_after is not None and runtime:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason this moved here is because in the feature flags tests we do data generation in the condition itself, and if the timeout runs before the condition then that raises a Flaky error.

@DRMacIver DRMacIver changed the title Add shrink pass for adjacent pairs of bytes Add shrink pass for pairs of bytes Jul 10, 2020
@DRMacIver DRMacIver requested a review from Zalathar July 10, 2020 14:53
@DRMacIver
Copy link
Member Author

I'm sortof on the fence as to whether this is still a good idea. I think #2496 might do the good bits of this. I'm going to leave it unmerged for a while and revisit it once some of my upcoming work on learning shrink passes is merged and see whether this still has any benefits after that.

@DRMacIver
Copy link
Member Author

In the spirit of #2526 I'm just going to close this now. I may revisit some of the ideas in it if they prove useful later.

@DRMacIver DRMacIver closed this Jul 31, 2020
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.

1 participant