From 3c8f5779f3a54b1d949ecc48a96dac00bd44673c Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Thu, 8 Dec 2016 00:24:45 +0200 Subject: [PATCH] Change ArraySortHelper to use Comparison The Array/List.Sort overloads that take a Comparison have worse performance than the ones that take a IComparer. That's because sorting is implemented around IComparer and a Comparison 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 based overloads are used. By changing the implementation to use Comparison we avoid interface calls in both cases. When IComparer overloads are used a Comparison delegate is created from IComparer.Compare, that's an extra object allocation but sorting is faster and we avoid having two separate sorting implementations. --- .../src/System/Array.cs | 18 +------ .../Collections/Generic/ArraySortHelper.cs | 51 +++++++++++++------ .../src/System/Collections/Generic/List.cs | 3 +- 3 files changed, 37 insertions(+), 35 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/Array.cs b/src/System.Private.CoreLib/src/System/Array.cs index dfdfaa9b4e1..4a6c561eb4a 100644 --- a/src/System.Private.CoreLib/src/System/Array.cs +++ b/src/System.Private.CoreLib/src/System/Array.cs @@ -1987,23 +1987,7 @@ public static void Sort(T[] array, Comparison comparison) throw new ArgumentNullException(nameof(comparison)); } - IComparer comparer = new FunctorComparer(comparison); - Array.Sort(array, comparer); - } - - internal sealed class FunctorComparer : IComparer - { - private Comparison _comparison; - - public FunctorComparer(Comparison comparison) - { - _comparison = comparison; - } - - public int Compare(T x, T y) - { - return _comparison(x, y); - } + ArraySortHelper.Sort(array, 0, array.Length, comparison); } public static void Sort(TKey[] keys, TValue[] items) diff --git a/src/System.Private.CoreLib/src/System/Collections/Generic/ArraySortHelper.cs b/src/System.Private.CoreLib/src/System/Collections/Generic/ArraySortHelper.cs index e5e50f61e60..8ea9affc042 100644 --- a/src/System.Private.CoreLib/src/System/Collections/Generic/ArraySortHelper.cs +++ b/src/System.Private.CoreLib/src/System/Collections/Generic/ArraySortHelper.cs @@ -53,7 +53,7 @@ public static void Sort(T[] keys, int index, int length, IComparer comparer) comparer = Comparer.Default; } - IntrospectiveSort(keys, index, length, comparer); + IntrospectiveSort(keys, index, length, comparer.Compare); } catch (IndexOutOfRangeException) { @@ -84,6 +84,27 @@ public static int BinarySearch(T[] array, int index, int length, T value, ICompa #endregion + internal static void Sort(T[] keys, int index, int length, Comparison comparer) + { + Debug.Assert(keys != null, "Check the arguments in the caller!"); + Debug.Assert(index >= 0 && length >= 0 && (keys.Length - index >= length), "Check the arguments in the caller!"); + Debug.Assert(comparer != null, "Check the arguments in the caller!"); + + // Add a try block here to detect bogus comparisons + try + { + IntrospectiveSort(keys, index, length, comparer); + } + catch (IndexOutOfRangeException) + { + IntrospectiveSortUtilities.ThrowOrIgnoreBadComparer(comparer); + } + catch (Exception e) + { + throw new InvalidOperationException(SR.InvalidOperation_IComparerFailed, e); + } + } + internal static int InternalBinarySearch(T[] array, int index, int length, T value, IComparer comparer) { Debug.Assert(array != null, "Check the arguments in the caller!"); @@ -110,11 +131,11 @@ internal static int InternalBinarySearch(T[] array, int index, int length, T val return ~lo; } - private static void SwapIfGreater(T[] keys, IComparer comparer, int a, int b) + private static void SwapIfGreater(T[] keys, Comparison comparer, int a, int b) { if (a != b) { - if (comparer.Compare(keys[a], keys[b]) > 0) + if (comparer(keys[a], keys[b]) > 0) { T key = keys[a]; keys[a] = keys[b]; @@ -133,7 +154,7 @@ private static void Swap(T[] a, int i, int j) } } - internal static void IntrospectiveSort(T[] keys, int left, int length, IComparer comparer) + internal static void IntrospectiveSort(T[] keys, int left, int length, Comparison comparer) { Debug.Assert(keys != null); Debug.Assert(comparer != null); @@ -148,7 +169,7 @@ internal static void IntrospectiveSort(T[] keys, int left, int length, IComparer IntroSort(keys, left, length + left - 1, 2 * IntrospectiveSortUtilities.FloorLog2(keys.Length), comparer); } - private static void IntroSort(T[] keys, int lo, int hi, int depthLimit, IComparer comparer) + private static void IntroSort(T[] keys, int lo, int hi, int depthLimit, Comparison comparer) { Debug.Assert(keys != null); Debug.Assert(comparer != null); @@ -195,7 +216,7 @@ private static void IntroSort(T[] keys, int lo, int hi, int depthLimit, ICompare } } - private static int PickPivotAndPartition(T[] keys, int lo, int hi, IComparer comparer) + private static int PickPivotAndPartition(T[] keys, int lo, int hi, Comparison comparer) { Debug.Assert(keys != null); Debug.Assert(comparer != null); @@ -218,8 +239,8 @@ private static int PickPivotAndPartition(T[] keys, int lo, int hi, IComparer while (left < right) { - while (comparer.Compare(keys[++left], pivot) < 0) ; - while (comparer.Compare(pivot, keys[--right]) < 0) ; + while (comparer(keys[++left], pivot) < 0) ; + while (comparer(pivot, keys[--right]) < 0) ; if (left >= right) break; @@ -232,7 +253,7 @@ private static int PickPivotAndPartition(T[] keys, int lo, int hi, IComparer return left; } - private static void Heapsort(T[] keys, int lo, int hi, IComparer comparer) + private static void Heapsort(T[] keys, int lo, int hi, Comparison comparer) { Debug.Assert(keys != null); Debug.Assert(comparer != null); @@ -252,7 +273,7 @@ private static void Heapsort(T[] keys, int lo, int hi, IComparer comparer) } } - private static void DownHeap(T[] keys, int i, int n, int lo, IComparer comparer) + private static void DownHeap(T[] keys, int i, int n, int lo, Comparison comparer) { Debug.Assert(keys != null); Debug.Assert(comparer != null); @@ -264,11 +285,11 @@ private static void DownHeap(T[] keys, int i, int n, int lo, IComparer compar while (i <= n / 2) { child = 2 * i; - if (child < n && comparer.Compare(keys[lo + child - 1], keys[lo + child]) < 0) + if (child < n && comparer(keys[lo + child - 1], keys[lo + child]) < 0) { child++; } - if (!(comparer.Compare(d, keys[lo + child - 1]) < 0)) + if (!(comparer(d, keys[lo + child - 1]) < 0)) break; keys[lo + i - 1] = keys[lo + child - 1]; i = child; @@ -276,7 +297,7 @@ private static void DownHeap(T[] keys, int i, int n, int lo, IComparer compar keys[lo + i - 1] = d; } - private static void InsertionSort(T[] keys, int lo, int hi, IComparer comparer) + private static void InsertionSort(T[] keys, int lo, int hi, Comparison comparer) { Debug.Assert(keys != null); Debug.Assert(lo >= 0); @@ -289,7 +310,7 @@ private static void InsertionSort(T[] keys, int lo, int hi, IComparer compare { j = i; t = keys[i + 1]; - while (j >= lo && comparer.Compare(t, keys[j]) < 0) + while (j >= lo && comparer(t, keys[j]) < 0) { keys[j + 1] = keys[j]; j--; @@ -582,5 +603,3 @@ private static void InsertionSort(TKey[] keys, TValue[] values, int lo, int hi, } #endregion } - - diff --git a/src/System.Private.CoreLib/src/System/Collections/Generic/List.cs b/src/System.Private.CoreLib/src/System/Collections/Generic/List.cs index 4bee0d2b7ed..6deed4b5adf 100644 --- a/src/System.Private.CoreLib/src/System/Collections/Generic/List.cs +++ b/src/System.Private.CoreLib/src/System/Collections/Generic/List.cs @@ -1103,8 +1103,7 @@ public void Sort(Comparison comparison) if (_size > 0) { - IComparer comparer = Comparer.Create(comparison); - Array.Sort(_items, 0, _size, comparer); + ArraySortHelper.Sort(_items, 0, _size, comparison); } }