Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split out some cold paths #215

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "smallvec"
version = "1.4.0"
version = "1.4.1"
edition = "2018"
authors = ["Simon Sapin <simon.sapin@exyr.org>"]
license = "MIT/Apache-2.0"
Expand Down
61 changes: 49 additions & 12 deletions lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ impl<T: Clone> ExtendFromSlice<T> for Vec<T> {
}

/// Error type for APIs with fallible heap allocation
#[derive(Debug)]
#[derive(Clone, Copy, Debug)]
pub enum CollectionAllocErr {
/// Overflow `usize::MAX` or other error during size computation
CapacityOverflow,
Expand All @@ -221,23 +221,33 @@ impl From<LayoutErr> for CollectionAllocErr {
CollectionAllocErr::CapacityOverflow
}
}
impl CollectionAllocErr {
#[cold]
#[inline(never)]
fn bail(&self) -> ! {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#[cold] here looks uncontroversial to me, though maybe the #[inline(never)] prevents stuff from getting potentially optimized away?

match self {
CollectionAllocErr::CapacityOverflow => panic!("capacity overflow"),
CollectionAllocErr::AllocErr { layout } => alloc::alloc::handle_alloc_error(*layout),
}
}
}

#[inline]
fn infallible<T>(result: Result<T, CollectionAllocErr>) -> T {
match result {
Ok(x) => x,
Err(CollectionAllocErr::CapacityOverflow) => panic!("capacity overflow"),
Err(CollectionAllocErr::AllocErr { layout }) => alloc::alloc::handle_alloc_error(layout),
Err(ref err) => err.bail(),
}
}

/// FIXME: use `Layout::array` when we require a Rust version where it’s stable
/// https://github.com/rust-lang/rust/issues/55724
fn layout_array<T>(n: usize) -> Result<Layout, CollectionAllocErr> {
let size = mem::size_of::<T>().checked_mul(n)
let size = mem::size_of::<T>()
.checked_mul(n)
.ok_or(CollectionAllocErr::CapacityOverflow)?;
let align = mem::align_of::<T>();
Layout::from_size_align(size, align)
.map_err(|_| CollectionAllocErr::CapacityOverflow)
Layout::from_size_align(size, align).map_err(|_| CollectionAllocErr::CapacityOverflow)
}

unsafe fn deallocate<T>(ptr: *mut T, capacity: usize) {
Expand Down Expand Up @@ -800,8 +810,19 @@ impl<A: Array> SmallVec<A> {
// from callers like insert()
let (_, &mut len, cap) = self.triple_mut();
if cap - len >= additional {
return Ok(());
Ok(())
} else {
self.try_reserve_cold(len, additional)
}
}

#[cold]
#[inline(never)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if it's worth the #[inline(never)], tbf.

fn try_reserve_cold(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure about this being tagged as #[cold] unconditionally. There's I suspect a somewhat common usage of:

let mut vec = SmallVec::new();
vec.reserve(num_elements);

for .... {
  vec.push(..);
}

For such cases #[cold] is not accurate, and #[inline(never)] is probably counter-productive.

I think only the caller knows whether the capacity is going to be likely-already enough. For example push() could use the try_reserve_cold without much trouble.

Maybe we should expose both, or something...

&mut self,
len: usize,
additional: usize,
) -> Result<(), CollectionAllocErr> {
let new_cap = len
.checked_add(additional)
.and_then(usize::checked_next_power_of_two)
Expand All @@ -817,11 +838,23 @@ impl<A: Array> SmallVec<A> {
}

/// Reserve the minimum capacity for `additional` more elements to be inserted.
#[inline]
pub fn try_reserve_exact(&mut self, additional: usize) -> Result<(), CollectionAllocErr> {
let (_, &mut len, cap) = self.triple_mut();
if cap - len >= additional {
return Ok(());
Ok(())
} else {
self.try_reserve_exact_cold(len, additional)
}
}

#[cold]
#[inline(never)]
fn try_reserve_exact_cold(
&mut self,
len: usize,
additional: usize,
) -> Result<(), CollectionAllocErr> {
let new_cap = len
.checked_add(additional)
.ok_or(CollectionAllocErr::CapacityOverflow)?;
Expand Down Expand Up @@ -1782,7 +1815,9 @@ impl<'a> Drop for SetLenOnDrop<'a> {
#[cfg(feature = "const_generics")]
unsafe impl<T, const N: usize> Array for [T; N] {
type Item = T;
fn size() -> usize { N }
fn size() -> usize {
N
}
}

#[cfg(not(feature = "const_generics"))]
Expand All @@ -1805,13 +1840,15 @@ impl_array!(
);

/// Convenience trait for constructing a `SmallVec`
pub trait ToSmallVec<A:Array> {
pub trait ToSmallVec<A: Array> {
/// Construct a new `SmallVec` from a slice.
fn to_smallvec(&self) -> SmallVec<A>;
}

impl<A:Array> ToSmallVec<A> for [A::Item]
where A::Item: Copy {
impl<A: Array> ToSmallVec<A> for [A::Item]
where
A::Item: Copy,
{
#[inline]
fn to_smallvec(&self) -> SmallVec<A> {
SmallVec::from_slice(self)
Expand Down