-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[QoI][stdlib] Improve some random call sites #20320
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
Conversation
@swift-ci please benchmark |
Build comment file:Code size: -Osize
How to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the regressions before you merge the PR. Noise: Sometimes the performance results (not code size!) contain false alarms. Unexpected regressions which are marked with '(?)' are probably noise. If you see regressions which you cannot explain you can try to run the benchmarks again. If regressions still show up, please consult with the performance team (@eeckstein). Hardware Overview
|
@swift-ci please test |
Build failed |
Failure is unrelated, let's run the tests again. @swift-ci please test |
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.
Looking good!
stdlib/public/core/Collection.swift
Outdated
startIndex, | ||
offsetBy: numericCast(random) | ||
offsetBy: random | ||
) |
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.
Microscopic nit: we can collapse these four lines into a single line now
@harlanhaskins is this ready for merge? |
Looks good to me! @swift-ci please smoke test and merge |
@harlanhaskins I’m not sure CI picked that comment up. Try again? |
@swift-ci please smoke test |
Thanks, @Azoy! |
Instead of using generator directly, use
Int.random(in:)
to improve clarity of these call sites.cc: @lorentey
Can you also kick off benchmarks as well?