Skip to content

Remove dead store to _Count in _Sort_unchecked and other cleanups #449

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.

Sign up for GitHub

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

Merged
BillyONeal merged 2 commits into from
Jan 25, 2020
Merged

Remove dead store to _Count in _Sort_unchecked and other cleanups #449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd23573
* Improve handling of the _Count variable in _Sort_unchecked that we …
BillyONeal Jan 24, 2020
af40733
Unbreak conditional.
BillyONeal Jan 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions stl/inc/algorithm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <class _Diff>
Expand Down Expand Up @@ -3520,11 +3520,22 @@ _CONSTEXPR20 pair<_RanIt, _RanIt> _Partition_by_median_guess_unchecked(_RanIt _F
template <class _RanIt, class _Pr>
_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
Copy link
Member

@StephanTLavavej StephanTLavavej Jan 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I observe that _Ideal should never be negative. It's initially _ULast - _UFirst (non-negative) and then decreases via _Ideal = (_Ideal >> 1) + (_Ideal >> 2). Perhaps an _STL_INTERNAL_CHECK for negative, and an exact == 0 test otherwise? However, there's probably no codegen difference in practice, so I don't really care.

Copy link
Member Author

@BillyONeal BillyONeal Jan 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is identical except for je vs. jle
image

_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);
Expand All @@ -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 <class _RanIt, class _Pr>
Expand Down Expand Up @@ -3665,9 +3669,8 @@ template <class _BidIt, class _Pr>
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 (; _ISORT_MAX < _Count; _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
Expand Down Expand Up @@ -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;
}

Expand Down
15 changes: 5 additions & 10 deletions stl/inc/execution
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2749,15 +2749,13 @@ struct _Sort_operation { // context for background threads
template <class _ExPo, class _RanIt, class _Pr, _Enable_if_execution_policy_t<_ExPo> /* = 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
Expand Down Expand Up @@ -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;
}

Expand Down