From 53196a8bcfe80db551aa9417ce93766c7ee0e46d Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Wed, 4 Nov 2020 23:55:41 +0200 Subject: [PATCH 1/4] Optimize write_vectored for BufWriter If the underlying writer does not support efficient vectored output, do it differently: always try to coalesce the slices in the buffer until one comes that does not fit entirely. Flush the buffer before the first slice if needed. --- library/std/src/io/buffered/bufwriter.rs | 62 +++++++++++++++++++----- 1 file changed, 50 insertions(+), 12 deletions(-) diff --git a/library/std/src/io/buffered/bufwriter.rs b/library/std/src/io/buffered/bufwriter.rs index 067ed6ba7ff50..d8d62c4b31444 100644 --- a/library/std/src/io/buffered/bufwriter.rs +++ b/library/std/src/io/buffered/bufwriter.rs @@ -328,19 +328,57 @@ impl Write for BufWriter { } fn write_vectored(&mut self, bufs: &[IoSlice<'_>]) -> io::Result { - let total_len = bufs.iter().map(|b| b.len()).sum::(); - if self.buf.len() + total_len > self.buf.capacity() { - self.flush_buf()?; - } - // FIXME: Why no len > capacity? Why not buffer len == capacity? #72919 - if total_len >= self.buf.capacity() { - self.panicked = true; - let r = self.get_mut().write_vectored(bufs); - self.panicked = false; - r + if self.get_ref().is_write_vectored() { + let total_len = bufs.iter().map(|b| b.len()).sum::(); + if self.buf.len() + total_len > self.buf.capacity() { + self.flush_buf()?; + } + if total_len >= self.buf.capacity() { + self.panicked = true; + let r = self.get_mut().write_vectored(bufs); + self.panicked = false; + r + } else { + bufs.iter().for_each(|b| self.buf.extend_from_slice(b)); + Ok(total_len) + } } else { - bufs.iter().for_each(|b| self.buf.extend_from_slice(b)); - Ok(total_len) + let mut total_written = 0; + let mut iter = bufs.iter(); + if let Some(buf) = iter.by_ref().find(|&buf| !buf.is_empty()) { + // This is the first non-empty slice to write, so if it does + // not fit in the buffer, we still get to flush and proceed. + if self.buf.len() + buf.len() > self.buf.capacity() { + self.flush_buf()?; + } + if buf.len() >= self.buf.capacity() { + // The slice is at least as large as the buffering capacity, + // so it's better to write it directly, bypassing the buffer. + self.panicked = true; + let r = self.get_mut().write(buf); + self.panicked = false; + return r; + } else { + self.buf.extend_from_slice(buf); + total_written += buf.len(); + } + debug_assert!(total_written != 0); + } + for buf in iter { + if buf.len() >= self.buf.capacity() { + // This slice should be written directly, but we have + // already buffered some of the input. Bail out, + // expecting it to be handled as the first slice in the + // next call to write_vectored. + break; + } else { + total_written += self.write_to_buf(buf); + if self.buf.capacity() == self.buf.len() { + break; + } + } + } + Ok(total_written) } } From 9fc44239ec7e9f682797bd9e38368d0ec4457077 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Sat, 7 Nov 2020 23:57:27 +0200 Subject: [PATCH 2/4] Make is_write_vectored return true for BufWriter BufWriter provides an efficient implementation of write_vectored also when the underlying writer does not support vectored writes. --- library/std/src/io/buffered/bufwriter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/io/buffered/bufwriter.rs b/library/std/src/io/buffered/bufwriter.rs index d8d62c4b31444..e42fa4297ef56 100644 --- a/library/std/src/io/buffered/bufwriter.rs +++ b/library/std/src/io/buffered/bufwriter.rs @@ -383,7 +383,7 @@ impl Write for BufWriter { } fn is_write_vectored(&self) -> bool { - self.get_ref().is_write_vectored() + true } fn flush(&mut self) -> io::Result<()> { From 00deeb35c8149508240549f5c9f3908a7ba9ee11 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Sun, 8 Nov 2020 01:46:05 +0200 Subject: [PATCH 3/4] Fix is_write_vectored in LineWriterShim Now that BufWriter always claims to support vectored writes, look through it at the wrapped writer to decide whether to use vectored writes for LineWriter. --- library/std/src/io/buffered/linewritershim.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/library/std/src/io/buffered/linewritershim.rs b/library/std/src/io/buffered/linewritershim.rs index a80d08db8692e..d0c859d2e0c87 100644 --- a/library/std/src/io/buffered/linewritershim.rs +++ b/library/std/src/io/buffered/linewritershim.rs @@ -20,6 +20,12 @@ impl<'a, W: Write> LineWriterShim<'a, W> { Self { buffer } } + /// Get a reference to the inner writer (that is, the writer + /// wrapped by the BufWriter). + fn inner(&self) -> &W { + self.buffer.get_ref() + } + /// Get a mutable reference to the inner writer (that is, the writer /// wrapped by the BufWriter). Be careful with this writer, as writes to /// it will bypass the buffer. @@ -227,7 +233,7 @@ impl<'a, W: Write> Write for LineWriterShim<'a, W> { } fn is_write_vectored(&self) -> bool { - self.buffer.is_write_vectored() + self.inner().is_write_vectored() } /// Write some data into this BufReader with line buffering. This means From 674dd623ee067d4298fda867f72442b13014eaa3 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Sun, 22 Nov 2020 17:00:48 +0200 Subject: [PATCH 4/4] Reduce branching in write_vectored for BufWriter Do what write does and optimize for the most likely case: slices are much smaller than the buffer. If a slice does not fit completely in the remaining capacity of the buffer, it is left out rather than buffered partially. Special treatment is only left for oversized slices that are written directly to the underlying writer. --- library/std/src/io/buffered/bufwriter.rs | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/library/std/src/io/buffered/bufwriter.rs b/library/std/src/io/buffered/bufwriter.rs index e42fa4297ef56..3b3399860ba7d 100644 --- a/library/std/src/io/buffered/bufwriter.rs +++ b/library/std/src/io/buffered/bufwriter.rs @@ -343,9 +343,8 @@ impl Write for BufWriter { Ok(total_len) } } else { - let mut total_written = 0; let mut iter = bufs.iter(); - if let Some(buf) = iter.by_ref().find(|&buf| !buf.is_empty()) { + let mut total_written = if let Some(buf) = iter.by_ref().find(|&buf| !buf.is_empty()) { // This is the first non-empty slice to write, so if it does // not fit in the buffer, we still get to flush and proceed. if self.buf.len() + buf.len() > self.buf.capacity() { @@ -360,22 +359,18 @@ impl Write for BufWriter { return r; } else { self.buf.extend_from_slice(buf); - total_written += buf.len(); + buf.len() } - debug_assert!(total_written != 0); - } + } else { + return Ok(0); + }; + debug_assert!(total_written != 0); for buf in iter { - if buf.len() >= self.buf.capacity() { - // This slice should be written directly, but we have - // already buffered some of the input. Bail out, - // expecting it to be handled as the first slice in the - // next call to write_vectored. + if self.buf.len() + buf.len() > self.buf.capacity() { break; } else { - total_written += self.write_to_buf(buf); - if self.buf.capacity() == self.buf.len() { - break; - } + self.buf.extend_from_slice(buf); + total_written += buf.len(); } } Ok(total_written)