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

Don’t heap-allocate for zero-size items #228

Merged
merged 1 commit into from Jul 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
@@ -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
50 changes: 38 additions & 12 deletions lib.rs
Expand Up @@ -462,8 +462,8 @@ unsafe impl<A: Array + Sync> Sync for SmallVecData<A> {}
/// ```
pub struct SmallVec<A: Array> {
// The capacity field is used to determine which of the storage variants is active:
// If capacity <= A::size() then the inline variant is used and capacity holds the current length of the vector (number of elements actually in use).
// If capacity > A::size() then the heap variant is used and capacity holds the size of the memory allocation.
// If capacity <= Self::inline_capacity() then the inline variant is used and capacity holds the current length of the vector (number of elements actually in use).
// If capacity > Self::inline_capacity() then the heap variant is used and capacity holds the size of the memory allocation.
capacity: usize,
data: SmallVecData<A>,
}
Expand Down Expand Up @@ -506,7 +506,7 @@ impl<A: Array> SmallVec<A> {

/// Construct a new `SmallVec` from a `Vec<A::Item>`.
///
/// Elements will be copied to the inline buffer if vec.capacity() <= A::size().
/// Elements will be copied to the inline buffer if vec.capacity() <= Self::inline_capacity().
///
/// ```rust
/// use smallvec::SmallVec;
Expand All @@ -518,7 +518,7 @@ impl<A: Array> SmallVec<A> {
/// ```
#[inline]
pub fn from_vec(mut vec: Vec<A::Item>) -> SmallVec<A> {
if vec.capacity() <= A::size() {
if vec.capacity() <= Self::inline_capacity() {
unsafe {
let mut data = SmallVecData::<A>::from_inline(MaybeUninit::uninit());
let len = vec.len();
Expand Down Expand Up @@ -611,10 +611,30 @@ impl<A: Array> SmallVec<A> {
*len_ptr = new_len;
}

/// The maximum number of elements this vector can hold inline
#[inline]
fn inline_capacity() -> usize {
if mem::size_of::<A::Item>() > 0 {
A::size()
} else {
// For zero-size items code like `ptr.add(offset)` always returns the same pointer.
// Therefore all items are at the same address,
// and any array size has capacity for infinitely many items.
// The capacity is limited by the bit width of the length field.
//
// `Vec` also does this:
// https://github.com/rust-lang/rust/blob/1.44.0/src/liballoc/raw_vec.rs#L186
//
// In our case, this also ensures that a smallvec of zero-size items never spills,
// and we never try to allocate zero bytes which `std::alloc::alloc` disallows.
core::usize::MAX
}
}

/// The maximum number of elements this vector can hold inline
#[inline]
pub fn inline_size(&self) -> usize {
Copy link
Member Author

Choose a reason for hiding this comment

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

This should not take &self, but it’s public so meh

A::size()
Self::inline_capacity()
}

/// The number of elements stored in the vector
Expand Down Expand Up @@ -644,7 +664,7 @@ impl<A: Array> SmallVec<A> {
let (ptr, len) = self.data.heap();
(ptr, len, self.capacity)
} else {
(self.data.inline(), self.capacity, A::size())
(self.data.inline(), self.capacity, Self::inline_capacity())
}
}
}
Expand All @@ -657,15 +677,15 @@ impl<A: Array> SmallVec<A> {
let &mut (ptr, ref mut len_ptr) = self.data.heap_mut();
(ptr, len_ptr, self.capacity)
} else {
(self.data.inline_mut(), &mut self.capacity, A::size())
(self.data.inline_mut(), &mut self.capacity, Self::inline_capacity())
}
}
}

/// Returns `true` if the data has spilled into a separate heap-allocated buffer.
#[inline]
pub fn spilled(&self) -> bool {
self.capacity > A::size()
self.capacity > Self::inline_capacity()
}

/// Creates a draining iterator that removes the specified range in the vector
Expand Down Expand Up @@ -770,6 +790,7 @@ impl<A: Array> SmallVec<A> {
deallocate(ptr, cap);
} else if new_cap != cap {
let layout = layout_array::<A::Item>(new_cap)?;
debug_assert!(layout.size() > 0);
let new_alloc;
if unspilled {
new_alloc = NonNull::new(alloc::alloc::alloc(layout))
Expand Down Expand Up @@ -1029,7 +1050,7 @@ impl<A: Array> SmallVec<A> {
/// This method returns `Err(Self)` if the SmallVec is too short (and the `A` contains uninitialized elements),
/// or if the SmallVec is too long (and all the elements were spilled to the heap).
pub fn into_inner(self) -> Result<A, Self> {
if self.spilled() || self.len() != A::size() {
if self.spilled() || self.len() != A::size() { // Note: A::size, not Self::inline_capacity
Err(self)
} else {
unsafe {
Expand Down Expand Up @@ -1219,7 +1240,7 @@ impl<A: Array> SmallVec<A> {
/// }
#[inline]
pub unsafe fn from_raw_parts(ptr: *mut A::Item, length: usize, capacity: usize) -> SmallVec<A> {
assert!(capacity > A::size());
assert!(capacity > Self::inline_capacity());
SmallVec {
capacity,
data: SmallVecData::from_heap(ptr, length),
Expand All @@ -1236,7 +1257,7 @@ where
/// For slices of `Copy` types, this is more efficient than `SmallVec::from(slice)`.
pub fn from_slice(slice: &[A::Item]) -> Self {
let len = slice.len();
if len <= A::size() {
if len <= Self::inline_capacity() {
SmallVec {
capacity: len,
data: SmallVecData::from_inline(unsafe {
Expand Down Expand Up @@ -1317,7 +1338,7 @@ where
/// assert_eq!(v, SmallVec::from_buf(['d', 'd']));
/// ```
pub fn from_elem(elem: A::Item, n: usize) -> Self {
if n > A::size() {
if n > Self::inline_capacity() {
vec![elem; n].into()
} else {
let mut v = SmallVec::<A>::new();
Expand Down Expand Up @@ -2731,4 +2752,9 @@ mod tests {
fn empty_macro() {
let _v: SmallVec<[u8; 1]> = smallvec![];
}

#[test]
fn zero_size_items() {
SmallVec::<[(); 0]>::new().push(());
}
}