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 r13-3815] libstdc++: basic_filebuf: don't flush more often than necessary [PR63746]
Date: Tue,  8 Nov 2022 17:44:23 +0000 (GMT)	[thread overview]
Message-ID: <20221108174423.0611338582AD@sourceware.org> (raw)

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

commit r13-3815-g3f1519eef5cbdcea2f18445852f5482798e3936a
Author: Charles-François Natali <cf.natali@gmail.com>
Date:   Thu Oct 6 20:02:56 2022 +0100

    libstdc++: basic_filebuf: don't flush more often than necessary [PR63746]
    
    `basic_filebuf::xsputn` would bypass the buffer when passed a chunk of
    size 1024 and above, seemingly as an optimisation.
    
    This can have a significant performance impact if the overhead of a
    `write` syscall is non-negligible, e.g. on a slow disk, on network
    filesystems, or simply during IO contention because instead of flushing
    every `BUFSIZ` (by default), we can flush every 1024 char.
    The impact is even greater with custom larger buffers, e.g. for network
    filesystems, because the code could issue `write` for example 1000X more
    often than necessary with respect to the buffer size.
    It also introduces a significant discontinuity in performance when
    writing chunks of size 1024 and above.
    
    Instead, it makes sense to only bypass the buffer if the amount of data
    to be written is larger than the buffer capacity.
    
    Signed-off-by: Charles-Francois Natali <cf.natali@gmail.com>
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/63746
            * include/bits/fstream.tcc (basic_filbuf::xsputn): Remove
            1024-byte chunking that bypasses the buffer for large writes.
            * testsuite/27_io/basic_filebuf/sputn/char/63746.cc: New test.

Diff:
---
 libstdc++-v3/include/bits/fstream.tcc              |  9 ++---
 .../27_io/basic_filebuf/sputn/char/63746.cc        | 38 ++++++++++++++++++++++
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/libstdc++-v3/include/bits/fstream.tcc b/libstdc++-v3/include/bits/fstream.tcc
index 7ccc887b882..2e936962837 100644
--- a/libstdc++-v3/include/bits/fstream.tcc
+++ b/libstdc++-v3/include/bits/fstream.tcc
@@ -757,23 +757,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     {
       streamsize __ret = 0;
       // Optimization in the always_noconv() case, to be generalized in the
-      // future: when __n is sufficiently large we write directly instead of
-      // using the buffer.
+      // future: when __n is larger than the available capacity we write
+      // directly instead of using the buffer.
       const bool __testout = (_M_mode & ios_base::out
 			      || _M_mode & ios_base::app);
       if (__check_facet(_M_codecvt).always_noconv()
 	  && __testout && !_M_reading)
 	{
-	  // Measurement would reveal the best choice.
-	  const streamsize __chunk = 1ul << 10;
 	  streamsize __bufavail = this->epptr() - this->pptr();
 
 	  // Don't mistake 'uncommitted' mode buffered with unbuffered.
 	  if (!_M_writing && _M_buf_size > 1)
 	    __bufavail = _M_buf_size - 1;
 
-	  const streamsize __limit = std::min(__chunk, __bufavail);
-	  if (__n >= __limit)
+	  if (__n >= __bufavail)
 	    {
 	      const streamsize __buffill = this->pptr() - this->pbase();
 	      const char* __buf = reinterpret_cast<const char*>(this->pbase());
diff --git a/libstdc++-v3/testsuite/27_io/basic_filebuf/sputn/char/63746.cc b/libstdc++-v3/testsuite/27_io/basic_filebuf/sputn/char/63746.cc
new file mode 100644
index 00000000000..baab9340734
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/basic_filebuf/sputn/char/63746.cc
@@ -0,0 +1,38 @@
+// { dg-require-fileio "" }
+
+#include <fstream>
+#include <testsuite_hooks.h>
+
+class testbuf : public std::filebuf {
+public:
+  char_type* pub_pprt() const
+  {
+    return this->pptr();
+  }
+
+  char_type* pub_pbase() const
+  {
+    return this->pbase();
+  }
+};
+
+void test01()
+{
+  using namespace std;
+
+  // Leave capacity to avoid flush.
+  const streamsize chunk_size = BUFSIZ - 1 - 1;
+  const char data[chunk_size] = {};
+
+  testbuf a_f;
+  VERIFY( a_f.open("tmp_63746_sputn", ios_base::out) );
+  VERIFY( chunk_size == a_f.sputn(data, chunk_size) );
+  VERIFY( (a_f.pub_pprt() - a_f.pub_pbase()) == chunk_size );
+  VERIFY( a_f.close() );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}

                 reply	other threads:[~2022-11-08 17:44 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=20221108174423.0611338582AD@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: 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).