-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Streamline integer-range randomElement. #16501
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
Streamline integer-range randomElement. #16501
Conversation
This is a small performance win; mainly I'm interested in simplifying the code so that there are fewer weird corners for bugs to creep in. Nonetheless, it seems to be about 5% faster with the (fast, dumb) LCG generator.
@swift-ci Please test. |
Tagging @Azoy because I can't directly add him as a reviewer. |
Im not sure if tests fail on warnings, but since open range delta is never mutated, it will spit out a warning wanting |
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 is great, my wrapping arithmetic skills did not get me here 👏👏👏
@Azoy At least running locally it does not cause a real problem, but if we want to try to get rid of warnings, I'll put some hack in here. |
@natecook1000 Steve's One Key Fact Of Wrapping Arithmetic: If:
you are guaranteed to get the right answer in the end, no matter how many times you overflowed in-between.
|
@swift-ci please smoke benchmark |
Build comment file:Optimized (O)Regression (21)
Improvement (23)
No Changes (391)
Unoptimized (Onone)Regression (24)
Improvement (25)
No Changes (386)
Hardware Overview
|
This is a small performance win; mainly I'm interested in simplifying the code so that there are fewer weird corners for bugs to creep in. Nonetheless, it seems to be about 5% faster with the (fast, dumb) LCG generator.
This is likely to be a larger perf win when working with hypothetical integer types bigger than the builtin ones, because it removes a lot of stuff that the optimizer is unlikely to be able to reason about with more general types.