public inbox for libstdc++-cvs@sourceware.org help / color / mirror / Atom feed
From: Jonathan Wakely <redi@gcc.gnu.org> To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org Subject: [gcc r14-3306] libstdc++: Optimize std::string::assign(Iter, Iter) [PR110945] Date: Thu, 17 Aug 2023 20:31:17 +0000 (GMT) [thread overview] Message-ID: <20230817203117.0C7EE386546F@sourceware.org> (raw) https://gcc.gnu.org/g:cc3d7baf2741777e99567d4301802c99f5775619 commit r14-3306-gcc3d7baf2741777e99567d4301802c99f5775619 Author: Jonathan Wakely <jwakely@redhat.com> Date: Tue Aug 8 16:31:42 2023 +0100 libstdc++: Optimize std::string::assign(Iter, Iter) [PR110945] Calling string::assign(Iter, Iter) with "foreign" iterators (not the string's own iterator or pointer types) currently constructs a temporary string and then calls replace to copy the characters from it. That means we copy from the iterators twice, and if the replace operation has to grow the string then we also allocate twice. By using *this = basic_string(first, last, get_allocator()) we only perform a single allocation+copy and then do a cheap move assignment instead of a second copy (and possible allocation). But that alternative has to be done conditionally, so that we don't pessimize the native iterator case (the string's own iterator and pointer types) which currently select efficient overloads of replace which will not allocate at all if the string already has sufficient capacity. For C++20 we can extend that efficient case to work for any contiguous iterator with the right value type, not just for the string's native iterators. So the change is to inline the code that decides whether to work in place or to allocate+copy (instead of deciding that via overload resolution for replace), and for the allocate+copy case do a move assignment instead of another call to replace. For C++98 there is no change, as we can't do an efficient move assignment anyway, so keep the current code. We can also simplify assign(initializer_list<CharT>) because the backing array for an initializer_list is always disjunct with *this, so most of the code in _M_replace is not needed. libstdc++-v3/ChangeLog: PR libstdc++/110945 * include/bits/basic_string.h (basic_string::assign(Iter, Iter)): Dispatch to _M_replace or move assignment from a temporary, based on the iterator type. Diff: --- libstdc++-v3/include/bits/basic_string.h | 42 +++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index f4bbf521bba7..09fd62afa665 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -1711,15 +1711,36 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 * Sets value of string to characters in the range [__first,__last). */ #if __cplusplus >= 201103L +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wc++17-extensions" template<class _InputIterator, typename = std::_RequireInputIter<_InputIterator>> _GLIBCXX20_CONSTEXPR + basic_string& + assign(_InputIterator __first, _InputIterator __last) + { +#if __cplusplus >= 202002L + if constexpr (contiguous_iterator<_InputIterator> + && is_same_v<iter_value_t<_InputIterator>, _CharT>) +#else + if constexpr (__is_one_of<_InputIterator, const_iterator, iterator, + const _CharT*, _CharT*>::value) +#endif + { + __glibcxx_requires_valid_range(__first, __last); + return _M_replace(size_type(0), size(), + std::__to_address(__first), __last - __first); + } + else + return *this = basic_string(__first, __last, get_allocator()); + } +#pragma GCC diagnostic pop #else template<class _InputIterator> + basic_string& + assign(_InputIterator __first, _InputIterator __last) + { return this->replace(begin(), end(), __first, __last); } #endif - basic_string& - assign(_InputIterator __first, _InputIterator __last) - { return this->replace(begin(), end(), __first, __last); } #if __cplusplus >= 201103L /** @@ -1730,7 +1751,20 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 _GLIBCXX20_CONSTEXPR basic_string& assign(initializer_list<_CharT> __l) - { return this->assign(__l.begin(), __l.size()); } + { + // The initializer_list array cannot alias the characters in *this + // so we don't need to use replace to that case. + const size_type __n = __l.size(); + if (__n > capacity()) + *this = basic_string(__l.begin(), __l.end(), get_allocator()); + else + { + if (__n) + _S_copy(_M_data(), __l.begin(), __n); + _M_set_length(__n); + } + return *this; + } #endif // C++11 #if __cplusplus >= 201703L
reply other threads:[~2023-08-17 20:31 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=20230817203117.0C7EE386546F@sourceware.org \ --to=redi@gcc.gnu.org \ --cc=gcc-cvs@gcc.gnu.org \ --cc=libstdc++-cvs@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: linkBe 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).