public inbox for libstdc++-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r14-3306] libstdc++: Optimize std::string::assign(Iter, Iter) [PR110945]
@ 2023-08-17 20:31 Jonathan Wakely
  0 siblings, 0 replies; only message in thread
From: Jonathan Wakely @ 2023-08-17 20:31 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

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

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-08-17 20:31 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-17 20:31 [gcc r14-3306] libstdc++: Optimize std::string::assign(Iter, Iter) [PR110945] Jonathan Wakely

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).