From eb07a61acf183818098d09e087803061dac72768 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 24 Sep 2024 13:32:29 -0700 Subject: [PATCH 1/4] Add `File::open_buffered` and `create_buffered` --- std/src/fs.rs | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/std/src/fs.rs b/std/src/fs.rs index 55f3b628ce8e9..6d1005272c603 100644 --- a/std/src/fs.rs +++ b/std/src/fs.rs @@ -375,6 +375,41 @@ impl File { OpenOptions::new().read(true).open(path.as_ref()) } + /// Attempts to open a file in read-only mode with buffering. + /// + /// See the [`OpenOptions::open`] method, the [`BufReader`][io::BufReader] type, + /// and the [`BufRead`][io::BufRead] trait for more details. + /// + /// If you only need to read the entire file contents, + /// consider [`std::fs::read()`][self::read] or + /// [`std::fs::read_to_string()`][self::read_to_string] instead. + /// + /// # Errors + /// + /// This function will return an error if `path` does not already exist. + /// Other errors may also be returned according to [`OpenOptions::open`]. + /// + /// # Examples + /// + /// ```no_run + /// #![feature(file_buffered)] + /// use std::fs::File; + /// use std::io::BufRead; + /// + /// fn main() -> std::io::Result<()> { + /// let mut f = File::open_buffered("foo.txt")?; + /// assert!(f.capacity() > 0); + /// for (line, i) in f.lines().zip(1..) { + /// println!("{i:6}: {}", line?); + /// } + /// Ok(()) + /// } + /// ``` + #[unstable(feature = "file_buffered", issue = "none")] + pub fn open_buffered>(path: P) -> io::Result> { + File::open(path).map(io::BufReader::new) + } + /// Opens a file in write-only mode. /// /// This function will create a file if it does not exist, @@ -404,6 +439,42 @@ impl File { OpenOptions::new().write(true).create(true).truncate(true).open(path.as_ref()) } + /// Opens a file in write-only mode with buffering. + /// + /// This function will create a file if it does not exist, + /// and will truncate it if it does. + /// + /// Depending on the platform, this function may fail if the + /// full directory path does not exist. + /// + /// See the [`OpenOptions::open`] method and the + /// [`BufWriter`][io::BufWriter] type for more details. + /// + /// See also [`std::fs::write()`][self::write] for a simple function to + /// create a file with some given data. + /// + /// # Examples + /// + /// ```no_run + /// #![feature(file_buffered)] + /// use std::fs::File; + /// use std::io::Write; + /// + /// fn main() -> std::io::Result<()> { + /// let mut f = File::create_buffered("foo.txt")?; + /// assert!(f.capacity() > 0); + /// for i in 0..100 { + /// writeln!(&mut f, "{i}")?; + /// } + /// f.flush()?; + /// Ok(()) + /// } + /// ``` + #[unstable(feature = "file_buffered", issue = "none")] + pub fn create_buffered>(path: P) -> io::Result> { + File::create(path).map(io::BufWriter::new) + } + /// Creates a new file in read-write mode; error if the file exists. /// /// This function will create a file if it does not exist, or return an error if it does. This From 15d69c9edf4f5e79aec62c70d55842a2e0657053 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 24 Sep 2024 13:33:31 -0700 Subject: [PATCH 2/4] Pre-allocate buffers in `File::open_buffered` and `create_buffered` --- std/src/fs.rs | 10 ++++++++-- std/src/io/buffered/bufreader.rs | 8 ++++++++ std/src/io/buffered/bufreader/buffer.rs | 12 +++++++++++- std/src/io/buffered/bufwriter.rs | 10 ++++++++++ std/src/lib.rs | 1 + 5 files changed, 38 insertions(+), 3 deletions(-) diff --git a/std/src/fs.rs b/std/src/fs.rs index 6d1005272c603..b8e3f316bebd9 100644 --- a/std/src/fs.rs +++ b/std/src/fs.rs @@ -407,7 +407,10 @@ impl File { /// ``` #[unstable(feature = "file_buffered", issue = "none")] pub fn open_buffered>(path: P) -> io::Result> { - File::open(path).map(io::BufReader::new) + // Allocate the buffer *first* so we don't affect the filesystem otherwise. + let buffer = io::BufReader::::try_new_buffer()?; + let file = File::open(path)?; + Ok(io::BufReader::with_buffer(file, buffer)) } /// Opens a file in write-only mode. @@ -472,7 +475,10 @@ impl File { /// ``` #[unstable(feature = "file_buffered", issue = "none")] pub fn create_buffered>(path: P) -> io::Result> { - File::create(path).map(io::BufWriter::new) + // Allocate the buffer *first* so we don't affect the filesystem otherwise. + let buffer = io::BufWriter::::try_new_buffer()?; + let file = File::create(path)?; + Ok(io::BufWriter::with_buffer(file, buffer)) } /// Creates a new file in read-write mode; error if the file exists. diff --git a/std/src/io/buffered/bufreader.rs b/std/src/io/buffered/bufreader.rs index e51dde994de45..fcb3e36027bab 100644 --- a/std/src/io/buffered/bufreader.rs +++ b/std/src/io/buffered/bufreader.rs @@ -74,6 +74,14 @@ impl BufReader { BufReader::with_capacity(DEFAULT_BUF_SIZE, inner) } + pub(crate) fn try_new_buffer() -> io::Result { + Buffer::try_with_capacity(DEFAULT_BUF_SIZE) + } + + pub(crate) fn with_buffer(inner: R, buf: Buffer) -> Self { + Self { inner, buf } + } + /// Creates a new `BufReader` with the specified buffer capacity. /// /// # Examples diff --git a/std/src/io/buffered/bufreader/buffer.rs b/std/src/io/buffered/bufreader/buffer.rs index 1bf84d8bef312..3df7e3971dac4 100644 --- a/std/src/io/buffered/bufreader/buffer.rs +++ b/std/src/io/buffered/bufreader/buffer.rs @@ -10,7 +10,7 @@ //! without encountering any runtime bounds checks. use crate::cmp; -use crate::io::{self, BorrowedBuf, Read}; +use crate::io::{self, BorrowedBuf, ErrorKind, Read}; use crate::mem::MaybeUninit; pub struct Buffer { @@ -36,6 +36,16 @@ impl Buffer { Self { buf, pos: 0, filled: 0, initialized: 0 } } + #[inline] + pub fn try_with_capacity(capacity: usize) -> io::Result { + match Box::try_new_uninit_slice(capacity) { + Ok(buf) => Ok(Self { buf, pos: 0, filled: 0, initialized: 0 }), + Err(_) => { + Err(io::const_io_error!(ErrorKind::OutOfMemory, "failed to allocate read buffer")) + } + } + } + #[inline] pub fn buffer(&self) -> &[u8] { // SAFETY: self.pos and self.cap are valid, and self.cap => self.pos, and diff --git a/std/src/io/buffered/bufwriter.rs b/std/src/io/buffered/bufwriter.rs index 13516d3b961f1..c41bae2aa4e81 100644 --- a/std/src/io/buffered/bufwriter.rs +++ b/std/src/io/buffered/bufwriter.rs @@ -94,6 +94,16 @@ impl BufWriter { BufWriter::with_capacity(DEFAULT_BUF_SIZE, inner) } + pub(crate) fn try_new_buffer() -> io::Result> { + Vec::try_with_capacity(DEFAULT_BUF_SIZE).map_err(|_| { + io::const_io_error!(ErrorKind::OutOfMemory, "failed to allocate write buffer") + }) + } + + pub(crate) fn with_buffer(inner: W, buf: Vec) -> Self { + Self { inner, buf, panicked: false } + } + /// Creates a new `BufWriter` with at least the specified buffer capacity. /// /// # Examples diff --git a/std/src/lib.rs b/std/src/lib.rs index 4d93af6ea652e..b81e7c18abbb0 100644 --- a/std/src/lib.rs +++ b/std/src/lib.rs @@ -374,6 +374,7 @@ #![feature(slice_concat_trait)] #![feature(thin_box)] #![feature(try_reserve_kind)] +#![feature(try_with_capacity)] #![feature(vec_into_raw_parts)] // tidy-alphabetical-end // From 963cefb2cd1985e53c128274d579a2845a218009 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 24 Sep 2024 14:25:16 -0700 Subject: [PATCH 3/4] Dogfood `feature(file_buffered)` --- std/src/sys/pal/unix/thread.rs | 4 ++-- test/src/lib.rs | 1 + test/src/term/terminfo/mod.rs | 6 ++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/std/src/sys/pal/unix/thread.rs b/std/src/sys/pal/unix/thread.rs index 7fe9b6c3e52f4..ce5e8ea5866d5 100644 --- a/std/src/sys/pal/unix/thread.rs +++ b/std/src/sys/pal/unix/thread.rs @@ -517,7 +517,7 @@ mod cgroups { use crate::borrow::Cow; use crate::ffi::OsString; use crate::fs::{File, exists}; - use crate::io::{BufRead, BufReader, Read}; + use crate::io::{BufRead, Read}; use crate::os::unix::ffi::OsStringExt; use crate::path::{Path, PathBuf}; use crate::str::from_utf8; @@ -690,7 +690,7 @@ mod cgroups { /// If the cgroupfs is a bind mount then `group_path` is adjusted to skip /// over the already-included prefix fn find_mountpoint(group_path: &Path) -> Option<(Cow<'static, str>, &Path)> { - let mut reader = BufReader::new(File::open("/proc/self/mountinfo").ok()?); + let mut reader = File::open_buffered("/proc/self/mountinfo").ok()?; let mut line = String::with_capacity(256); loop { line.clear(); diff --git a/test/src/lib.rs b/test/src/lib.rs index ebbe50cc65154..4b2c65cfdf548 100644 --- a/test/src/lib.rs +++ b/test/src/lib.rs @@ -18,6 +18,7 @@ #![doc(test(attr(deny(warnings))))] #![doc(rust_logo)] #![feature(rustdoc_internals)] +#![feature(file_buffered)] #![feature(internal_output_capture)] #![feature(staged_api)] #![feature(process_exitcode_internals)] diff --git a/test/src/term/terminfo/mod.rs b/test/src/term/terminfo/mod.rs index ac10ec2b850ec..974b8afd598dd 100644 --- a/test/src/term/terminfo/mod.rs +++ b/test/src/term/terminfo/mod.rs @@ -3,9 +3,8 @@ use std::collections::HashMap; use std::fs::File; use std::io::prelude::*; -use std::io::{self, BufReader}; use std::path::Path; -use std::{env, error, fmt}; +use std::{env, error, fmt, io}; use parm::{Param, Variables, expand}; use parser::compiled::{msys_terminfo, parse}; @@ -102,8 +101,7 @@ impl TermInfo { } // Keep the metadata small fn _from_path(path: &Path) -> Result { - let file = File::open(path).map_err(Error::IoError)?; - let mut reader = BufReader::new(file); + let mut reader = File::open_buffered(path).map_err(Error::IoError)?; parse(&mut reader, false).map_err(Error::MalformedTerminfo) } } From 2ab86f01c97b62e6064c5b36822e4176622c3939 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 24 Sep 2024 15:06:55 -0700 Subject: [PATCH 4/4] Add a tracking issue for `file_buffered` --- std/src/fs.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/std/src/fs.rs b/std/src/fs.rs index b8e3f316bebd9..db7867337dd59 100644 --- a/std/src/fs.rs +++ b/std/src/fs.rs @@ -405,7 +405,7 @@ impl File { /// Ok(()) /// } /// ``` - #[unstable(feature = "file_buffered", issue = "none")] + #[unstable(feature = "file_buffered", issue = "130804")] pub fn open_buffered>(path: P) -> io::Result> { // Allocate the buffer *first* so we don't affect the filesystem otherwise. let buffer = io::BufReader::::try_new_buffer()?; @@ -473,7 +473,7 @@ impl File { /// Ok(()) /// } /// ``` - #[unstable(feature = "file_buffered", issue = "none")] + #[unstable(feature = "file_buffered", issue = "130804")] pub fn create_buffered>(path: P) -> io::Result> { // Allocate the buffer *first* so we don't affect the filesystem otherwise. let buffer = io::BufWriter::::try_new_buffer()?;