Skip to content

Revert nonnull optimizations and remove as casts #53

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.

Sign up for GitHub

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

Open
dilr wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Revert nonnull optimizations and remove as casts #53

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e852ffb
Revert "perf: use `NonNull` for null pointer optimization (#45)"
dilr Aug 3, 2025
cd42e55
`ptr.addr()` not stable in rust 1.80, switch back to as cast.
dilr Aug 3, 2025
6838f73
Update src/smallbox.rs
dilr Aug 6, 2025
84d32cc
fix
andylokandy Aug 7, 2025
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
115 changes: 37 additions & 78 deletions src/smallbox.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use core::fmt;
use core::future::Future;
use core::hash::Hash;
use core::hash::{self};
use core::hint::unreachable_unchecked;
use core::marker::PhantomData;
#[cfg(feature = "coerce")]
use core::marker::Unsize;
Expand All @@ -17,7 +16,6 @@ use core::ops;
use core::ops::CoerceUnsized;
use core::pin::Pin;
use core::ptr;
use core::ptr::NonNull;

use ::alloc::alloc;
use ::alloc::alloc::Layout;
Expand All @@ -26,17 +24,6 @@ use ::alloc::boxed::Box;

use crate::sptr;

/// A sentinel pointer that signals that the value is stored on the stack
///
/// It is never supposed to be dereferenced
const INLINE_SENTINEL: *mut u8 = sptr::without_provenance_mut(0x1);

/// Minimum alignment for allocations
///
/// Forcing a minimum alignment prevents the allocator
/// from returning a pointer with the same address as `INLINE_SENTINEL`
const MIN_ALIGNMENT: usize = 2;

#[cfg(feature = "coerce")]
impl<T: ?Sized + Unsize<U>, U: ?Sized, Space> CoerceUnsized<SmallBox<U, Space>>
for SmallBox<T, Space>
Expand Down Expand Up @@ -87,7 +74,7 @@ macro_rules! smallbox {
/// An optimized box that store value on stack or on heap depending on its size
pub struct SmallBox<T: ?Sized, Space> {
space: MaybeUninit<UnsafeCell<Space>>,
ptr: NonNull<T>,
ptr: *const T,
_phantom: PhantomData<T>,
}

Expand Down Expand Up @@ -177,49 +164,39 @@ impl<T: ?Sized, Space> SmallBox<T, Space> {
/// ```
#[inline]
pub fn is_heap(&self) -> bool {
self.ptr.as_ptr().cast::<u8>() != INLINE_SENTINEL
!self.ptr.is_null()
}

unsafe fn new_copy<U>(val: &U, metadata_ptr: *const T) -> SmallBox<T, Space>
where U: ?Sized {
let layout = Layout::for_value::<U>(val);
let space_layout = Layout::new::<Space>();
let size = mem::size_of_val(val);
let align = mem::align_of_val(val);

let mut space = MaybeUninit::<UnsafeCell<Space>>::uninit();

let (ptr_this, val_dst): (*mut u8, *mut u8) =
if layout.size() <= space_layout.size() && layout.align() <= space_layout.align() {
// Stack.
(INLINE_SENTINEL, space.as_mut_ptr().cast())
} else if layout.size() == 0 {
// ZST with alignment greater than Space, which will behave like being stored on
// heap but will not actually allocate.
(
sptr::without_provenance_mut(layout.align()),
sptr::without_provenance_mut(layout.align()),
)
} else {
// Heap.
let layout = layout
// Safety: MIN_ALIGNMENT is 2, which is a valid power-of-two alignment.
.align_to(MIN_ALIGNMENT)
.unwrap_or_else(|_| unreachable_unchecked());
let heap_ptr = alloc::alloc(layout);

if heap_ptr.is_null() {
handle_alloc_error(layout)
}
let (ptr_this, val_dst): (*mut u8, *mut u8) = if size == 0 {
(
sptr::without_provenance_mut(align),
sptr::without_provenance_mut(align),
)
} else if size > mem::size_of::<Space>() || align > mem::align_of::<Space>() {
Comment thread
dilr marked this conversation as resolved.
Outdated
// Heap
let layout = Layout::for_value::<U>(val);
let heap_ptr = alloc::alloc(layout);

if heap_ptr.is_null() {
handle_alloc_error(layout)
}

(heap_ptr, heap_ptr)
};
(heap_ptr, heap_ptr)
} else {
(ptr::null_mut(), space.as_mut_ptr().cast())
};

// `self.ptr` always holds the metadata, even if stack allocated.
let ptr = sptr::with_metadata_of_mut(ptr_this, metadata_ptr);
// Safety: ptr is either INLINE_SENTINEL or returned from the allocator and checked for
// null.
let ptr = NonNull::new_unchecked(ptr);

ptr::copy_nonoverlapping(sptr::from_ref(val).cast(), val_dst, layout.size());
ptr::copy_nonoverlapping(sptr::from_ref(val).cast(), val_dst, size);

SmallBox {
space,
Expand Down Expand Up @@ -254,18 +231,18 @@ impl<T: ?Sized, Space> SmallBox<T, Space> {
#[inline]
unsafe fn as_ptr(&self) -> *const T {
if self.is_heap() {
self.ptr.as_ptr()
self.ptr
} else {
sptr::with_metadata_of(self.space.as_ptr(), self.ptr.as_ptr())
sptr::with_metadata_of(self.space.as_ptr(), self.ptr)
}
}

#[inline]
unsafe fn as_mut_ptr(&mut self) -> *mut T {
if self.is_heap() {
self.ptr.as_ptr()
self.ptr.cast_mut()
} else {
sptr::with_metadata_of_mut(self.space.as_mut_ptr(), self.ptr.as_ptr())
sptr::with_metadata_of_mut(self.space.as_mut_ptr(), self.ptr.cast_mut())
}
}

Expand All @@ -292,14 +269,9 @@ impl<T: ?Sized, Space> SmallBox<T, Space> {

// Just deallocates the heap memory without dropping the boxed value
if this.is_heap() && mem::size_of::<T>() != 0 {
// Safety: MIN_ALIGNMENT is 2, aligning to 2 should not create an invalid layout
let layout = unsafe {
Layout::new::<T>()
.align_to(MIN_ALIGNMENT)
.unwrap_or_else(|_| unreachable_unchecked())
};
let layout = Layout::new::<T>();
unsafe {
alloc::dealloc(this.ptr.as_ptr().cast::<u8>(), layout);
alloc::dealloc(this.ptr.cast_mut().cast::<u8>(), layout);
}
}

Expand Down Expand Up @@ -328,14 +300,12 @@ impl<T: ?Sized, Space> SmallBox<T, Space> {
/// assert_eq!(*small_box, [1, 2, 3, 4]);
/// ```
pub fn from_box(boxed: ::alloc::boxed::Box<T>) -> Self {
unsafe {
let ptr = NonNull::new_unchecked(Box::into_raw(boxed));
let space = MaybeUninit::<UnsafeCell<Space>>::uninit();
SmallBox {
space,
ptr,
_phantom: PhantomData,
}
let ptr = Box::into_raw(boxed);
let space = MaybeUninit::<UnsafeCell<Space>>::uninit();
SmallBox {
space,
ptr,
_phantom: PhantomData,
}
}

Expand Down Expand Up @@ -461,13 +431,11 @@ impl<T: ?Sized, Space> ops::DerefMut for SmallBox<T, Space> {
impl<T: ?Sized, Space> ops::Drop for SmallBox<T, Space> {
fn drop(&mut self) {
unsafe {
let layout = Layout::for_value::<T>(&*self)
.align_to(MIN_ALIGNMENT)
.unwrap_or_else(|_| unreachable_unchecked());
let layout = Layout::for_value::<T>(&*self);

ptr::drop_in_place::<T>(&mut **self);
if self.is_heap() && layout.size() != 0 {
alloc::dealloc(self.ptr.as_ptr().cast::<u8>(), layout);
alloc::dealloc(self.ptr.cast_mut().cast::<u8>(), layout);
}
}
}
Expand Down Expand Up @@ -566,7 +534,6 @@ unsafe impl<T: ?Sized + Sync, Space> Sync for SmallBox<T, Space> {}
#[cfg(test)]
mod tests {
use core::any::Any;
use core::mem;
use core::ptr::addr_of;

use ::alloc::boxed::Box;
Expand Down Expand Up @@ -830,18 +797,10 @@ mod tests {

let zst: SmallBox<dyn Foo, S1> = smallbox!(OveralignedZst);
#[allow(clippy::as_conversions)]
let zst_addr = addr_of!(*zst) as *const () as usize;
let zst_addr = addr_of!(*zst) as *const u8 as usize;
assert_eq!(zst_addr % 512, 0);
}

#[test]
fn test_null_ptr_optimization() {
assert_eq!(
mem::size_of::<SmallBox<i32, S1>>(),
mem::size_of::<Option<SmallBox<i32, S1>>>()
);
}

#[test]
fn test_box_roundtrip() {
// Box -> SmallBox -> Box
Expand Down