From a3cc435f5755f5550d4235779de58b53a22f0f1e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 11 May 2020 11:46:13 +1000 Subject: [PATCH 1/3] Remove `RawVec::double_in_place`. It's unused. --- src/liballoc/raw_vec.rs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/liballoc/raw_vec.rs b/src/liballoc/raw_vec.rs index a8e19c9cbaa..ac5399acddb 100644 --- a/src/liballoc/raw_vec.rs +++ b/src/liballoc/raw_vec.rs @@ -269,24 +269,6 @@ impl RawVec { } } - /// Attempts to double the size of the type's backing allocation in place. This is common - /// enough to want to do that it's easiest to just have a dedicated method. Slightly - /// more efficient logic can be provided for this than the general case. - /// - /// Returns `true` if the reallocation attempt has succeeded. - /// - /// # Panics - /// - /// * Panics if `T` is zero-sized on the assumption that you managed to exhaust - /// all `usize::MAX` slots in your imaginary buffer. - /// * Panics on 32-bit platforms if the requested capacity exceeds - /// `isize::MAX` bytes. - #[inline(never)] - #[cold] - pub fn double_in_place(&mut self) -> bool { - self.grow(Double, InPlace, Uninitialized).is_ok() - } - /// Ensures that the buffer contains at least enough space to hold /// `used_capacity + needed_extra_capacity` elements. If it doesn't already have /// enough capacity, will reallocate enough space plus comfortable slack From f420726566587862ef4da9153fbc2800ed444033 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 11 May 2020 12:26:59 +1000 Subject: [PATCH 2/3] Remove `RawVec::double`. It's only used once, for `VecDeque`, and can easily be replaced by something else. The commit changes `grow_if_necessary` to `grow` to avoid some small regressions caused by changed inlining. The commit also removes `Strategy::Double`, and streamlines the remaining variants of `Strategy`. It's a compile time win on some benchmarks because the many instantations of `RawVec::grow` are a little smaller. --- src/liballoc/collections/vec_deque.rs | 20 ++++-- src/liballoc/raw_vec.rs | 91 +++------------------------ 2 files changed, 23 insertions(+), 88 deletions(-) diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index 2f50234b6d5..540649c61b3 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -1354,7 +1354,9 @@ impl VecDeque { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn push_front(&mut self, value: T) { - self.grow_if_necessary(); + if self.is_full() { + self.grow(); + } self.tail = self.wrap_sub(self.tail, 1); let tail = self.tail; @@ -1377,7 +1379,9 @@ impl VecDeque { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn push_back(&mut self, value: T) { - self.grow_if_necessary(); + if self.is_full() { + self.grow(); + } let head = self.head; self.head = self.wrap_add(self.head, 1); @@ -1485,7 +1489,9 @@ impl VecDeque { #[stable(feature = "deque_extras_15", since = "1.5.0")] pub fn insert(&mut self, index: usize, value: T) { assert!(index <= self.len(), "index out of bounds"); - self.grow_if_necessary(); + if self.is_full() { + self.grow(); + } // Move the least number of elements in the ring buffer and insert // the given object @@ -2003,11 +2009,13 @@ impl VecDeque { } // This may panic or abort - #[inline] - fn grow_if_necessary(&mut self) { + #[inline(never)] + fn grow(&mut self) { if self.is_full() { let old_cap = self.cap(); - self.buf.double(); + // Double the buffer size. + self.buf.reserve_exact(old_cap, old_cap); + assert!(self.cap() == old_cap * 2); unsafe { self.handle_capacity_increase(old_cap); } diff --git a/src/liballoc/raw_vec.rs b/src/liballoc/raw_vec.rs index ac5399acddb..921ef447be0 100644 --- a/src/liballoc/raw_vec.rs +++ b/src/liballoc/raw_vec.rs @@ -211,64 +211,6 @@ impl RawVec { } } - /// Doubles the size of the type's backing allocation. This is common enough - /// to want to do that it's easiest to just have a dedicated method. Slightly - /// more efficient logic can be provided for this than the general case. - /// - /// This function is ideal for when pushing elements one-at-a-time because - /// you don't need to incur the costs of the more general computations - /// reserve needs to do to guard against overflow. You do however need to - /// manually check if your `len == capacity`. - /// - /// # Panics - /// - /// * Panics if `T` is zero-sized on the assumption that you managed to exhaust - /// all `usize::MAX` slots in your imaginary buffer. - /// * Panics on 32-bit platforms if the requested capacity exceeds - /// `isize::MAX` bytes. - /// - /// # Aborts - /// - /// Aborts on OOM - /// - /// # Examples - /// - /// ``` - /// # #![feature(raw_vec_internals)] - /// # extern crate alloc; - /// # use std::ptr; - /// # use alloc::raw_vec::RawVec; - /// struct MyVec { - /// buf: RawVec, - /// len: usize, - /// } - /// - /// impl MyVec { - /// pub fn push(&mut self, elem: T) { - /// if self.len == self.buf.capacity() { self.buf.double(); } - /// // double would have aborted or panicked if the len exceeded - /// // `isize::MAX` so this is safe to do unchecked now. - /// unsafe { - /// ptr::write(self.buf.ptr().add(self.len), elem); - /// } - /// self.len += 1; - /// } - /// } - /// # fn main() { - /// # let mut vec = MyVec { buf: RawVec::new(), len: 0 }; - /// # vec.push(1); - /// # } - /// ``` - #[inline(never)] - #[cold] - pub fn double(&mut self) { - match self.grow(Double, MayMove, Uninitialized) { - Err(CapacityOverflow) => capacity_overflow(), - Err(AllocError { layout, .. }) => handle_alloc_error(layout), - Ok(()) => { /* yay */ } - } - } - /// Ensures that the buffer contains at least enough space to hold /// `used_capacity + needed_extra_capacity` elements. If it doesn't already have /// enough capacity, will reallocate enough space plus comfortable slack @@ -336,7 +278,7 @@ impl RawVec { needed_extra_capacity: usize, ) -> Result<(), TryReserveError> { if self.needs_to_grow(used_capacity, needed_extra_capacity) { - self.grow(Amortized { used_capacity, needed_extra_capacity }, MayMove, Uninitialized) + self.grow(Amortized, used_capacity, needed_extra_capacity, MayMove, Uninitialized) } else { Ok(()) } @@ -363,7 +305,7 @@ impl RawVec { // This is more readable than putting this in one line: // `!self.needs_to_grow(...) || self.grow(...).is_ok()` if self.needs_to_grow(used_capacity, needed_extra_capacity) { - self.grow(Amortized { used_capacity, needed_extra_capacity }, InPlace, Uninitialized) + self.grow(Amortized, used_capacity, needed_extra_capacity, InPlace, Uninitialized) .is_ok() } else { true @@ -405,7 +347,7 @@ impl RawVec { needed_extra_capacity: usize, ) -> Result<(), TryReserveError> { if self.needs_to_grow(used_capacity, needed_extra_capacity) { - self.grow(Exact { used_capacity, needed_extra_capacity }, MayMove, Uninitialized) + self.grow(Exact, used_capacity, needed_extra_capacity, MayMove, Uninitialized) } else { Ok(()) } @@ -432,9 +374,8 @@ impl RawVec { #[derive(Copy, Clone)] enum Strategy { - Double, - Amortized { used_capacity: usize, needed_extra_capacity: usize }, - Exact { used_capacity: usize, needed_extra_capacity: usize }, + Amortized, + Exact, } use Strategy::*; @@ -459,6 +400,8 @@ impl RawVec { fn grow( &mut self, strategy: Strategy, + used_capacity: usize, + needed_extra_capacity: usize, placement: ReallocPlacement, init: AllocInit, ) -> Result<(), TryReserveError> { @@ -469,23 +412,7 @@ impl RawVec { return Err(CapacityOverflow); } let new_layout = match strategy { - Double => unsafe { - // Since we guarantee that we never allocate more than `isize::MAX` bytes, - // `elem_size * self.cap <= isize::MAX` as a precondition, so this can't overflow. - // Additionally the alignment will never be too large as to "not be satisfiable", - // so `Layout::from_size_align` will always return `Some`. - // - // TL;DR, we bypass runtime checks due to dynamic assertions in this module, - // allowing us to use `from_size_align_unchecked`. - let cap = if self.cap == 0 { - // Skip to 4 because tiny `Vec`'s are dumb; but not if that would cause overflow. - if elem_size > usize::MAX / 8 { 1 } else { 4 } - } else { - self.cap * 2 - }; - Layout::from_size_align_unchecked(cap * elem_size, mem::align_of::()) - }, - Amortized { used_capacity, needed_extra_capacity } => { + Amortized => { // Nothing we can really do about these checks, sadly. let required_cap = used_capacity.checked_add(needed_extra_capacity).ok_or(CapacityOverflow)?; @@ -495,7 +422,7 @@ impl RawVec { let cap = cmp::max(double_cap, required_cap); Layout::array::(cap).map_err(|_| CapacityOverflow)? } - Exact { used_capacity, needed_extra_capacity } => { + Exact => { let cap = used_capacity.checked_add(needed_extra_capacity).ok_or(CapacityOverflow)?; Layout::array::(cap).map_err(|_| CapacityOverflow)? From 68b75033ad78d88872450a81745cacfc11e58178 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 11 May 2020 13:17:28 +1000 Subject: [PATCH 3/3] Split `RawVec::grow` up. The amortized case is much more common than the exact case, and it is typically instantiated many times. Also, we can put a chunk of the code into a function that isn't generic over T, which reduces the amount of LLVM IR generated quite a lot, improving compile times. --- src/liballoc/raw_vec.rs | 129 ++++++++++++++++++++++++---------------- 1 file changed, 79 insertions(+), 50 deletions(-) diff --git a/src/liballoc/raw_vec.rs b/src/liballoc/raw_vec.rs index 921ef447be0..d46bf81f996 100644 --- a/src/liballoc/raw_vec.rs +++ b/src/liballoc/raw_vec.rs @@ -1,7 +1,7 @@ #![unstable(feature = "raw_vec_internals", reason = "implementation detail", issue = "none")] #![doc(hidden)] -use core::alloc::MemoryBlock; +use core::alloc::{LayoutErr, MemoryBlock}; use core::cmp; use core::mem::{self, ManuallyDrop, MaybeUninit}; use core::ops::Drop; @@ -278,7 +278,7 @@ impl RawVec { needed_extra_capacity: usize, ) -> Result<(), TryReserveError> { if self.needs_to_grow(used_capacity, needed_extra_capacity) { - self.grow(Amortized, used_capacity, needed_extra_capacity, MayMove, Uninitialized) + self.grow_amortized(used_capacity, needed_extra_capacity, MayMove) } else { Ok(()) } @@ -305,8 +305,7 @@ impl RawVec { // This is more readable than putting this in one line: // `!self.needs_to_grow(...) || self.grow(...).is_ok()` if self.needs_to_grow(used_capacity, needed_extra_capacity) { - self.grow(Amortized, used_capacity, needed_extra_capacity, InPlace, Uninitialized) - .is_ok() + self.grow_amortized(used_capacity, needed_extra_capacity, InPlace).is_ok() } else { true } @@ -347,7 +346,7 @@ impl RawVec { needed_extra_capacity: usize, ) -> Result<(), TryReserveError> { if self.needs_to_grow(used_capacity, needed_extra_capacity) { - self.grow(Exact, used_capacity, needed_extra_capacity, MayMove, Uninitialized) + self.grow_exact(used_capacity, needed_extra_capacity) } else { Ok(()) } @@ -372,13 +371,6 @@ impl RawVec { } } -#[derive(Copy, Clone)] -enum Strategy { - Amortized, - Exact, -} -use Strategy::*; - impl RawVec { /// Returns if the buffer needs to grow to fulfill the needed extra capacity. /// Mainly used to make inlining reserve-calls possible without inlining `grow`. @@ -396,54 +388,59 @@ impl RawVec { self.cap = Self::capacity_from_bytes(memory.size); } - /// Single method to handle all possibilities of growing the buffer. - fn grow( + // This method is usually instantiated many times. So we want it to be as + // small as possible, to improve compile times. But we also want as much of + // its contents to be statically computable as possible, to make the + // generated code run faster. Therefore, this method is carefully written + // so that all of the code that depends on `T` is within it, while as much + // of the code that doesn't depend on `T` as possible is in functions that + // are non-generic over `T`. + fn grow_amortized( &mut self, - strategy: Strategy, used_capacity: usize, needed_extra_capacity: usize, placement: ReallocPlacement, - init: AllocInit, ) -> Result<(), TryReserveError> { - let elem_size = mem::size_of::(); - if elem_size == 0 { + if mem::size_of::() == 0 { // Since we return a capacity of `usize::MAX` when `elem_size` is // 0, getting to here necessarily means the `RawVec` is overfull. return Err(CapacityOverflow); } - let new_layout = match strategy { - Amortized => { - // Nothing we can really do about these checks, sadly. - let required_cap = - used_capacity.checked_add(needed_extra_capacity).ok_or(CapacityOverflow)?; - // Cannot overflow, because `cap <= isize::MAX`, and type of `cap` is `usize`. - let double_cap = self.cap * 2; - // `double_cap` guarantees exponential growth. - let cap = cmp::max(double_cap, required_cap); - Layout::array::(cap).map_err(|_| CapacityOverflow)? - } - Exact => { - let cap = - used_capacity.checked_add(needed_extra_capacity).ok_or(CapacityOverflow)?; - Layout::array::(cap).map_err(|_| CapacityOverflow)? - } - }; - alloc_guard(new_layout.size())?; - let memory = if let Some((ptr, old_layout)) = self.current_memory() { - debug_assert_eq!(old_layout.align(), new_layout.align()); - unsafe { - self.alloc - .grow(ptr, old_layout, new_layout.size(), placement, init) - .map_err(|_| AllocError { layout: new_layout, non_exhaustive: () })? - } - } else { - match placement { - MayMove => self.alloc.alloc(new_layout, init), - InPlace => Err(AllocErr), - } - .map_err(|_| AllocError { layout: new_layout, non_exhaustive: () })? - }; + // Nothing we can really do about these checks, sadly. + let required_cap = + used_capacity.checked_add(needed_extra_capacity).ok_or(CapacityOverflow)?; + // Cannot overflow, because `cap <= isize::MAX`, and type of `cap` is `usize`. + let double_cap = self.cap * 2; + // `double_cap` guarantees exponential growth. + let cap = cmp::max(double_cap, required_cap); + let new_layout = Layout::array::(cap); + + // `finish_grow` is non-generic over `T`. + let memory = finish_grow(new_layout, placement, self.current_memory(), &mut self.alloc)?; + self.set_memory(memory); + Ok(()) + } + + // The constraints on this method are much the same as those on + // `grow_amortized`, but this method is usually instantiated less often so + // it's less critical. + fn grow_exact( + &mut self, + used_capacity: usize, + needed_extra_capacity: usize, + ) -> Result<(), TryReserveError> { + if mem::size_of::() == 0 { + // Since we return a capacity of `usize::MAX` when the type size is + // 0, getting to here necessarily means the `RawVec` is overfull. + return Err(CapacityOverflow); + } + + let cap = used_capacity.checked_add(needed_extra_capacity).ok_or(CapacityOverflow)?; + let new_layout = Layout::array::(cap); + + // `finish_grow` is non-generic over `T`. + let memory = finish_grow(new_layout, MayMove, self.current_memory(), &mut self.alloc)?; self.set_memory(memory); Ok(()) } @@ -471,6 +468,38 @@ impl RawVec { } } +// This function is outside `RawVec` to minimize compile times. See the comment +// above `RawVec::grow_amortized` for details. (The `A` parameter isn't +// significant, because the number of different `A` types seen in practice is +// much smaller than the number of `T` types.) +fn finish_grow( + new_layout: Result, + placement: ReallocPlacement, + current_memory: Option<(NonNull, Layout)>, + alloc: &mut A, +) -> Result +where + A: AllocRef, +{ + // Check for the error here to minimize the size of `RawVec::grow_*`. + let new_layout = new_layout.map_err(|_| CapacityOverflow)?; + + alloc_guard(new_layout.size())?; + + let memory = if let Some((ptr, old_layout)) = current_memory { + debug_assert_eq!(old_layout.align(), new_layout.align()); + unsafe { alloc.grow(ptr, old_layout, new_layout.size(), placement, Uninitialized) } + } else { + match placement { + MayMove => alloc.alloc(new_layout, Uninitialized), + InPlace => Err(AllocErr), + } + } + .map_err(|_| AllocError { layout: new_layout, non_exhaustive: () })?; + + Ok(memory) +} + impl RawVec { /// Converts the entire buffer into `Box<[MaybeUninit]>` with the specified `len`. ///