Repository navigation
Allocate less in Seed.split - #495
xperiandri wants to merge 2 commits into
Conversation
The seeds were recorded with the implementation that took the next seed twice and mixed the two values: the two seeds of a split of four seeds, and two seeds five levels down the tree of splits. Every generator that draws several values splits the seed, and a recorded recheck depends on the seeds. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A split took the next seed twice and mixed the two values, which allocated a tuple and a seed for each step on top of the two seeds and the tuple that it returns: 192 bytes. The same two seeds are made from the same numbers without the ones in between: 96 bytes. The seeds are the same: 200 seeds and the 32 seeds five levels down the tree of splits of each (the second seed of every split included) were compared with the previous implementation, and the tests have the seeds it gave. A split is done for every element of a collection and for every bind, so this saves about 11 % of what a collection of integers allocates per element. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
The arithmetic preserves existing behavior and output order, with regression tests covering compatibility.
No open issues 🎉
What changed in this PR
Reduces allocations in Seed.split while preserving deterministic seed outputs.
Changes:
- Computes both seeds directly, avoiding intermediate seeds and tuples.
- Adds regression tests for known outputs, nested splits, and sequence continuation.
| File | Description |
|---|---|
| tests/Hedgehog.Tests/SeedTests.fs | Adds seed compatibility regression tests. |
| src/Hedgehog/Seed.fs | Removes intermediate allocations from Seed.split. |
Review effort: Balanced
moodmosaic
left a comment
There was a problem hiding this comment.
Thank you for the PR! Are we sure that the distribution is not affected by this? We run into similar issues in the past, e.g. hedgehogqa/haskell-hedgehog#198. Perhaps worth considering a dieharder battery test.
| let g = seed.Gamma | ||
| let value = seed.Value + g | ||
| let gamma = value + g | ||
| (newSeed gamma g, mixSeed value gamma) |
There was a problem hiding this comment.
Is this passing the dieharder battery tests? Something as simple as https://github.com/hedgehogqa/haskell-hedgehog/blob/master/hedgehog-dieharder/test/Dieharder.hs passed into dieharder can tell us.
Seed.splittakes the next seed twice and mixes the two values. That allocates a seed and a tuple for each step, on top of the two seeds and the tuple that it returns: 192 bytes per split. The same two seeds can be made from the same numbers without the ones in between: 96 bytes.The seeds are exactly the same:
A split is done for every element of a collection and for every
bind, so this saves about 11 % of what a collection of integers allocates per element (the generator of an integer allocates about 780 bytes, the split 192). Measured withGC.GetAllocatedBytesForCurrentThread: 192 B -> 96 B per call.It is independent of the array core for
Gen.list,Gen.arrayandGen.seq(#489), which gets the same saving per element.🤖 Generated with Claude Code