Skip to content

Add to_boxed_slice() to clone slice into boxed slice #80460

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

Closed
Closed
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
10 changes: 3 additions & 7 deletions library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
@@ -1229,13 +1229,9 @@ impl<T: Copy> From<&[T]> for Box<[T]> {
///
/// println!("{:?}", boxed_slice);
/// ```
#[inline]
fn from(slice: &[T]) -> Box<[T]> {
let len = slice.len();
let buf = RawVec::with_capacity(len);
unsafe {
ptr::copy_nonoverlapping(slice.as_ptr(), buf.ptr(), len);
buf.into_box(slice.len()).assume_init()
}
slice.to_boxed_slice()
}
}

@@ -1578,7 +1574,7 @@ impl<I> FromIterator<I> for Box<[I]> {
impl<T: Clone, A: Allocator + Clone> Clone for Box<[T], A> {
fn clone(&self) -> Self {
let alloc = Box::allocator(self).clone();
self.to_vec_in(alloc).into_boxed_slice()
self.to_boxed_slice_in(alloc)
}

fn clone_from(&mut self, other: &Self) {
112 changes: 77 additions & 35 deletions library/alloc/src/slice.rs
Original file line number Diff line number Diff line change
@@ -143,6 +143,7 @@ pub use hack::to_vec;
// `test_permutations` test
mod hack {
use core::alloc::Allocator;
use core::mem::MaybeUninit;

use crate::boxed::Box;
use crate::vec::Vec;
@@ -159,64 +160,62 @@ mod hack {
}

#[inline]
pub fn to_vec<T: ConvertVec, A: Allocator>(s: &[T], alloc: A) -> Vec<T, A> {
T::to_vec(s, alloc)
pub fn to_vec<T: ConvertBoxed, A: Allocator>(s: &[T], alloc: A) -> Vec<T, A> {
into_vec(to_boxed_slice(s, alloc))
}

pub trait ConvertVec {
fn to_vec<A: Allocator>(s: &[Self], alloc: A) -> Vec<Self, A>
#[inline]
pub fn to_boxed_slice<T: ConvertBoxed, A: Allocator>(s: &[T], alloc: A) -> Box<[T], A> {
T::to_boxed_slice(s, alloc)
}

pub trait ConvertBoxed {
fn to_boxed_slice<A: Allocator>(s: &[Self], alloc: A) -> Box<[Self], A>
where
Self: Sized;
}

impl<T: Clone> ConvertVec for T {
impl<T: Clone> ConvertBoxed for T {
#[inline]
default fn to_vec<A: Allocator>(s: &[Self], alloc: A) -> Vec<Self, A> {
struct DropGuard<'a, T, A: Allocator> {
vec: &'a mut Vec<T, A>,
default fn to_boxed_slice<A: Allocator>(s: &[Self], alloc: A) -> Box<[Self], A> {
struct DropGuard<T> {
dst: *mut T,
num_init: usize,
}
impl<'a, T, A: Allocator> Drop for DropGuard<'a, T, A> {
impl<T> Drop for DropGuard<T> {
#[inline]
fn drop(&mut self) {
// SAFETY:
// items were marked initialized in the loop below
let init_slice = core::ptr::slice_from_raw_parts_mut(self.dst, self.num_init);
// SAFETY: all elements in this slice have been initialized
unsafe {
self.vec.set_len(self.num_init);
core::ptr::drop_in_place(init_slice);
}
}
}
let mut vec = Vec::with_capacity_in(s.len(), alloc);
let mut guard = DropGuard { vec: &mut vec, num_init: 0 };
let slots = guard.vec.spare_capacity_mut();
// .take(slots.len()) is necessary for LLVM to remove bounds checks
// and has better codegen than zip.
for (i, b) in s.iter().enumerate().take(slots.len()) {
guard.num_init = i;
slots[i].write(b.clone());

let mut boxed = Box::new_uninit_slice_in(s.len(), alloc);
let mut guard =
DropGuard { dst: MaybeUninit::slice_as_mut_ptr(&mut boxed), num_init: 0 };
while guard.num_init < s.len() {
boxed[guard.num_init].write(s[guard.num_init].clone());
guard.num_init += 1;
}
core::mem::forget(guard);
// SAFETY:
// the vec was allocated and initialized above to at least this length.
unsafe {
vec.set_len(s.len());
}
vec
// SAFETY: each element is initialized by the loop above
unsafe { boxed.assume_init() }
}
}

impl<T: Copy> ConvertVec for T {
impl<T: Copy> ConvertBoxed for T {
#[inline]
fn to_vec<A: Allocator>(s: &[Self], alloc: A) -> Vec<Self, A> {
let mut v = Vec::with_capacity_in(s.len(), alloc);
// SAFETY:
// allocated above with the capacity of `s`, and initialize to `s.len()` in
// ptr::copy_to_non_overlapping below.
fn to_boxed_slice<A: Allocator>(s: &[Self], alloc: A) -> Box<[Self], A> {
let mut boxed = Box::new_uninit_slice_in(s.len(), alloc);
Copy link
Member

Choose a reason for hiding this comment

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

Lots of effort went into the Vec allocation path to reduce the amount of IR generated, perhaps the Box equivalent simply isn't as fine-tuned and that causes the regressions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're identical except for the additional RawVec::into_box() call, which just moves some pointers and lengths around, so it seems unlikely that this would be the culprit:

impl<T, A: Allocator> Box<[T], A> {
    pub fn new_uninit_slice_in(len: usize, alloc: A) -> Box<[mem::MaybeUninit<T>], A> {
        unsafe { RawVec::with_capacity_in(len, alloc).into_box(len) }
    }
}

impl<T, A: Allocator> Vec<T, A> {
    pub fn with_capacity_in(capacity: usize, alloc: A) -> Self {
        Vec { buf: RawVec::with_capacity_in(capacity, alloc), len: 0 }
    }
}

impl<T, A: Allocator> RawVec<T, A> {
    pub unsafe fn into_box(self, len: usize) -> Box<[MaybeUninit<T>], A> {
        // Sanity-check one half of the safety requirement (we cannot check the other half).
        debug_assert!(
            len <= self.capacity(),
            "`len` must be smaller than or equal to `self.capacity()`"
        );

        let me = ManuallyDrop::new(self);
        unsafe {
            let slice = slice::from_raw_parts_mut(me.ptr() as *mut MaybeUninit<T>, len);
            Box::from_raw_in(slice, ptr::read(&me.alloc))
        }
    }
}

let boxed_ptr = MaybeUninit::slice_as_mut_ptr(&mut boxed);
// SAFETY: `boxed` contains `s.len()` elements and all are initialized
unsafe {
s.as_ptr().copy_to_nonoverlapping(v.as_mut_ptr(), s.len());
v.set_len(s.len());
s.as_ptr().copy_to_nonoverlapping(boxed_ptr, s.len());
boxed.assume_init()
}
v
}
}
}
@@ -477,6 +476,49 @@ impl<T> [T] {
hack::to_vec(self, alloc)
}

/// Clones `self` into a new boxed slice.
///
/// # Examples
///
/// ```
/// #![feature(slice_to_boxed)]
///
/// let s = [1, 2, 3];
/// let x: Box<[i32]> = s.to_boxed_slice();
/// assert_eq!(&s, x.as_ref());
/// ```
#[inline]
#[unstable(feature = "slice_to_boxed", issue = "82725")]
pub fn to_boxed_slice(&self) -> Box<Self>
where
T: Clone,
{
self.to_boxed_slice_in(Global)
}

/// Clones `self` into a new boxed slice with an allocator.
///
/// # Examples
///
/// ```
/// #![feature(allocator_api)]
///
/// use std::alloc::System;
///
/// let s = [1, 2, 3];
/// let x: Box<[i32], System> = s.to_boxed_slice_in(System);
/// assert_eq!(&s, x.as_ref());
/// ```
#[inline]
#[unstable(feature = "allocator_api", issue = "32838")]
// #[unstable(feature = "slice_to_boxed", issue = "82725")]
pub fn to_boxed_slice_in<A: Allocator>(&self, alloc: A) -> Box<Self, A>
where
T: Clone,
{
hack::to_boxed_slice(self, alloc)
}

/// Converts `self` into a vector without clones or allocation.
///
/// The resulting vector can be converted back into a box via
3 changes: 1 addition & 2 deletions library/std/src/sys/unix/fs.rs
Original file line number Diff line number Diff line change
@@ -464,8 +464,7 @@ impl Iterator for ReadDir {
let ret = DirEntry {
entry: *entry_ptr,
name: slice::from_raw_parts(name as *const u8, namelen as usize)
.to_owned()
.into_boxed_slice(),
.to_boxed_slice(),
dir: Arc::clone(&self.inner),
};
if ret.name_bytes() != b"." && ret.name_bytes() != b".." {