Skip to content

Optimize the is_permutation family and _Hash::operator== for multicontainers #423

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 6 commits into from
Jan 19, 2020
Merged

Optimize the is_permutation family and _Hash::operator== for multicontainers #423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fea9cba
Optimize the is_permutation family and _Hash::operator== for multicon…
BillyONeal Jan 15, 2020
3d7f760
Apply a bunch of code review comments from Casey.
BillyONeal Jan 16, 2020
ce1ff2d
clang-format
BillyONeal Jan 16, 2020
02c0d90
Apply @miscco's code deduplication idea for <xhash>.
BillyONeal Jan 18, 2020
49bac6f
Merge remote-tracking branch 'origin/master' into optimize_permutations
BillyONeal Jan 18, 2020
38453b8
Fix code review comments from Stephan: comments and add DMIs.
BillyONeal Jan 18, 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
153 changes: 117 additions & 36 deletions stl/inc/xhash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1401,14 +1401,14 @@ private:
template <class _Keyty>
_NODISCARD _Equal_range_result _Equal_range(const _Keyty& _Keyval, const size_t _Hashval) const
noexcept(_Nothrow_compare<_Traits, key_type, _Keyty>&& _Nothrow_compare<_Traits, _Keyty, key_type>) {
const size_type _Bucket = _Hashval & _Mask;
_Unchecked_const_iterator _Where = _Vec._Mypair._Myval2._Myfirst[_Bucket << 1];
const _Unchecked_const_iterator _Bucket_hi = _Vec._Mypair._Myval2._Myfirst[(_Bucket << 1) + 1];
const _Unchecked_const_iterator _End = _Unchecked_end();
const size_type _Bucket = _Hashval & _Mask;
_Unchecked_const_iterator _Where = _Vec._Mypair._Myval2._Myfirst[_Bucket << 1];
const _Unchecked_const_iterator _End = _Unchecked_end();
if (_Where == _End) {
return {_End, _End, 0};
}

const _Unchecked_const_iterator _Bucket_hi = _Vec._Mypair._Myval2._Myfirst[(_Bucket << 1) + 1];
for (; _Traitsobj(_Traits::_Kfn(*_Where), _Keyval); ++_Where) {
if (_Where == _Bucket_hi) {
return {_End, _End, 0};
Expand Down Expand Up @@ -1915,6 +1915,117 @@ protected:
return _List._Getal();
}

struct _Multi_equal_check_result {
bool _Equal_possible = false;
_Unchecked_const_iterator _Subsequent_first{}; // only useful if _Equal_possible
};

_NODISCARD _Multi_equal_check_result _Multi_equal_check_equal_range(
const _Hash& _Right, _Unchecked_const_iterator _First1) const {
// check that an equal_range of elements starting with *_First1 are a permutation of the corresponding
// equal_range of elements in _Right
auto& _Keyval = _Traits::_Kfn(*_First1);
// find the start of the matching run in the other container
const size_t _Hashval = _Right._Traitsobj(_Keyval);
const size_type _Bucket = _Hashval & _Right._Mask;
auto _First2 = _Right._Vec._Mypair._Myval2._Myfirst[_Bucket << 1];
if (_First2 == _Right._Unchecked_end()) {
// no matching bucket, therefore no matching run
return {};
}

const auto _Bucket_hi = _Right._Vec._Mypair._Myval2._Myfirst[(_Bucket << 1) + 1];
for (; _Right._Traitsobj(_Traits::_Kfn(*_First2), _Keyval); ++_First2) {
// find first matching element in _Right
if (_First2 == _Bucket_hi) {
return {};
}
}

_Unchecked_const_iterator _Left_stop_at;
if
_CONSTEXPR_IF(_Traits::_Standard) {
_Left_stop_at = _Unchecked_end();
}
else {
// check the first elements for equivalence when !_Standard
if (_Right._Traitsobj(_Keyval, _Traits::_Kfn(*_First2))) {
return {};
}

const size_t _LHashval = _Traitsobj(_Keyval);
const size_type _LBucket = _LHashval & _Mask;
const auto _LBucket_hi = _Vec._Mypair._Myval2._Myfirst[(_LBucket << 1) + 1];
_Left_stop_at = _LBucket_hi;
++_Left_stop_at;
}

// trim matching prefixes
while (*_First1 == *_First2) {
// the right equal_range ends at the end of the bucket or on the first nonequal element
bool _Right_range_end = _First2 == _Bucket_hi;
++_First2;
if (!_Right_range_end) {
_Right_range_end = _Right._Traitsobj(_Keyval, _Traits::_Kfn(*_First2));
}

// the left equal_range ends at the end of the container or on the first nonequal element
++_First1;
const bool _Left_range_end = _First1 == _Left_stop_at || _Traitsobj(_Keyval, _Traits::_Kfn(*_First1));

if (_Left_range_end && _Right_range_end) {
// the equal_ranges were completely equal
return {true, _First1};
}

if (_Left_range_end || _Right_range_end) {
// one equal_range is a prefix of the other; not equal
return {};
}
}

// found a mismatched element, find the end of the equal_ranges and dispatch to _Check_match_counts
auto _Last1 = _First1;
auto _Last2 = _First2;
for (;;) {
bool _Right_range_end = _Last2 == _Bucket_hi;
++_Last2;
if (!_Right_range_end) {
_Right_range_end = _Right._Traitsobj(_Keyval, _Traits::_Kfn(*_Last2));
}

++_Last1;
const bool _Left_range_end = _Last1 == _Left_stop_at || _Traitsobj(_Keyval, _Traits::_Kfn(*_Last1));

if (_Left_range_end && _Right_range_end) {
// equal_ranges had the same length, check for permutation
return {_Check_match_counts(_First1, _Last1, _First2, _Last2, equal_to<>{}), _Last1};
}

if (_Left_range_end || _Right_range_end) {
// different number of elements in the range, not a permutation
return {};
}
}
}

_NODISCARD bool _Multi_equal(const _Hash& _Right) const {
static_assert(_Traits::_Multi, "This function only works with multi containers");
_STL_INTERNAL_CHECK(this->size() == _Right.size());
const auto _Last1 = _Unchecked_end();
auto _First1 = _Unchecked_begin();
while (_First1 != _Last1) {
const auto _Result = _Multi_equal_check_equal_range(_Right, _First1);
if (!_Result._Equal_possible) {
return false;
}

_First1 = _Result._Subsequent_first;
}

return true;
}

#ifdef _ENABLE_STL_INTERNAL_CHECK
public:
void _Stl_internal_check_container_invariants() const noexcept {
Expand Down Expand Up @@ -2018,23 +2129,7 @@ _NODISCARD bool _Hash_equal_elements(const _Hash<_Traits>& _Left, const _Hash<_T

template <class _Traits>
_NODISCARD bool _Hash_equal_elements(const _Hash<_Traits>& _Left, const _Hash<_Traits>& _Right, true_type) {
const auto _Left_end = _Left._Unchecked_end();
for (auto _Next1 = _Left._Unchecked_begin(); _Next1 != _Left_end;) { // look for elements with equivalent keys
const auto& _Keyval = _Traits::_Kfn(*_Next1);
const size_t _Lhashval = _Left._Traitsobj(_Keyval);
const size_t _Rhashval = _Right._Traitsobj(_Keyval); // P0809 prohibits reusing _Lhashval
const auto _Lrange = _Left._Equal_range(_Keyval, _Lhashval);
const auto _Rrange = _Right._Equal_range(_Keyval, _Rhashval);

if (_Lrange._Distance != _Rrange._Distance
|| !_Is_permutation_unchecked(_Lrange._First, _Lrange._Last, _Rrange._First, equal_to<>{})) {
return false;
}

_Next1 = _Lrange._Last; // continue just past range
}

return true;
return _Left._Multi_equal(_Right);
}
#endif // !_HAS_IF_CONSTEXPR

Expand All @@ -2046,21 +2141,7 @@ _NODISCARD bool _Hash_equal(const _Hash<_Traits>& _Left, const _Hash<_Traits>& _

#if _HAS_IF_CONSTEXPR
if constexpr (_Traits::_Multi) {
const auto _Left_end = _Left._Unchecked_end();
for (auto _Next1 = _Left._Unchecked_begin(); _Next1 != _Left_end;) { // look for elements with equivalent keys
const auto& _Keyval = _Traits::_Kfn(*_Next1);
const size_t _Lhashval = _Left._Traitsobj(_Keyval);
const size_t _Rhashval = _Right._Traitsobj(_Keyval); // P0809 prohibits reusing _Lhashval
const auto _Lrange = _Left._Equal_range(_Keyval, _Lhashval);
const auto _Rrange = _Right._Equal_range(_Keyval, _Rhashval);

if (_Lrange._Distance != _Rrange._Distance
|| !_Is_permutation_unchecked(_Lrange._First, _Lrange._Last, _Rrange._First, equal_to<>{})) {
return false;
}

_Next1 = _Lrange._Last; // continue just past range
}
return _Left._Multi_equal(_Right);
} else {
for (const auto& _LVal : _Left) {
// look for element with equivalent key
Expand Down
112 changes: 77 additions & 35 deletions stl/inc/xutility
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4658,18 +4658,6 @@ template <class _ExPo, class _FwdIt, class _Ty, _Enable_if_execution_policy_t<_E
_NODISCARD _FwdIt find(_ExPo&& _Exec, _FwdIt _First, const _FwdIt _Last, const _Ty& _Val) noexcept; // terminates
#endif // _HAS_CXX17

// FUNCTION TEMPLATE _Find_pr
template <class _InIt, class _Ty, class _Pr>
_InIt _Find_pr(_InIt _First, _InIt _Last, const _Ty& _Val, _Pr _Pred) { // find first matching _Val, using _Pred
for (; _First != _Last; ++_First) {
if (_Pred(*_First, _Val)) {
break;
}
}

return _First;
}

// FUNCTION TEMPLATE count
template <class _InIt, class _Ty>
_NODISCARD _Iter_diff_t<_InIt> count(const _InIt _First, const _InIt _Last, const _Ty& _Val) {
Expand All @@ -4694,10 +4682,20 @@ _NODISCARD _Iter_diff_t<_FwdIt> count(
_ExPo&& _Exec, const _FwdIt _First, const _FwdIt _Last, const _Ty& _Val) noexcept; // terminates
#endif // _HAS_CXX17

// FUNCTION TEMPLATE _Count_pr
// FUNCTION TEMPLATE is_permutation
Copy link
Member

@StephanTLavavej StephanTLavavej Jan 18, 2020

Choose a reason for hiding this comment

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

FYI, #279 tracks a possible improvement to is_permutation, although it isn't necessary to implement at this time.

Copy link
Member Author

@BillyONeal BillyONeal Jan 18, 2020

Choose a reason for hiding this comment

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

I do agree that looks interesting but would prefer that be a separate change (as it'll need its own perf tests and similar justification)

template <class _InIt, class _Ty, class _Pr>
_Iter_diff_t<_InIt> _Count_pr(_InIt _First, _InIt _Last, const _Ty& _Val, _Pr _Pred) {
// count elements that match _Val, using _Pred
_NODISCARD _InIt _Find_pr(_InIt _First, const _InIt _Last, const _Ty& _Val, _Pr _Pred) {
for (; _First != _Last; ++_First) {
if (_Pred(*_First, _Val)) {
break;
}
}

return _First;
}

template <class _InIt, class _Ty, class _Pr>
_NODISCARD _Iter_diff_t<_InIt> _Count_pr(_InIt _First, const _InIt _Last, const _Ty& _Val, _Pr _Pred) {
_Iter_diff_t<_InIt> _Count = 0;

for (; _First != _Last; ++_First) {
Expand All @@ -4709,7 +4707,7 @@ _Iter_diff_t<_InIt> _Count_pr(_InIt _First, _InIt _Last, const _Ty& _Val, _Pr _P
return _Count;
}

// FUNCTION TEMPLATE _Trim_matching_suffixes
#if !_HAS_IF_CONSTEXPR
template <class _FwdIt1, class _FwdIt2, class _Pr>
void _Trim_matching_suffixes(_FwdIt1&, _FwdIt2&, _Pr, forward_iterator_tag, forward_iterator_tag) {
// trim matching suffixes, forward iterators (do nothing)
Expand All @@ -4727,39 +4725,53 @@ void _Trim_matching_suffixes(
++_Last1;
++_Last2;
}
#endif // !_HAS_IF_CONSTEXPR

// FUNCTION TEMPLATE _Check_match_counts
template <class _FwdIt1, class _FwdIt2, class _Pr>
bool _Check_match_counts(_FwdIt1 _First1, _FwdIt1 _Last1, _FwdIt2 _First2, _FwdIt2 _Last2, _Pr _Pred) {
// test if [_First1, _Last1) == permuted [_First2, _Last2), using _Pred, same lengths
_NODISCARD bool _Check_match_counts(
const _FwdIt1 _First1, _FwdIt1 _Last1, const _FwdIt2 _First2, _FwdIt2 _Last2, _Pr _Pred) {
// test if [_First1, _Last1) == permuted [_First2, _Last2), after matching prefix removal
_STL_INTERNAL_CHECK(!_Pred(*_First1, *_First2));
_STL_INTERNAL_CHECK(_STD distance(_First1, _Last1) == _STD distance(_First2, _Last2));
#if _HAS_IF_CONSTEXPR
if constexpr (_Is_bidi_iter_v<_FwdIt1> && _Is_bidi_iter_v<_FwdIt2>) {
do { // find last inequality
--_Last1;
--_Last2;
} while (_Pred(*_Last1, *_Last2));
++_Last1;
++_Last2;
}
#else // ^^^ _HAS_IF_CONSTEXPR // !_HAS_IF_CONSTEXPR vvv
_Trim_matching_suffixes(_Last1, _Last2, _Pred, _Iter_cat_t<_FwdIt1>(), _Iter_cat_t<_FwdIt2>());
#endif // _HAS_IF_CONSTEXPR
for (_FwdIt1 _Next1 = _First1; _Next1 != _Last1; ++_Next1) {
if (_Next1 == _Find_pr(_First1, _Next1, *_Next1, _Pred)) { // new value, compare match counts
_Iter_diff_t<_FwdIt2> _Count2 = _Count_pr(_First2, _Last2, *_Next1, _Pred);
if (_Count2 == 0) {
return false; // second range lacks value, fail
return false; // second range lacks value, not a permutation
}

_FwdIt1 _Skip1 = _Next_iter(_Next1);
_Iter_diff_t<_FwdIt1> _Count1 = _Count_pr(_Skip1, _Last1, *_Next1, _Pred) + 1;
if (_Count2 != _Count1) {
return false; // match counts differ, fail
return false; // match counts differ, not a permutation
}
}
}

return true;
}

// FUNCTION TEMPLATE is_permutation
template <class _FwdIt1, class _FwdIt2, class _Pr>
bool _Is_permutation_unchecked(_FwdIt1 _First1, _FwdIt1 _Last1, _FwdIt2 _First2, _Pr _Pred) {
_NODISCARD bool _Is_permutation_unchecked(_FwdIt1 _First1, _FwdIt1 _Last1, _FwdIt2 _First2, _Pr _Pred) {
// test if [_First1, _Last1) == permuted [_First2, ...), using _Pred
for (; _First1 != _Last1; ++_First1, (void) ++_First2) {
for (; _First1 != _Last1; ++_First1, (void) ++_First2) { // trim matching prefix
if (!_Pred(*_First1, *_First2)) {
// found first inequality, check match counts in suffix narrowing _Iter_diff_t<_FwdIt1> to
// _Iter_diff_t<_FwdIt2> is OK because if the 2nd range is shorter than the 1st, the user already
// triggered UB
// found first inequality, check match counts in suffix
//
// narrowing _Iter_diff_t<_FwdIt1> to _Iter_diff_t<_FwdIt2> is OK because if the second range is shorter
// than the first, the user already triggered UB
auto _Last2 = _STD next(_First2, static_cast<_Iter_diff_t<_FwdIt2>>(_STD distance(_First1, _Last1)));
return _Check_match_counts(_First1, _Last1, _First2, _Last2, _Pred);
}
Expand Down Expand Up @@ -4805,17 +4817,41 @@ template <class _FwdIt1, class _FwdIt2, class _Pr>
bool _Is_permutation_unchecked(_FwdIt1 _First1, _FwdIt1 _Last1, _FwdIt2 _First2, _FwdIt2 _Last2, _Pr _Pred,
forward_iterator_tag, forward_iterator_tag) {
// test if [_First1, _Last1) == permuted [_First2, _Last2), using _Pred, arbitrary iterators
for (; _First1 != _Last1 && _First2 != _Last2; ++_First1, (void) ++_First2) {
for (;;) { // trim matching prefix
if (_First1 == _Last1) {
return _First2 == _Last2;
}

if (_First2 == _Last2) {
return false;
}

if (!_Pred(*_First1, *_First2)) { // found first inequality, check match counts in suffix
if (_STD distance(_First1, _Last1) == _STD distance(_First2, _Last2)) {
break;
}

++_First1;
++_First2;
}

auto _Next1 = _First1;
auto _Next2 = _First2;
for (;;) { // check for same lengths
if (_Next1 == _Last1) {
if (_Next2 == _Last2) {
return _Check_match_counts(_First1, _Last1, _First2, _Last2, _Pred);
} else {
return false; // lengths differ, fail
}

return false; // sequence 1 is shorter than sequence 2, not a permutation
}
}

return _First1 == _Last1 && _First2 == _Last2;
if (_Next2 == _Last2) {
return false; // sequence 1 is longer than sequence 2, not a permutation
}

++_Next1;
++_Next2;
}
}

template <class _FwdIt1, class _FwdIt2, class _Pr>
Expand All @@ -4826,7 +4862,14 @@ bool _Is_permutation_unchecked(_FwdIt1 _First1, _FwdIt1 _Last1, _FwdIt2 _First2,
return false;
}

return _Is_permutation_unchecked(_First1, _Last1, _First2, _Pred);
for (; _First1 != _Last1; ++_First1, (void) ++_First2) { // trim matching prefix
if (!_Pred(*_First1, *_First2)) {
// found first inequality, check match counts in suffix
return _Check_match_counts(_First1, _Last1, _First2, _Last2, _Pred);
}
}

return true;
}

template <class _FwdIt1, class _FwdIt2, class _Pr>
Expand All @@ -4838,7 +4881,6 @@ _NODISCARD bool is_permutation(_FwdIt1 _First1, _FwdIt1 _Last1, _FwdIt2 _First2,
_Get_unwrapped(_Last2), _Pass_fn(_Pred), _Iter_cat_t<_FwdIt1>(), _Iter_cat_t<_FwdIt2>());
}

// FUNCTION TEMPLATE is_permutation WITH TWO RANGES
template <class _FwdIt1, class _FwdIt2>
_NODISCARD bool is_permutation(_FwdIt1 _First1, _FwdIt1 _Last1, _FwdIt2 _First2, _FwdIt2 _Last2) {
// test if [_First1, _Last1) == permuted [_First2, _Last2)
Expand Down