-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
<random>
: Implement Lemire's fast integer generation
#3012
<random>
: Implement Lemire's fast integer generation
#3012
Conversation
<random>
: Implement Lemire's fast integer generation
These results are interesting...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose that this is a great step towards DevCom-879048.
I added xoshiro256** and xoshiro128** for comparison, which are fast and have small states. Benchmark results on MattStephanson@0156b8d (reformatted): AMD Ryzen 7 5700X, x86
AMD Ryzen 7 5700X, x64
Intel Core i5-8400, x86
Intel Core i5-8400, x64
|
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this approach looks good to me! (I skipped the int128 changes as I assume we'll want to land @frederick-vs-ja's #3036 first.)
The benchmark results look convincing enough to me, especially @statementreply's cases. 😻
Some of @StephanTLavavej's feedback affects codegen, so here are my updated benchmark results. I think they're pretty similar to what I originally posted. x86
x64
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
I see, a damaged commit was amended, but the history wasn't further rewritten. |
@strega-nil-ms I pushed minor changes to the benchmark after validating that it still builds and runs properly. Thanks for adding it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems correct to me, as far as my understanding of goes. I think this is ready.
I've pushed a merge with ✅ I double-checked that this PR should have no impact on modules. |
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
Thank you for improving the performance of This will be available in either VS 2022 17.5 Preview 1 or Preview 2 (depending on internal merge logistics; the Changelog will record our current expectation). |
Co-authored-by: Nicole Mazzuca <mazzucan@outlook.com> Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
Implements @lemire's "Fast Random Integer Generation in an Interval", https://dl.acm.org/doi/10.1145/3230636 and https://arxiv.org/abs/1805.10941. Fixes #178.
I'm not happy with the x86 or LCG performance, but I've been tinkering with it for weeks and haven't been able to improve it further. I'm using a Surface Pro 8, i5-1135G7. It's plugged in and set to "Best Performance", but I'm otherwise not very knowledgeable about how to run good microbenchmarks. If anyone has any thoughts, I'd love to hear them.
Benchmark code
Benchmark results
x86
x64