From c40ac57efb88b308b869be2ec47da59aba3c842c Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Tue, 28 Dec 2021 11:28:05 +0800 Subject: [PATCH 1/7] Add try_reserve for OsString Signed-off-by: Xuanwo --- library/std/src/ffi/os_str.rs | 81 ++++++++++++++++++++++++++++++ library/std/src/sys/unix/os_str.rs | 11 ++++ 2 files changed, 92 insertions(+) diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs index 0f9912fa64d8b..2933018a49536 100644 --- a/library/std/src/ffi/os_str.rs +++ b/library/std/src/ffi/os_str.rs @@ -3,6 +3,7 @@ mod tests; use crate::borrow::{Borrow, Cow}; use crate::cmp; +use crate::collections::TryReserveError; use crate::fmt; use crate::hash::{Hash, Hasher}; use crate::iter::{Extend, FromIterator}; @@ -265,6 +266,43 @@ impl OsString { self.inner.reserve(additional) } + /// Tries to reserve capacity for at least `additional` more elements to be inserted + /// in the given `OsString`. The collection may reserve more space to avoid + /// frequent reallocations. After calling `try_reserve`, capacity will be + /// greater than or equal to `self.len() + additional`. Does nothing if + /// capacity is already sufficient. + /// + /// # Errors + /// + /// If the capacity overflows, or the allocator reports a failure, then an error + /// is returned. + /// + /// # Examples + /// + /// ``` + /// #![feature(try_reserve_2)] + /// use std::ffi::OsString; + /// use std::collections::TryReserveError; + /// + /// fn find_max_slow(data: &str) -> Result { + /// let mut s = OsString::new(); + /// + /// // Pre-reserve the memory, exiting if we can't + /// s.try_reserve(data.len())?; + /// + /// // Now we know this can't OOM in the middle of our complex work + /// s.push(data); + /// + /// Ok(s) + /// } + /// # find_max_slow("123").expect("why is the test harness OOMing on 12 bytes?"); + /// ``` + #[unstable(feature = "try_reserve_2", issue = "91789")] + #[inline] + pub fn try_reserve(&mut self, additional: usize) -> Result<(), TryReserveError> { + self.inner.try_reserve(additional) + } + /// Reserves the minimum capacity for exactly `additional` more capacity to /// be inserted in the given `OsString`. Does nothing if the capacity is /// already sufficient. @@ -290,6 +328,49 @@ impl OsString { self.inner.reserve_exact(additional) } + /// Tries to reserve the minimum capacity for exactly `additional` + /// elements to be inserted in the given `OsString`. After calling + /// `try_reserve_exact`, capacity will be greater than or equal to + /// `self.len() + additional` if it returns `Ok(())`. + /// Does nothing if the capacity is already sufficient. + /// + /// Note that the allocator may give the collection more space than it + /// requests. Therefore, capacity can not be relied upon to be precisely + /// minimal. Prefer [`try_reserve`] if future insertions are expected. + /// + /// [`try_reserve`]: OsString::try_reserve + /// + /// # Errors + /// + /// If the capacity overflows, or the allocator reports a failure, then an error + /// is returned. + /// + /// # Examples + /// + /// ``` + /// #![feature(try_reserve_2)] + /// use std::ffi::OsString; + /// use std::collections::TryReserveError; + /// + /// fn find_max_slow(data: &str) -> Result { + /// let mut s = OsString::from(data); + /// + /// // Pre-reserve the memory, exiting if we can't + /// s.try_reserve_exact(data.len())?; + /// + /// // Now we know this can't OOM in the middle of our complex work + /// s.push(data); + /// + /// Ok(s) + /// } + /// # find_max_slow("123").expect("why is the test harness OOMing on 12 bytes?"); + /// ``` + #[unstable(feature = "try_reserve_2", issue = "91789")] + #[inline] + pub fn try_reserve_exact(&mut self, additional: usize) -> Result<(), TryReserveError> { + self.inner.try_reserve_exact(additional) + } + /// Shrinks the capacity of the `OsString` to match its length. /// /// # Examples diff --git a/library/std/src/sys/unix/os_str.rs b/library/std/src/sys/unix/os_str.rs index ae96d6b4df44e..ccbc182240cf3 100644 --- a/library/std/src/sys/unix/os_str.rs +++ b/library/std/src/sys/unix/os_str.rs @@ -2,6 +2,7 @@ //! systems: just a `Vec`/`[u8]`. use crate::borrow::Cow; +use crate::collections::TryReserveError; use crate::fmt; use crate::fmt::Write; use crate::mem; @@ -112,11 +113,21 @@ impl Buf { self.inner.reserve(additional) } + #[inline] + pub fn try_reserve(&mut self, additional: usize) -> Result<(), TryReserveError> { + self.inner.try_reserve(additional) + } + #[inline] pub fn reserve_exact(&mut self, additional: usize) { self.inner.reserve_exact(additional) } + #[inline] + pub fn try_reserve_exact(&mut self, additional: usize) -> Result<(), TryReserveError> { + self.inner.try_reserve_exact(additional) + } + #[inline] pub fn shrink_to_fit(&mut self) { self.inner.shrink_to_fit() From 013fbc61877c8b1ca964274f171bd79952247fc3 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Tue, 28 Dec 2021 11:40:58 +0800 Subject: [PATCH 2/7] Fix windows build Signed-off-by: Xuanwo --- library/std/src/sys/windows/os_str.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/library/std/src/sys/windows/os_str.rs b/library/std/src/sys/windows/os_str.rs index 7e09a4fd56130..78e92a3331a1c 100644 --- a/library/std/src/sys/windows/os_str.rs +++ b/library/std/src/sys/windows/os_str.rs @@ -1,6 +1,7 @@ /// The underlying OsString/OsStr implementation on Windows is a /// wrapper around the "WTF-8" encoding; see the `wtf8` module for more. use crate::borrow::Cow; +use crate::collections::TryReserveError; use crate::fmt; use crate::mem; use crate::rc::Rc; @@ -104,10 +105,18 @@ impl Buf { self.inner.reserve(additional) } + pub fn try_reserve(&mut self, additional: usize) -> Result<(), TryReserveError> { + self.inner.try_reserve(additional) + } + pub fn reserve_exact(&mut self, additional: usize) { self.inner.reserve_exact(additional) } + pub fn try_reserve_exact(&mut self, additional: usize) -> Result<(), TryReserveError> { + self.inner.try_reserve_exact(additional) + } + pub fn shrink_to_fit(&mut self) { self.inner.shrink_to_fit() } From 27b92c9f98a0ea3070b988819bb3aa1111ed0382 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Tue, 28 Dec 2021 11:53:14 +0800 Subject: [PATCH 3/7] Implement support in wtf8 Signed-off-by: Xuanwo --- library/std/src/sys_common/wtf8.rs | 37 ++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/library/std/src/sys_common/wtf8.rs b/library/std/src/sys_common/wtf8.rs index 6e29bc61454c8..3f3f4abd214c6 100644 --- a/library/std/src/sys_common/wtf8.rs +++ b/library/std/src/sys_common/wtf8.rs @@ -22,6 +22,7 @@ use core::str::next_code_point; use crate::borrow::Cow; use crate::char; +use crate::collections::TryReserveError; use crate::fmt; use crate::hash::{Hash, Hasher}; use crate::iter::FromIterator; @@ -231,11 +232,47 @@ impl Wtf8Buf { self.bytes.reserve(additional) } + /// Tries to reserve capacity for at least `additional` more elements to be inserted + /// in the given `Wtf8Buf`. The collection may reserve more space to avoid + /// frequent reallocations. After calling `try_reserve`, capacity will be + /// greater than or equal to `self.len() + additional`. Does nothing if + /// capacity is already sufficient. + /// + /// # Errors + /// + /// If the capacity overflows, or the allocator reports a failure, then an error + /// is returned. + #[inline] + pub fn try_reserve(&mut self, additional: usize) -> Result<(), TryReserveError> { + self.bytes.try_reserve(additional) + } + #[inline] pub fn reserve_exact(&mut self, additional: usize) { self.bytes.reserve_exact(additional) } + /// Tries to reserve the minimum capacity for exactly `additional` + /// elements to be inserted in the given `Wtf8Buf`. After calling + /// `try_reserve_exact`, capacity will be greater than or equal to + /// `self.len() + additional` if it returns `Ok(())`. + /// Does nothing if the capacity is already sufficient. + /// + /// Note that the allocator may give the collection more space than it + /// requests. Therefore, capacity can not be relied upon to be precisely + /// minimal. Prefer [`try_reserve`] if future insertions are expected. + /// + /// [`try_reserve`]: Wtf8Buf::try_reserve + /// + /// # Errors + /// + /// If the capacity overflows, or the allocator reports a failure, then an error + /// is returned. + #[inline] + pub fn try_reserve_exact(&mut self, additional: usize) -> Result<(), TryReserveError> { + self.bytes.try_reserve_exact(additional) + } + #[inline] pub fn shrink_to_fit(&mut self) { self.bytes.shrink_to_fit() From 9166428be18f5348b2c6cb5301986d29e5a0af9c Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Wed, 29 Dec 2021 13:49:39 +0800 Subject: [PATCH 4/7] Update library/std/src/ffi/os_str.rs Co-authored-by: David Tolnay --- library/std/src/ffi/os_str.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs index 2933018a49536..9b853fba41b79 100644 --- a/library/std/src/ffi/os_str.rs +++ b/library/std/src/ffi/os_str.rs @@ -267,7 +267,7 @@ impl OsString { } /// Tries to reserve capacity for at least `additional` more elements to be inserted - /// in the given `OsString`. The collection may reserve more space to avoid + /// in the given `OsString`. The string may reserve more space to avoid /// frequent reallocations. After calling `try_reserve`, capacity will be /// greater than or equal to `self.len() + additional`. Does nothing if /// capacity is already sufficient. From b07ae1c4d5232abcb89151eb64d81631d11a5d14 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Wed, 29 Dec 2021 14:02:20 +0800 Subject: [PATCH 5/7] Address comments Signed-off-by: Xuanwo --- library/std/src/ffi/os_str.rs | 12 ++++++------ library/std/src/sys_common/wtf8.rs | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs index 9b853fba41b79..b3ad3fe8c2c7e 100644 --- a/library/std/src/ffi/os_str.rs +++ b/library/std/src/ffi/os_str.rs @@ -266,7 +266,7 @@ impl OsString { self.inner.reserve(additional) } - /// Tries to reserve capacity for at least `additional` more elements to be inserted + /// Tries to reserve capacity for at least `additional` more length units /// in the given `OsString`. The string may reserve more space to avoid /// frequent reallocations. After calling `try_reserve`, capacity will be /// greater than or equal to `self.len() + additional`. Does nothing if @@ -288,7 +288,7 @@ impl OsString { /// let mut s = OsString::new(); /// /// // Pre-reserve the memory, exiting if we can't - /// s.try_reserve(data.len())?; + /// s.try_reserve(OsString::from(data).len())?; /// /// // Now we know this can't OOM in the middle of our complex work /// s.push(data); @@ -329,12 +329,12 @@ impl OsString { } /// Tries to reserve the minimum capacity for exactly `additional` - /// elements to be inserted in the given `OsString`. After calling + /// more length units in the given `OsString`. After calling /// `try_reserve_exact`, capacity will be greater than or equal to /// `self.len() + additional` if it returns `Ok(())`. /// Does nothing if the capacity is already sufficient. /// - /// Note that the allocator may give the collection more space than it + /// Note that the allocator may give the `OsString` more space than it /// requests. Therefore, capacity can not be relied upon to be precisely /// minimal. Prefer [`try_reserve`] if future insertions are expected. /// @@ -353,10 +353,10 @@ impl OsString { /// use std::collections::TryReserveError; /// /// fn find_max_slow(data: &str) -> Result { - /// let mut s = OsString::from(data); + /// let mut s = OsString::new(); /// /// // Pre-reserve the memory, exiting if we can't - /// s.try_reserve_exact(data.len())?; + /// s.try_reserve_exact(OsString::from(data).len())?; /// /// // Now we know this can't OOM in the middle of our complex work /// s.push(data); diff --git a/library/std/src/sys_common/wtf8.rs b/library/std/src/sys_common/wtf8.rs index 3f3f4abd214c6..7a6e6246357d1 100644 --- a/library/std/src/sys_common/wtf8.rs +++ b/library/std/src/sys_common/wtf8.rs @@ -232,8 +232,8 @@ impl Wtf8Buf { self.bytes.reserve(additional) } - /// Tries to reserve capacity for at least `additional` more elements to be inserted - /// in the given `Wtf8Buf`. The collection may reserve more space to avoid + /// Tries to reserve capacity for at least `additional` more length units + /// in the given `Wtf8Buf`. The `Wtf8Buf` may reserve more space to avoid /// frequent reallocations. After calling `try_reserve`, capacity will be /// greater than or equal to `self.len() + additional`. Does nothing if /// capacity is already sufficient. @@ -253,12 +253,12 @@ impl Wtf8Buf { } /// Tries to reserve the minimum capacity for exactly `additional` - /// elements to be inserted in the given `Wtf8Buf`. After calling + /// length units in the given `Wtf8Buf`. After calling /// `try_reserve_exact`, capacity will be greater than or equal to /// `self.len() + additional` if it returns `Ok(())`. /// Does nothing if the capacity is already sufficient. /// - /// Note that the allocator may give the collection more space than it + /// Note that the allocator may give the `Wtf8Buf` more space than it /// requests. Therefore, capacity can not be relied upon to be precisely /// minimal. Prefer [`try_reserve`] if future insertions are expected. /// From 1f62c24d5a11fcbc3b9d489bcac9492daf7fb82a Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Thu, 30 Dec 2021 12:40:51 -0800 Subject: [PATCH 6/7] Fix some copy/paste hysteresis in OsString try_reserve docs It appears `find_max_slow` comes from the BinaryHeap docs, where the try_reserve example is a slow implementation of find_max. It has no relevance to this code in OsString though. --- library/std/src/ffi/os_str.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs index b3ad3fe8c2c7e..8424e56556952 100644 --- a/library/std/src/ffi/os_str.rs +++ b/library/std/src/ffi/os_str.rs @@ -284,7 +284,7 @@ impl OsString { /// use std::ffi::OsString; /// use std::collections::TryReserveError; /// - /// fn find_max_slow(data: &str) -> Result { + /// fn process_data(data: &str) -> Result { /// let mut s = OsString::new(); /// /// // Pre-reserve the memory, exiting if we can't @@ -295,7 +295,7 @@ impl OsString { /// /// Ok(s) /// } - /// # find_max_slow("123").expect("why is the test harness OOMing on 12 bytes?"); + /// # process_data("123").expect("why is the test harness OOMing on 3 bytes?"); /// ``` #[unstable(feature = "try_reserve_2", issue = "91789")] #[inline] @@ -352,7 +352,7 @@ impl OsString { /// use std::ffi::OsString; /// use std::collections::TryReserveError; /// - /// fn find_max_slow(data: &str) -> Result { + /// fn process_data(data: &str) -> Result { /// let mut s = OsString::new(); /// /// // Pre-reserve the memory, exiting if we can't @@ -363,7 +363,7 @@ impl OsString { /// /// Ok(s) /// } - /// # find_max_slow("123").expect("why is the test harness OOMing on 12 bytes?"); + /// # process_data("123").expect("why is the test harness OOMing on 3 bytes?"); /// ``` #[unstable(feature = "try_reserve_2", issue = "91789")] #[inline] From d29941e7249c73327317b0c3ebaa98ae3228bcfc Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Thu, 30 Dec 2021 12:45:02 -0800 Subject: [PATCH 7/7] Remove needless allocation from example code of OsString --- library/std/src/ffi/os_str.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs index 8424e56556952..982ad1898788e 100644 --- a/library/std/src/ffi/os_str.rs +++ b/library/std/src/ffi/os_str.rs @@ -281,14 +281,14 @@ impl OsString { /// /// ``` /// #![feature(try_reserve_2)] - /// use std::ffi::OsString; + /// use std::ffi::{OsStr, OsString}; /// use std::collections::TryReserveError; /// /// fn process_data(data: &str) -> Result { /// let mut s = OsString::new(); /// /// // Pre-reserve the memory, exiting if we can't - /// s.try_reserve(OsString::from(data).len())?; + /// s.try_reserve(OsStr::new(data).len())?; /// /// // Now we know this can't OOM in the middle of our complex work /// s.push(data); @@ -349,14 +349,14 @@ impl OsString { /// /// ``` /// #![feature(try_reserve_2)] - /// use std::ffi::OsString; + /// use std::ffi::{OsStr, OsString}; /// use std::collections::TryReserveError; /// /// fn process_data(data: &str) -> Result { /// let mut s = OsString::new(); /// /// // Pre-reserve the memory, exiting if we can't - /// s.try_reserve_exact(OsString::from(data).len())?; + /// s.try_reserve_exact(OsStr::new(data).len())?; /// /// // Now we know this can't OOM in the middle of our complex work /// s.push(data);