From 6cb3717e834bb67e92158e98b09b3b2f2034955d Mon Sep 17 00:00:00 2001 From: Chris Gregory Date: Sat, 6 Apr 2019 16:47:33 -0400 Subject: [PATCH 1/4] BufWriter remove Option around inner Write implementation --- src/libstd/io/buffered.rs | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/libstd/io/buffered.rs b/src/libstd/io/buffered.rs index 3370a447fcc8e..8584baf9062c0 100644 --- a/src/libstd/io/buffered.rs +++ b/src/libstd/io/buffered.rs @@ -410,7 +410,7 @@ impl Seek for BufReader { /// [`flush`]: #method.flush #[stable(feature = "rust1", since = "1.0.0")] pub struct BufWriter { - inner: Option, + inner: W, buf: Vec, // #30888: If the inner writer panics in a call to write, we don't want to // write the buffered data a second time in BufWriter's destructor. This @@ -479,7 +479,7 @@ impl BufWriter { #[stable(feature = "rust1", since = "1.0.0")] pub fn with_capacity(cap: usize, inner: W) -> BufWriter { BufWriter { - inner: Some(inner), + inner, buf: Vec::with_capacity(cap), panicked: false, } @@ -491,7 +491,7 @@ impl BufWriter { let mut ret = Ok(()); while written < len { self.panicked = true; - let r = self.inner.as_mut().unwrap().write(&self.buf[written..]); + let r = self.inner.write(&self.buf[written..]); self.panicked = false; match r { @@ -526,7 +526,7 @@ impl BufWriter { /// let reference = buffer.get_ref(); /// ``` #[stable(feature = "rust1", since = "1.0.0")] - pub fn get_ref(&self) -> &W { self.inner.as_ref().unwrap() } + pub fn get_ref(&self) -> &W { self.inner } /// Gets a mutable reference to the underlying writer. /// @@ -544,7 +544,7 @@ impl BufWriter { /// let reference = buffer.get_mut(); /// ``` #[stable(feature = "rust1", since = "1.0.0")] - pub fn get_mut(&mut self) -> &mut W { self.inner.as_mut().unwrap() } + pub fn get_mut(&mut self) -> &mut W { self.inner } /// Returns a reference to the internally buffered data. /// @@ -588,7 +588,13 @@ impl BufWriter { pub fn into_inner(mut self) -> Result>> { match self.flush_buf() { Err(e) => Err(IntoInnerError(self, e)), - Ok(()) => Ok(self.inner.take().unwrap()) + Ok(()) => { + use std::mem::{replace, uninitialized, forget}; + let inner = replace(&mut self.inner, unsafe { uninitialized() }); + replace(&mut self.buf, unsafe { uninitialized() }); + forget(self); + Ok(inner) + } } } } @@ -601,7 +607,7 @@ impl Write for BufWriter { } if buf.len() >= self.buf.capacity() { self.panicked = true; - let r = self.inner.as_mut().unwrap().write(buf); + let r = self.inner.write(buf); self.panicked = false; r } else { @@ -616,7 +622,7 @@ impl Write for BufWriter { } if total_len >= self.buf.capacity() { self.panicked = true; - let r = self.inner.as_mut().unwrap().write_vectored(bufs); + let r = self.inner.write_vectored(bufs); self.panicked = false; r } else { @@ -633,7 +639,7 @@ impl Write for BufWriter { impl fmt::Debug for BufWriter where W: fmt::Debug { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt.debug_struct("BufWriter") - .field("writer", &self.inner.as_ref().unwrap()) + .field("writer", &self.inner) .field("buffer", &format_args!("{}/{}", self.buf.len(), self.buf.capacity())) .finish() } @@ -652,7 +658,7 @@ impl Seek for BufWriter { #[stable(feature = "rust1", since = "1.0.0")] impl Drop for BufWriter { fn drop(&mut self) { - if self.inner.is_some() && !self.panicked { + if !self.panicked { // dtors should not panic, so we ignore a failed flush let _r = self.flush_buf(); } From fadb8dc2d7c4693119d71da3862f174ce80dd5ac Mon Sep 17 00:00:00 2001 From: Chris Gregory Date: Sat, 6 Apr 2019 16:48:29 -0400 Subject: [PATCH 2/4] Add benchmarks --- src/libstd/io/buffered.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/libstd/io/buffered.rs b/src/libstd/io/buffered.rs index 8584baf9062c0..52727a24c4637 100644 --- a/src/libstd/io/buffered.rs +++ b/src/libstd/io/buffered.rs @@ -1398,6 +1398,32 @@ mod tests { }); } + #[bench] + fn bench_buffered_writer_large_write_causing_flush(b: &mut test::Bencher) { + b.iter(|| { + let mut buf_writer = BufWriter::with_capacity(10, io::sink()); + buf_writer.write(b"abcdefghijklmnopqrstuvwxyz").unwrap(); + }); + } + + #[bench] + fn bench_buffered_writer_small_write(b: &mut test::Bencher) { + b.iter(|| { + let mut buf_writer = BufWriter::with_capacity(10, io::sink()); + buf_writer.write(b"abcdef").unwrap(); + }); + } + + #[bench] + fn bench_buffered_writer_multiple_small_writes(b: &mut test::Bencher) { + b.iter(|| { + let mut buf_writer = BufWriter::with_capacity(10, io::sink()); + buf_writer.write(b"abcdef").unwrap(); + buf_writer.write(b"abcdef").unwrap(); + buf_writer.write(b"abcdef").unwrap(); + }); + } + struct AcceptOneThenFail { written: bool, flushed: bool, From d1fe13d52cc2808c2fcb0a87020a1d163a078da0 Mon Sep 17 00:00:00 2001 From: Chris Gregory Date: Sat, 6 Apr 2019 17:01:39 -0400 Subject: [PATCH 3/4] Use read and drop_in_place instead of replace and uninitialized --- src/libstd/io/buffered.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libstd/io/buffered.rs b/src/libstd/io/buffered.rs index 52727a24c4637..81db962240235 100644 --- a/src/libstd/io/buffered.rs +++ b/src/libstd/io/buffered.rs @@ -589,9 +589,10 @@ impl BufWriter { match self.flush_buf() { Err(e) => Err(IntoInnerError(self, e)), Ok(()) => { - use std::mem::{replace, uninitialized, forget}; - let inner = replace(&mut self.inner, unsafe { uninitialized() }); - replace(&mut self.buf, unsafe { uninitialized() }); + use crate::mem::forget; + use crate::ptr::{drop_in_place, read}; + let inner = unsafe { read(&mut self.inner) }; + unsafe { drop_in_place(&mut self.buf) }; forget(self); Ok(inner) } From d3311b7be9ff23cb43943e6efabbcd2cadfc7d51 Mon Sep 17 00:00:00 2001 From: Chris Gregory Date: Sat, 6 Apr 2019 17:09:25 -0400 Subject: [PATCH 4/4] Fix get_ref and get_mut implementations --- src/libstd/io/buffered.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstd/io/buffered.rs b/src/libstd/io/buffered.rs index 81db962240235..db17b00cd0104 100644 --- a/src/libstd/io/buffered.rs +++ b/src/libstd/io/buffered.rs @@ -526,7 +526,7 @@ impl BufWriter { /// let reference = buffer.get_ref(); /// ``` #[stable(feature = "rust1", since = "1.0.0")] - pub fn get_ref(&self) -> &W { self.inner } + pub fn get_ref(&self) -> &W { &self.inner } /// Gets a mutable reference to the underlying writer. /// @@ -544,7 +544,7 @@ impl BufWriter { /// let reference = buffer.get_mut(); /// ``` #[stable(feature = "rust1", since = "1.0.0")] - pub fn get_mut(&mut self) -> &mut W { self.inner } + pub fn get_mut(&mut self) -> &mut W { &mut self.inner } /// Returns a reference to the internally buffered data. ///