From a69fba890cf69e1b19520c6ef55e4bfd0a7f075d Mon Sep 17 00:00:00 2001 From: "main()" Date: Mon, 13 Apr 2020 12:32:46 +0200 Subject: [PATCH 1/4] Specialize io::copy for BufRead sources Allocating an additional buffer and then copying through it is completely unnecessary if the source we're copying from is BufRead (i.e. already maintains a buffer internally). Using specialization, we can detect this and handle it accordingly. Note that this also makes it possible to specify a custom buffer size for copy operations by wrapping the source in a BufReader::with_capacity. --- src/libstd/io/util.rs | 86 +++++++++++++++++++++++++++++++------------ 1 file changed, 63 insertions(+), 23 deletions(-) diff --git a/src/libstd/io/util.rs b/src/libstd/io/util.rs index b09161b97aa5..f97467d27b8d 100644 --- a/src/libstd/io/util.rs +++ b/src/libstd/io/util.rs @@ -1,8 +1,67 @@ #![allow(missing_copy_implementations)] use crate::fmt; -use crate::io::{self, BufRead, ErrorKind, Initializer, IoSlice, IoSliceMut, Read, Write}; -use crate::mem::MaybeUninit; +use crate::io::{self, BufRead, Initializer, IoSlice, IoSliceMut, Read, Write}; + +mod copy_specialization { + use crate::mem::MaybeUninit; + use crate::io::{self, Read, BufRead, Write, ErrorKind}; + + pub trait Copyable { + fn copy_to(&mut self, writer: &mut W) -> io::Result; + } + + impl Copyable for T { + default fn copy_to(&mut self, writer: &mut W) -> io::Result { + let mut buf = MaybeUninit::<[u8; io::DEFAULT_BUF_SIZE]>::uninit(); + // FIXME(#53491): This is calling `get_mut` and `get_ref` on an uninitialized + // `MaybeUninit`. Revisit this once we decided whether that is valid or not. + // This is still technically undefined behavior due to creating a reference + // to uninitialized data, but within libstd we can rely on more guarantees + // than if this code were in an external lib. + unsafe { + self.initializer().initialize(buf.get_mut()); + } + + let mut written = 0; + loop { + let len = match self.read(unsafe { buf.get_mut() }) { + Ok(0) => return Ok(written), + Ok(len) => len, + Err(ref e) if e.kind() == ErrorKind::Interrupted => continue, + Err(e) => return Err(e), + }; + writer.write_all(unsafe { &buf.get_ref()[..len] })?; + written += len as u64; + } + } + } + + impl Copyable for T { + fn copy_to(&mut self, writer: &mut W) -> io::Result { + let mut written = 0; + loop { + let (write_result, consumed) = { + let buf = match self.fill_buf() { + Err(ref e) if e.kind() == ErrorKind::Interrupted => continue, + Ok(b) if b.is_empty() => return Ok(written), + x => x, + }?; + (writer.write_all(buf), buf.len()) + }; + + // make sure to consume the data before dealing with write errors + // to maintain the same semantics as the default impl + // + // we have to do it this way because BufRead::consume has + // a borrow conflict on the returned buffer + self.consume(consumed); + written += consumed as u64; + write_result?; + } + } + } +} /// Copies the entire contents of a reader into a writer. /// @@ -45,27 +104,8 @@ where R: Read, W: Write, { - let mut buf = MaybeUninit::<[u8; super::DEFAULT_BUF_SIZE]>::uninit(); - // FIXME(#53491): This is calling `get_mut` and `get_ref` on an uninitialized - // `MaybeUninit`. Revisit this once we decided whether that is valid or not. - // This is still technically undefined behavior due to creating a reference - // to uninitialized data, but within libstd we can rely on more guarantees - // than if this code were in an external lib. - unsafe { - reader.initializer().initialize(buf.get_mut()); - } - - let mut written = 0; - loop { - let len = match reader.read(unsafe { buf.get_mut() }) { - Ok(0) => return Ok(written), - Ok(len) => len, - Err(ref e) if e.kind() == ErrorKind::Interrupted => continue, - Err(e) => return Err(e), - }; - writer.write_all(unsafe { &buf.get_ref()[..len] })?; - written += len as u64; - } + use self::copy_specialization::Copyable; + reader.copy_to(writer) } /// A reader which is always at EOF. From 84640a5bd128f6b731b610a8c5e2ecf494018264 Mon Sep 17 00:00:00 2001 From: "main()" Date: Mon, 13 Apr 2020 12:50:50 +0200 Subject: [PATCH 2/4] Fix trailing whitespace that github didn't show me --- src/libstd/io/util.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstd/io/util.rs b/src/libstd/io/util.rs index f97467d27b8d..6904ac20f527 100644 --- a/src/libstd/io/util.rs +++ b/src/libstd/io/util.rs @@ -6,11 +6,11 @@ use crate::io::{self, BufRead, Initializer, IoSlice, IoSliceMut, Read, Write}; mod copy_specialization { use crate::mem::MaybeUninit; use crate::io::{self, Read, BufRead, Write, ErrorKind}; - + pub trait Copyable { fn copy_to(&mut self, writer: &mut W) -> io::Result; } - + impl Copyable for T { default fn copy_to(&mut self, writer: &mut W) -> io::Result { let mut buf = MaybeUninit::<[u8; io::DEFAULT_BUF_SIZE]>::uninit(); @@ -49,7 +49,7 @@ mod copy_specialization { }?; (writer.write_all(buf), buf.len()) }; - + // make sure to consume the data before dealing with write errors // to maintain the same semantics as the default impl // From 4b13cfd9b9c8832e6f1877ab75605f9a3fe32f1c Mon Sep 17 00:00:00 2001 From: "main()" Date: Tue, 14 Apr 2020 12:43:38 +0200 Subject: [PATCH 3/4] Sort imports --- src/libstd/io/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/io/util.rs b/src/libstd/io/util.rs index 6904ac20f527..d909f94cdd6b 100644 --- a/src/libstd/io/util.rs +++ b/src/libstd/io/util.rs @@ -5,7 +5,7 @@ use crate::io::{self, BufRead, Initializer, IoSlice, IoSliceMut, Read, Write}; mod copy_specialization { use crate::mem::MaybeUninit; - use crate::io::{self, Read, BufRead, Write, ErrorKind}; + use crate::io::{self, BufRead, ErrorKind, Read, Write}; pub trait Copyable { fn copy_to(&mut self, writer: &mut W) -> io::Result; From b904703b87275af41dfdbdd11d1a2082b3c2dd4c Mon Sep 17 00:00:00 2001 From: "main()" Date: Tue, 14 Apr 2020 17:20:13 +0200 Subject: [PATCH 4/4] are you kidding me --- src/libstd/io/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/io/util.rs b/src/libstd/io/util.rs index d909f94cdd6b..9b5dc47759a3 100644 --- a/src/libstd/io/util.rs +++ b/src/libstd/io/util.rs @@ -4,8 +4,8 @@ use crate::fmt; use crate::io::{self, BufRead, Initializer, IoSlice, IoSliceMut, Read, Write}; mod copy_specialization { - use crate::mem::MaybeUninit; use crate::io::{self, BufRead, ErrorKind, Read, Write}; + use crate::mem::MaybeUninit; pub trait Copyable { fn copy_to(&mut self, writer: &mut W) -> io::Result;