-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
[stdlib] De-gyb Sort #17954
[stdlib] De-gyb Sort #17954
Conversation
@swift-ci please smoke benchmark |
@swift-ci please smoke test compiler performance |
@swift-ci please test |
Build failed |
Build failed |
Build comment file:Compilation-performance test failed |
Build comment file:Build failed before running benchmark. |
Helps if you commit the CMake change. |
@swift-ci please test |
@swift-ci please smoke benchmark |
Build comment file:Optimized (O)Regression (15)
Improvement (10)
No Changes (433)
Hardware Overview
|
@swift-ci please smoke test compiler performance |
@swift-ci please benchmark |
Build comment file:Optimized (O)Regression (10)
Improvement (11)
No Changes (439)
Unoptimized (Onone)Regression (10)
Improvement (22)
No Changes (428)
Hardware Overview
|
…Um. Is this worth the regression? NibbleSort and PhoneBook are both actual sorting tests, no? |
NibbleSort is kind of a pathological case, but we do care about PhoneBook. But it's easier to debug why the hand-specialization is needed after landing on master. We can revert if it can't be figured out in time for 5.0. I raised https://bugs.swift.org/browse/SR-8267 to track that. |
Also, note this isn't just for the sake of de-gybbing, this was code bloat in the std lib (two near-identical copies of multiple functions) |
Taking another run at this. Should improve stdlib code size, so long as it doesn't hurt the benchmarks.
Also moves the
sort
/sorted
definitions on the protocols over to live with the sort implementation itself.