Skip to content

[benchmark] Add SplitMix64 PRNG #24357

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

Merged
merged 1 commit into from
Apr 29, 2019
Merged

Conversation

palimondo
Copy link
Contributor

Very fast pseudorandom number generator with 64 bits of state, conforming to RandomNumberGenerator protocol, passing BigCrush.

See #24206 for use case.

Very fast pseudorandom number generator with 64 bits of state, conforming to `RandomNumberGenerator` protocol, passing BigCrush.
@palimondo palimondo requested review from atrick and eeckstein April 28, 2019 17:26
@palimondo
Copy link
Contributor Author

cc @rajbarik

Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

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

Cool. Thanks.

@palimondo
Copy link
Contributor Author

@swift-ci please smoke test and merge

@palimondo
Copy link
Contributor Author

@swift-ci please smoke test Linux platform

@eeckstein
Copy link
Contributor

Is this intended to be used by new benchmarks?
Any specific reason to add this now?
(I'm asking because unused code usually vanishes over time)

@atrick
Copy link
Contributor

atrick commented Apr 29, 2019

@eeckstein It's intended to replace Int.random in BucketSort.

It could also replace LFSR in TestUtils now. I added that to try to encourage benchmark authors not to use some platform specific, nondeterministic random, and to make sure all benchmarks used a fixed seed value, but it doesn't seem to be used anymore.

@lorentey should also probably use a PRNG from TestUtils in BreadCrumbs rather than rolling his own.

Of these three PNRG's, I don't have a preference. I originally wrote LFSR to be used as a benchmark itself, before the stdlib had any random API. Presumably @palimondo wanted something faster as a pure utility. I don't know if there's a reason that @lorentey rolled his own.

@palimondo
Copy link
Contributor Author

I can only speculate, but IIRC Breadcrumbs was a first benchmark to use PRNG after random unification, therefore @lorentey had to roll his own implementation. He reasonably kept it local since that LCRNG was good enough for his needs, without making any claims about its suitability for other purposes.

I’m suggesting putting SplitMix64 into shared place, because it has equally minimal implementation footprint but according to its authors it also has reasonably good quality, since it passes BigCrunch tests.

@eeckstein I’ll test if we can substitute it in Breadcrumbs without altering performance results, but as @atrick said, primary motivation was to supply a common utility for use in BucketSort now and for any future benchmarks that need PRNG.

Speed per-se wasn’t that important to me, even thought I got SplitMix64‘s impl. as a side effect of studying Xoshiro family of PRNGs, which relies on it for its seed initialization. If you want, we might also add Xoshiro256** if we want something that’s ~30% faster and has longer period, but I didn’t think it’s required by any of our current use cases. It would be additional 15–20 LOC.

@palimondo palimondo merged commit 6ca706e into swiftlang:master Apr 29, 2019
@palimondo palimondo deleted the prng-splitmix64 branch April 29, 2019 06:04
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.

3 participants