From fd2357370b112580b9dbcfae2d2d621d0c18d769 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Thu, 23 Jan 2020 18:12:37 -0800 Subject: [PATCH 1/2] * Improve handling of the _Count variable in _Sort_unchecked that we were forced to initialize in the constexpr algorithms review by eliminating the variable entirely. * Consistently test _ISORT_MAX with <=, and make that an _INLINE_VAR constexpr variable. * Remove _Count guards of 1 in front of _Insertion_sort_unchecked for consistency. I did performance testing and there was no measurable difference in keeping this check, and it's more code to reason about. * Avoid needless casts of _ISORT_MAX given that it is now a constexpr constant. --- stl/inc/algorithm | 38 +++++++++++++++++++------------------- stl/inc/execution | 15 +++++---------- 2 files changed, 24 insertions(+), 29 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 77777c5b4b5..faa110bd91c 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -27,7 +27,7 @@ _END_EXTERN_C _STD_BEGIN // COMMON SORT PARAMETERS -const int _ISORT_MAX = 32; // maximum size for insertion sort +_INLINE_VAR constexpr int _ISORT_MAX = 32; // maximum size for insertion sort // STRUCT TEMPLATE _Optimistic_temporary_buffer template @@ -3520,11 +3520,22 @@ _CONSTEXPR20 pair<_RanIt, _RanIt> _Partition_by_median_guess_unchecked(_RanIt _F template _CONSTEXPR20_ICE void _Sort_unchecked(_RanIt _First, _RanIt _Last, _Iter_diff_t<_RanIt> _Ideal, _Pr _Pred) { // order [_First, _Last), using _Pred - _Iter_diff_t<_RanIt> _Count = 0; - while (_ISORT_MAX < (_Count = _Last - _First) && 0 < _Ideal) { // divide and conquer by quicksort + for (;;) { + if (_Last - _First <= _ISORT_MAX) { // small + _Insertion_sort_unchecked(_First, _Last, _Pred); + return; + } + + if (_Ideal <= 0) { // heap sort if too many divisions + _Make_heap_unchecked(_First, _Last, _Pred); + _Sort_heap_unchecked(_First, _Last, _Pred); + return; + } + + // divide and conquer by quicksort auto _Mid = _Partition_by_median_guess_unchecked(_First, _Last, _Pred); - _Ideal = _Ideal / 2 + _Ideal / 4; // allow 1.5 log2(N) divisions + _Ideal = (_Ideal >> 1) + (_Ideal >> 2); // allow 1.5 log2(N) divisions if (_Mid.first - _First < _Last - _Mid.second) { // loop on second half _Sort_unchecked(_First, _Mid.first, _Ideal, _Pred); @@ -3534,13 +3545,6 @@ _CONSTEXPR20_ICE void _Sort_unchecked(_RanIt _First, _RanIt _Last, _Iter_diff_t< _Last = _Mid.first; } } - - if (_ISORT_MAX < _Count) { // heap sort if too many divisions - _Make_heap_unchecked(_First, _Last, _Pred); - _Sort_heap_unchecked(_First, _Last, _Pred); - } else if (2 <= _Count) { - _Insertion_sort_unchecked(_First, _Last, _Pred); // small - } } template @@ -3665,9 +3669,8 @@ template void _Insertion_sort_isort_max_chunks(_BidIt _First, const _BidIt _Last, _Iter_diff_t<_BidIt> _Count, _Pr _Pred) { // insertion sort every chunk of distance _ISORT_MAX in [_First, _Last) // pre: _Count == distance(_First, _Last) - constexpr auto _Diffsort_max = static_cast<_Iter_diff_t<_BidIt>>(_ISORT_MAX); - for (; _Diffsort_max < _Count; _Count -= _Diffsort_max) { // sort chunks - _First = _Insertion_sort_unchecked(_First, _STD next(_First, _Diffsort_max), _Pred); + for (; _Count <= _ISORT_MAX; _Count -= _ISORT_MAX) { // sort chunks + _First = _Insertion_sort_unchecked(_First, _STD next(_First, _ISORT_MAX), _Pred); } _Insertion_sort_unchecked(_First, _Last, _Pred); // sort partial last chunk @@ -3736,10 +3739,7 @@ void stable_sort(const _BidIt _First, const _BidIt _Last, _Pr _Pred) { const auto _ULast = _Get_unwrapped(_Last); const auto _Count = _STD distance(_UFirst, _ULast); if (_Count <= _ISORT_MAX) { - if (_Count > 1) { - _Insertion_sort_unchecked(_UFirst, _ULast, _Pass_fn(_Pred)); - } - + _Insertion_sort_unchecked(_UFirst, _ULast, _Pass_fn(_Pred)); return; } @@ -3884,7 +3884,7 @@ _CONSTEXPR20_ICE void nth_element(_RanIt _First, _RanIt _Nth, _RanIt _Last, _Pr return; // nothing to do } - while (_ISORT_MAX < _ULast - _UFirst) { // divide and conquer, ordering partition containing Nth + while (_ULast - _UFirst <= _ISORT_MAX) { // divide and conquer, ordering partition containing Nth auto _UMid = _Partition_by_median_guess_unchecked(_UFirst, _ULast, _Pass_fn(_Pred)); if (_UMid.second <= _UNth) { diff --git a/stl/inc/execution b/stl/inc/execution index 15c210a8871..e2044a5ca94 100644 --- a/stl/inc/execution +++ b/stl/inc/execution @@ -2749,15 +2749,13 @@ struct _Sort_operation { // context for background threads template /* = 0 */> void sort(_ExPo&&, const _RanIt _First, const _RanIt _Last, _Pr _Pred) noexcept /* terminates */ { // order [_First, _Last), using _Pred - using _Diff = _Iter_diff_t<_RanIt>; _Adl_verify_range(_First, _Last); - const auto _UFirst = _Get_unwrapped(_First); - const auto _ULast = _Get_unwrapped(_Last); - const _Diff _Ideal = _ULast - _UFirst; + const auto _UFirst = _Get_unwrapped(_First); + const auto _ULast = _Get_unwrapped(_Last); + const _Iter_diff_t<_RanIt> _Ideal = _ULast - _UFirst; if constexpr (remove_reference_t<_ExPo>::_Parallelize) { - constexpr auto _Diffsort_max = static_cast<_Diff>(_ISORT_MAX); size_t _Threads; - if (_Ideal > _Diffsort_max && (_Threads = __std_parallel_algorithms_hw_threads()) > 1) { + if (_Ideal > _ISORT_MAX && (_Threads = __std_parallel_algorithms_hw_threads()) > 1) { // parallelize when input is large enough and we aren't on a uniprocessor machine _TRY_BEGIN _Sort_operation _Operation(_UFirst, _Pass_fn(_Pred), _Threads, _Ideal); // throws @@ -3014,10 +3012,7 @@ void stable_sort(_ExPo&&, const _BidIt _First, const _BidIt _Last, _Pr _Pred) no const auto _ULast = _Get_unwrapped(_Last); const auto _Count = _STD distance(_UFirst, _ULast); if (_Count <= _ISORT_MAX) { - if (_Count > 1) { - _Insertion_sort_unchecked(_UFirst, _ULast, _Pass_fn(_Pred)); - } - + _Insertion_sort_unchecked(_UFirst, _ULast, _Pass_fn(_Pred)); return; } From af407330dbcbecd98cb83380e29fef5eaa544494 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Thu, 23 Jan 2020 18:59:48 -0800 Subject: [PATCH 2/2] Unbreak conditional. --- stl/inc/algorithm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index faa110bd91c..c701a3c0d8a 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -3669,7 +3669,7 @@ template void _Insertion_sort_isort_max_chunks(_BidIt _First, const _BidIt _Last, _Iter_diff_t<_BidIt> _Count, _Pr _Pred) { // insertion sort every chunk of distance _ISORT_MAX in [_First, _Last) // pre: _Count == distance(_First, _Last) - for (; _Count <= _ISORT_MAX; _Count -= _ISORT_MAX) { // sort chunks + for (; _ISORT_MAX < _Count; _Count -= _ISORT_MAX) { // sort chunks _First = _Insertion_sort_unchecked(_First, _STD next(_First, _ISORT_MAX), _Pred); } @@ -3884,7 +3884,7 @@ _CONSTEXPR20_ICE void nth_element(_RanIt _First, _RanIt _Nth, _RanIt _Last, _Pr return; // nothing to do } - while (_ULast - _UFirst <= _ISORT_MAX) { // divide and conquer, ordering partition containing Nth + while (_ISORT_MAX < _ULast - _UFirst) { // divide and conquer, ordering partition containing Nth auto _UMid = _Partition_by_median_guess_unchecked(_UFirst, _ULast, _Pass_fn(_Pred)); if (_UMid.second <= _UNth) {