From: Will Hawkins <hawkinsw@obs.cr>
To: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org
Subject: Re: [PATCH] libstdc++: Refactor implementation of operator+ for std::string
Date: Wed, 2 Nov 2022 16:25:53 -0400 [thread overview]
Message-ID: <CADx9qWig_m2FuXur5ascsbTPMJgL_1AgZn_UWOip7X2zwNXUuw@mail.gmail.com> (raw)
In-Reply-To: <20221020000559.371886-1-whh8b@obs.cr>
Just wanted to see if there was anything else I can do to help move
this over the finish line! Thanks for all the work that you all do!
Sincerely,
Will
On Wed, Oct 19, 2022 at 8:06 PM Will Hawkins <whh8b@obs.cr> wrote:
>
> Sorry for the delay. Tested on x86-64 Linux.
>
> -->8--
>
> After consultation with Jonathan, it seemed like a good idea to create a
> single function that performed one-allocation string concatenation that
> could be used by various different version of operator+. This patch adds
> such a function and calls it from the relevant implementations.
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/basic_string.h:
> Add common function that performs single-allocation string
> concatenation. (__str_cat)
> Use __str_cat to perform optimized operator+, where relevant.
> * include/bits/basic_string.tcc::
> Remove single-allocation implementation of operator+.
>
> Signed-off-by: Will Hawkins <whh8b@obs.cr>
> ---
> libstdc++-v3/include/bits/basic_string.h | 66 ++++++++++++++++------
> libstdc++-v3/include/bits/basic_string.tcc | 41 --------------
> 2 files changed, 49 insertions(+), 58 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
> index cd244191df4..9c2b57f5a1d 100644
> --- a/libstdc++-v3/include/bits/basic_string.h
> +++ b/libstdc++-v3/include/bits/basic_string.h
> @@ -3485,6 +3485,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> _GLIBCXX_END_NAMESPACE_CXX11
> #endif
>
> + template<typename _Str>
> + _GLIBCXX20_CONSTEXPR
> + inline _Str
> + __str_concat(typename _Str::value_type const* __lhs,
> + typename _Str::size_type __lhs_len,
> + typename _Str::value_type const* __rhs,
> + typename _Str::size_type __rhs_len,
> + typename _Str::allocator_type const& __a)
> + {
> + typedef typename _Str::allocator_type allocator_type;
> + typedef __gnu_cxx::__alloc_traits<allocator_type> _Alloc_traits;
> + _Str __str(_Alloc_traits::_S_select_on_copy(__a));
> + __str.reserve(__lhs_len + __rhs_len);
> + __str.append(__lhs, __lhs_len);
> + __str.append(__rhs, __rhs_len);
> + return __str;
> + }
> +
> // operator+
> /**
> * @brief Concatenate two strings.
> @@ -3494,13 +3512,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
> */
> template<typename _CharT, typename _Traits, typename _Alloc>
> _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> - basic_string<_CharT, _Traits, _Alloc>
> + inline basic_string<_CharT, _Traits, _Alloc>
> operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
> const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> {
> - basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> - __str.append(__rhs);
> - return __str;
> + typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> + return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> + __rhs.c_str(), __rhs.size(),
> + __lhs.get_allocator());
> }
>
> /**
> @@ -3511,9 +3530,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
> */
> template<typename _CharT, typename _Traits, typename _Alloc>
> _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> - basic_string<_CharT,_Traits,_Alloc>
> + inline basic_string<_CharT,_Traits,_Alloc>
> operator+(const _CharT* __lhs,
> - const basic_string<_CharT,_Traits,_Alloc>& __rhs);
> + const basic_string<_CharT,_Traits,_Alloc>& __rhs)
> + {
> + __glibcxx_requires_string(__lhs);
> + typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> + return std::__str_concat<_Str>(__lhs, _Traits::length(__lhs),
> + __rhs.c_str(), __rhs.size(),
> + __rhs.get_allocator());
> + }
>
> /**
> * @brief Concatenate character and string.
> @@ -3523,8 +3549,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
> */
> template<typename _CharT, typename _Traits, typename _Alloc>
> _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> - basic_string<_CharT,_Traits,_Alloc>
> - operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs);
> + inline basic_string<_CharT,_Traits,_Alloc>
> + operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs)
> + {
> + typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> + return std::__str_concat<_Str>(__builtin_addressof(__lhs), 1,
> + __rhs.c_str(), __rhs.size(),
> + __rhs.get_allocator());
> + }
>
> /**
> * @brief Concatenate string and C string.
> @@ -3538,11 +3570,12 @@ _GLIBCXX_END_NAMESPACE_CXX11
> operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
> const _CharT* __rhs)
> {
> - basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> - __str.append(__rhs);
> - return __str;
> + __glibcxx_requires_string(__rhs);
> + typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> + return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> + __rhs, _Traits::length(__rhs),
> + __lhs.get_allocator());
> }
> -
> /**
> * @brief Concatenate string and character.
> * @param __lhs First string.
> @@ -3554,11 +3587,10 @@ _GLIBCXX_END_NAMESPACE_CXX11
> inline basic_string<_CharT, _Traits, _Alloc>
> operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs, _CharT __rhs)
> {
> - typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
> - typedef typename __string_type::size_type __size_type;
> - __string_type __str(__lhs);
> - __str.append(__size_type(1), __rhs);
> - return __str;
> + typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> + return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> + __builtin_addressof(__rhs), 1,
> + __lhs.get_allocator());
> }
>
> #if __cplusplus >= 201103L
> diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
> index 710c2dfe030..56114f120ba 100644
> --- a/libstdc++-v3/include/bits/basic_string.tcc
> +++ b/libstdc++-v3/include/bits/basic_string.tcc
> @@ -605,47 +605,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> #else
> # define _GLIBCXX_STRING_CONSTEXPR
> #endif
> -
> - template<typename _CharT, typename _Traits, typename _Alloc>
> - _GLIBCXX20_CONSTEXPR
> - basic_string<_CharT, _Traits, _Alloc>
> - operator+(const _CharT* __lhs,
> - const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> - {
> - __glibcxx_requires_string(__lhs);
> - typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
> - typedef typename __string_type::size_type __size_type;
> - typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
> - rebind<_CharT>::other _Char_alloc_type;
> - typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
> - const __size_type __len = _Traits::length(__lhs);
> - __string_type __str(_Alloc_traits::_S_select_on_copy(
> - __rhs.get_allocator()));
> - __str.reserve(__len + __rhs.size());
> - __str.append(__lhs, __len);
> - __str.append(__rhs);
> - return __str;
> - }
> -
> - template<typename _CharT, typename _Traits, typename _Alloc>
> - _GLIBCXX20_CONSTEXPR
> - basic_string<_CharT, _Traits, _Alloc>
> - operator+(_CharT __lhs, const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> - {
> - typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
> - typedef typename __string_type::size_type __size_type;
> - typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
> - rebind<_CharT>::other _Char_alloc_type;
> - typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
> - __string_type __str(_Alloc_traits::_S_select_on_copy(
> - __rhs.get_allocator()));
> - const __size_type __len = __rhs.size();
> - __str.reserve(__len + 1);
> - __str.append(__size_type(1), __lhs);
> - __str.append(__rhs);
> - return __str;
> - }
> -
> template<typename _CharT, typename _Traits, typename _Alloc>
> _GLIBCXX_STRING_CONSTEXPR
> typename basic_string<_CharT, _Traits, _Alloc>::size_type
> --
> 2.37.3
>
next prev parent reply other threads:[~2022-11-02 20:26 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-20 0:05 Will Hawkins
2022-11-02 20:25 ` Will Hawkins [this message]
2022-11-08 17:44 ` Jonathan Wakely
-- strict thread matches above, loose matches on Subject: below --
2022-09-05 18:30 Will Hawkins
2022-09-08 17:50 ` François Dumont
2022-09-08 18:05 ` Jonathan Wakely
2022-09-09 13:53 ` Will Hawkins
2022-09-09 20:28 ` Jonathan Wakely
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CADx9qWig_m2FuXur5ascsbTPMJgL_1AgZn_UWOip7X2zwNXUuw@mail.gmail.com \
--to=hawkinsw@obs.cr \
--cc=gcc-patches@gcc.gnu.org \
--cc=libstdc++@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).