Change ArraySortHelper to use Comparison<T>#8504
Conversation
The Array/List.Sort overloads that take a Comparison<T> have worse performance than the ones that take a IComparer<T>. That's because sorting is implemented around IComparer<T> and a Comparison<T> needs to be wrapped in a comparer object to be used. At the same time, interface calls are slower than delegate calls so the existing implementation doesn't offer the best performance even when the IComparer<T> based overloads are used. By changing the implementation to use Comparison<T> we avoid interface calls in both cases. When IComparer<T> overloads are used a Comparison<T> delegate is created from IComparer<T>.Compare, that's an extra object allocation but sorting is faster and we avoid having two separate sorting implementations.
|
|
||
| IntrospectiveSort(keys, index, length, comparer); | ||
| #else | ||
| if (BinaryCompatibility.TargetsAtLeast_Desktop_V4_5) |
There was a problem hiding this comment.
@jkotas @AlexGhiondea BinaryCompatibility has been removed from this repository but there are still references to it in ifdefed out code. Should such code be removed as well? We'd get rid of DepthLimitedQuickSort which isn't used anywhere in CoreCLR.
There was a problem hiding this comment.
Yes, it is fine to delete the code under #if !FEATURE_CORECLR. https://github.com/dotnet/coreclr/issues/972#issuecomment-257633318
|
Looks pretty good to me. |
These are never used in CoreCLR
|
I suspect that there's more that can be done about the default comparer case, e.g. caching the delegate built from the default comparer. I doubt it's worth it and can be done in a separate PR anyway. I didn't modify I also didn't modify |
|
Thanks! |
|
Would you mind porting the change to CoreRT as well? |
Change ArraySortHelper to use Comparison<T> Commit migrated from dotnet/coreclr@2d888b5
The Array/List.Sort overloads that take a
Comparison<T>have worse performance than the ones that take aIComparer<T>. That's because sorting is implemented aroundIComparer<T>and aComparison<T>needs to be wrapped in a comparer object to be used.At the same time, interface calls are slower than delegate calls so the existing implementation doesn't offer the best performance even when the
IComparer<T>based overloads are used.By changing the implementation to use
Comparison<T>we avoid interface calls in both cases.When
IComparer<T>overloads are used aComparison<T>delegate is created fromIComparer<T>.Compare, that's an extra object allocation but sorting is faster and we avoid having two separate sorting implementations.Fixes #8481