[libcxx] r180072 - Modest performance improvement for std::string's operator==.

[Howard Hinnant]

On Apr 23, 2013, at 2:10 AM, Douglas Gregor <dgregor at apple.com> wrote:

> 
> 
> Sent from my iPhone
> 
> On Apr 22, 2013, at 4:55 PM, Howard Hinnant <hhinnant at apple.com> wrote:
> 
>> Author: hhinnant
>> Date: Mon Apr 22 18:55:13 2013
>> New Revision: 180072
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=180072&view=rev
>> Log:
>> Modest performance improvement for std::string's operator==.
>> 
>> Modified:
>>   libcxx/trunk/include/string
>> 
>> Modified: libcxx/trunk/include/string
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=180072&r1=180071&r2=180072&view=diff
>> ==============================================================================
>> --- libcxx/trunk/include/string (original)
>> +++ libcxx/trunk/include/string Mon Apr 22 18:55:13 2013
>> @@ -1462,6 +1462,11 @@ public:
>>    int compare(size_type __pos1, size_type __n1, const_pointer __s, size_type __n2) const;
>> 
>>    _LIBCPP_INLINE_VISIBILITY bool __invariants() const;
>> +
>> +    _LIBCPP_INLINE_VISIBILITY
>> +    bool __is_long() const _NOEXCEPT
>> +        {return bool(__r_.first().__s.__size_ & __short_mask);}
>> +
>> private:
>>    _LIBCPP_INLINE_VISIBILITY
>>    allocator_type& __alloc() _NOEXCEPT
>> @@ -1471,10 +1476,6 @@ private:
>>        {return __r_.second();}
>> 
>>    _LIBCPP_INLINE_VISIBILITY
>> -    bool __is_long() const _NOEXCEPT
>> -        {return bool(__r_.first().__s.__size_ & __short_mask);}
>> -
>> -    _LIBCPP_INLINE_VISIBILITY
>>    void __set_short_size(size_type __s) _NOEXCEPT
>> #if _LIBCPP_BIG_ENDIAN
>>        {__r_.first().__s.__size_ = (unsigned char)(__s);}
>> @@ -3561,9 +3562,29 @@ bool
>> operator==(const basic_string<_CharT, _Traits, _Allocator>& __lhs,
>>           const basic_string<_CharT, _Traits, _Allocator>& __rhs) _NOEXCEPT
>> {
>> -    return __lhs.size() == __rhs.size() && _Traits::compare(__lhs.data(),
>> -                                                            __rhs.data(),
>> -                                                            __lhs.size()) == 0;
>> +    size_t __lhs_sz = __lhs.size();
>> +    return __lhs_sz == __rhs.size() && _Traits::compare(__lhs.data(),
>> +                                                        __rhs.data(),
>> +                                                        __lhs_sz) == 0;
>> +}
>> +
>> +template<class _Allocator>
>> +_LIBCPP_INLINE_VISIBILITY inline
>> +bool
>> +operator==(const basic_string<char, char_traits<char>, _Allocator>& __lhs,
>> +           const basic_string<char, char_traits<char>, _Allocator>& __rhs) _NOEXCEPT
>> +{
>> +    size_t __lhs_sz = __lhs.size();
>> +    if (__lhs_sz != __rhs.size())
>> +        return false;
>> +    const char* __lp = __lhs.data();
>> +    const char* __rp = __rhs.data();
>> +    if (__lhs.__is_long())
>> +        return char_traits<char>::compare(__lp, __rp, __lhs_sz) == 0;
>> +    for (; __lhs_sz != 0; --__lhs_sz, ++__lp, ++__rp)
>> +        if (*__lp != *__rp)
>> +            return false;
> 
> Shouldn't this go through traits for the character comparison?

<shrug> It could.  There's no way to tell that it didn't.  This is exactly what char_traits<char> would do, this function only uses char_traits<char>, and the client is not allowed to replace char_traits<char>.  And if the client could detect that I wasn't calling char_traits<char>::eq, changing to call char_traits<char>::eq wouldn't make it conforming.  The general form of this function is required to call Traits::compare, not Traits::eq.

Howard