Lazily construct collection shrink candidates - #4152
Open
mizchi wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Improves QuickCheck shrinking performance for large collections by deferring per-element shrink-chain construction until after structural (remove-chunk / empty) candidates are exhausted, while also reordering several shrink candidate streams to prefer simpler values (notably empty collections and zero) earlier.
Changes:
- Make Array/List shrinking return the empty candidate first without traversing/copying the original collection, and lazily construct sub-term shrink iterators.
- Reorder numeric/option shrink candidates to try simpler values first (e.g.,
0/Noneearlier) and avoid duplicate0candidates for decimal shrinking. - Add/adjust regression tests and add a benchmark test for first-candidate shrinking on 10k-element collections.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| quickcheck/shrink/utils.mbt | Adds fast-path empty Array candidate and removes duplicate decimal 0 candidates. |
| quickcheck/shrink/shrink.mbt | Reorders integer/BigInt/byte shrink candidates to prefer 0 first. |
| quickcheck/shrink/shrink_test.mbt | Updates snapshots for the new numeric shrink ordering and double shrink output. |
| quickcheck/shrink/composite.mbt | Reorders Option shrinking (None first) and defers Array sub-term shrink construction lazily. |
| quickcheck/shrink/composite_test.mbt | Updates snapshots to match new composite shrink ordering (option/result/array/tuples). |
| quickcheck/shrink/collection.mbt | Makes List shrinking emit empty first and defers length/sub-term shrink construction until needed. |
| quickcheck/shrink/collection_test.mbt | Updates collection shrink snapshots and adds a single-element list → empty regression test. |
| quickcheck/shrink_collection_bench_test.mbt | Adds benchmark tests for first shrink candidate on 10k-element Array/List. |
| quickcheck/README.mbt.md | Updates documentation example output to reflect new shrink ordering. |
| quickcheck/moon.pkg | Adds bench import for test-only benchmark compilation. |
| quickcheck/driver_test.mbt | Adds a regression test ensuring numeric shrinking reaches the boundary within default shrink budget. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Defers construction of per-element shrink iterator chains until structural collection candidates have been exhausted. It also returns the empty Array/List candidate without traversing or copying the original collection.
For obtaining the first shrink candidate from a 10,000-element collection on native release:
This is currently stacked on #4149 because that PR establishes empty-first List shrinking. Once #4149 merges, this PR’s diff will reduce to the lazy construction change and its benchmark.
Validation:
moon test quickcheck/shrink --target all,moon check --target all, andmoon info.