public inbox for libstdc++-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r14-6567] libstdc++: Tweaks for std::format fast path
@ 2023-12-15  0:01 Jonathan Wakely
  0 siblings, 0 replies; only message in thread
From: Jonathan Wakely @ 2023-12-15  0:01 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

https://gcc.gnu.org/g:3fa0f9404b1bcd5a47629d4e121390603039f951

commit r14-6567-g3fa0f9404b1bcd5a47629d4e121390603039f951
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Dec 14 12:57:53 2023 +0000

    libstdc++: Tweaks for std::format fast path
    
    Fix an incorrect call to _Sink::_M_reserve() which should have passed
    the __n parameter. This was not actually a problem because it was in an
    discarded statement, because only the _Seq_sink<basic_string<C>>
    specialization was used.
    
    Also add some branch prediction hints, explanatory comments, and debug
    mode assertions to _Seq_sink.
    
    libstdc++-v3/ChangeLog:
    
            * include/std/format (_Seq_sink): Fix missing argument in
            discarded statement. Add comments, likely/unlikely attributes
            and debug assertions as sanity checks.

Diff:
---
 libstdc++-v3/include/std/format | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
index 1f8cd5c06be..6204fd0e3c1 100644
--- a/libstdc++-v3/include/std/format
+++ b/libstdc++-v3/include/std/format
@@ -2609,15 +2609,13 @@ namespace __format
       virtual _Reservation
       _M_reserve(size_t __n)
       {
-	auto __avail = _M_unused();
-	if (__n <= __avail.size())
+	if (__n <= _M_unused().size())
 	  return { this };
 
 	if (__n <= _M_span.size()) // Cannot meet the request.
 	  {
 	    _M_overflow(); // Make more space available.
-	    __avail = _M_unused();
-	    if (__n <= __avail.size())
+	    if (__n <= _M_unused().size())
 	      return { this };
 	  }
 	return { nullptr };
@@ -2669,24 +2667,38 @@ namespace __format
       _M_overflow() override
       {
 	auto __s = this->_M_used();
-	if (__s.empty())
-	  return;
+	if (__s.empty()) [[unlikely]]
+	  return; // Nothing in the buffer to transfer to _M_seq.
+
+	// If _M_reserve was called then _M_bump must have been called too.
+	_GLIBCXX_DEBUG_ASSERT(__s.data() != _M_seq.data());
+
 	if constexpr (__is_specialization_of<_Seq, basic_string>)
 	  _M_seq.append(__s.data(), __s.size());
 	else
 	  _M_seq.insert(_M_seq.end(), __s.begin(), __s.end());
+
+	// Make the whole of _M_buf available for the next write:
 	this->_M_rewind();
       }
 
       typename _Sink<_CharT>::_Reservation
       _M_reserve(size_t __n) override
       {
+	// We might already have n characters available in this->_M_unused(),
+	// but the whole point of this function is to be an optimization for
+	// the std::format("{}", x) case. We want to avoid writing to _M_buf
+	// and then copying that into a basic_string if possible, so this
+	// function prefers to create space directly in _M_seq rather than
+	// using _M_buf.
+
 	if constexpr (__is_specialization_of<_Seq, basic_string>
 			|| __is_specialization_of<_Seq, vector>)
 	  {
-	    // Flush the buffer to _M_seq first:
-	    if (this->_M_used().size())
-	      _M_overflow();
+	    // Flush the buffer to _M_seq first (should not be needed).
+	    if (this->_M_used().size()) [[unlikely]]
+	      _Seq_sink::_M_overflow();
+
 	    // Expand _M_seq to make __n new characters available:
 	    const auto __sz = _M_seq.size();
 	    if constexpr (is_same_v<string, _Seq> || is_same_v<wstring, _Seq>)
@@ -2696,12 +2708,14 @@ namespace __format
 					    });
 	    else
 	      _M_seq.resize(__sz + __n);
-	    // Set _M_used() to be a span over the original part of _M_seq:
+
+	    // Set _M_used() to be a span over the original part of _M_seq
+	    // and _M_unused() to be the extra capacity we just created:
 	    this->_M_reset(_M_seq, __sz);
 	    return { this };
 	  }
 	else // Try to use the base class' buffer.
-	  return _Sink<_CharT>::_M_reserve();
+	  return _Sink<_CharT>::_M_reserve(__n);
       }
 
       void
@@ -2710,8 +2724,10 @@ namespace __format
 	if constexpr (__is_specialization_of<_Seq, basic_string>
 			|| __is_specialization_of<_Seq, vector>)
 	  {
+	    auto __s = this->_M_used();
+	    _GLIBCXX_DEBUG_ASSERT(__s.data() == _M_seq.data());
 	    // Truncate the sequence to the part that was actually written to:
-	    _M_seq.resize(this->_M_used().size() + __n);
+	    _M_seq.resize(__s.size() + __n);
 	    // Switch back to using buffer:
 	    this->_M_reset(this->_M_buf);
 	  }

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

only message in thread, other threads:[~2023-12-15  0:01 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-15  0:01 [gcc r14-6567] libstdc++: Tweaks for std::format fast path 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).