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

Revert Vec/Rc storage reuse opt #104571

Merged
merged 1 commit into from
Nov 19, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 6 additions & 53 deletions library/alloc/src/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1441,48 +1441,6 @@ impl<T> Rc<[T]> {
}
}

/// Create an `Rc<[T]>` by reusing the underlying memory
/// of a `Vec<T>`. This will return the vector if the existing allocation
/// is not large enough.
#[cfg(not(no_global_oom_handling))]
fn try_from_vec_in_place(mut v: Vec<T>) -> Result<Rc<[T]>, Vec<T>> {
let layout_elements = Layout::array::<T>(v.len()).unwrap();
let layout_allocation = Layout::array::<T>(v.capacity()).unwrap();
let layout_rcbox = rcbox_layout_for_value_layout(layout_elements);
let mut ptr = NonNull::new(v.as_mut_ptr()).expect("`Vec<T>` stores `NonNull<T>`");
if layout_rcbox.size() > layout_allocation.size()
|| layout_rcbox.align() > layout_allocation.align()
{
// Can't fit - calling `grow` would involve `realloc`
// (which copies the elements), followed by copying again.
return Err(v);
}
if layout_rcbox.size() < layout_allocation.size()
|| layout_rcbox.align() < layout_allocation.align()
{
// We need to shrink the allocation so that it fits
// https://doc.rust-lang.org/nightly/std/alloc/trait.Allocator.html#memory-fitting
// SAFETY:
// - Vec allocates by requesting `Layout::array::<T>(capacity)`, so this capacity matches
// - `layout_rcbox` is smaller
// If this fails, the ownership has not been transferred
if let Ok(p) = unsafe { Global.shrink(ptr.cast(), layout_allocation, layout_rcbox) } {
ptr = p.cast();
} else {
return Err(v);
}
}
// Make sure the vec's memory isn't deallocated now
let v = mem::ManuallyDrop::new(v);
let ptr: *mut RcBox<[T]> = ptr::slice_from_raw_parts_mut(ptr.as_ptr(), v.len()) as _;
unsafe {
ptr::copy(ptr.cast::<T>(), &mut (*ptr).value as *mut [T] as *mut T, v.len());
ptr::write(&mut (*ptr).strong, Cell::new(1));
ptr::write(&mut (*ptr).weak, Cell::new(1));
Ok(Self::from_ptr(ptr))
}
}

/// Constructs an `Rc<[T]>` from an iterator known to be of a certain size.
///
/// Behavior is undefined should the size be wrong.
Expand Down Expand Up @@ -2008,17 +1966,12 @@ impl<T> From<Vec<T>> for Rc<[T]> {
/// assert_eq!(vec![1, 2, 3], *shared);
/// ```
#[inline]
fn from(v: Vec<T>) -> Rc<[T]> {
match Rc::try_from_vec_in_place(v) {
Ok(rc) => rc,
Err(mut v) => {
unsafe {
let rc = Rc::copy_from_slice(&v);
// Allow the Vec to free its memory, but not destroy its contents
v.set_len(0);
rc
}
}
fn from(mut v: Vec<T>) -> Rc<[T]> {
unsafe {
let rc = Rc::copy_from_slice(&v);
// Allow the Vec to free its memory, but not destroy its contents
v.set_len(0);
rc
}
}
}
Expand Down
60 changes: 6 additions & 54 deletions library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1261,49 +1261,6 @@ impl<T> Arc<[T]> {
}
}

/// Create an `Arc<[T]>` by reusing the underlying memory
/// of a `Vec<T>`. This will return the vector if the existing allocation
/// is not large enough.
#[cfg(not(no_global_oom_handling))]
fn try_from_vec_in_place(mut v: Vec<T>) -> Result<Arc<[T]>, Vec<T>> {
let layout_elements = Layout::array::<T>(v.len()).unwrap();
let layout_allocation = Layout::array::<T>(v.capacity()).unwrap();
let layout_arcinner = arcinner_layout_for_value_layout(layout_elements);
let mut ptr = NonNull::new(v.as_mut_ptr()).expect("`Vec<T>` stores `NonNull<T>`");
if layout_arcinner.size() > layout_allocation.size()
|| layout_arcinner.align() > layout_allocation.align()
{
// Can't fit - calling `grow` would involve `realloc`
// (which copies the elements), followed by copying again.
return Err(v);
}
if layout_arcinner.size() < layout_allocation.size()
|| layout_arcinner.align() < layout_allocation.align()
{
// We need to shrink the allocation so that it fits
// https://doc.rust-lang.org/nightly/std/alloc/trait.Allocator.html#memory-fitting
// SAFETY:
// - Vec allocates by requesting `Layout::array::<T>(capacity)`, so this capacity matches
// - `layout_arcinner` is smaller
// If this fails, the ownership has not been transferred
if let Ok(p) = unsafe { Global.shrink(ptr.cast(), layout_allocation, layout_arcinner) }
{
ptr = p.cast();
} else {
return Err(v);
}
}
// Make sure the vec's memory isn't deallocated now
let v = mem::ManuallyDrop::new(v);
let ptr: *mut ArcInner<[T]> = ptr::slice_from_raw_parts_mut(ptr.as_ptr(), v.len()) as _;
unsafe {
ptr::copy(ptr.cast::<T>(), &mut (*ptr).data as *mut [T] as *mut T, v.len());
ptr::write(&mut (*ptr).strong, atomic::AtomicUsize::new(1));
ptr::write(&mut (*ptr).weak, atomic::AtomicUsize::new(1));
Ok(Self::from_ptr(ptr))
}
}

/// Constructs an `Arc<[T]>` from an iterator known to be of a certain size.
///
/// Behavior is undefined should the size be wrong.
Expand Down Expand Up @@ -2615,17 +2572,12 @@ impl<T> From<Vec<T>> for Arc<[T]> {
/// assert_eq!(&[1, 2, 3], &shared[..]);
/// ```
#[inline]
fn from(v: Vec<T>) -> Arc<[T]> {
match Arc::try_from_vec_in_place(v) {
Ok(rc) => rc,
Err(mut v) => {
unsafe {
let rc = Arc::copy_from_slice(&v);
// Allow the Vec to free its memory, but not destroy its contents
v.set_len(0);
rc
}
}
fn from(mut v: Vec<T>) -> Arc<[T]> {
unsafe {
let rc = Arc::copy_from_slice(&v);
// Allow the Vec to free its memory, but not destroy its contents
v.set_len(0);
rc
}
}
}
Expand Down
15 changes: 0 additions & 15 deletions library/alloc/tests/arc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,18 +210,3 @@ fn weak_may_dangle() {
// `val` dropped here while still borrowed
// borrow might be used here, when `val` is dropped and runs the `Drop` code for type `std::sync::Weak`
}

#[test]
fn arc_from_vec_opt() {
let mut v = Vec::with_capacity(64);
v.push(0usize);
let addr = v.as_ptr().cast::<u8>();
let arc: Arc<[_]> = v.into();
unsafe {
assert_eq!(
arc.as_ptr().cast::<u8>().offset_from(addr),
(std::mem::size_of::<usize>() * 2) as isize,
"Vector allocation not reused"
);
}
}
15 changes: 0 additions & 15 deletions library/alloc/tests/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,18 +206,3 @@ fn weak_may_dangle() {
// `val` dropped here while still borrowed
// borrow might be used here, when `val` is dropped and runs the `Drop` code for type `std::rc::Weak`
}

#[test]
fn rc_from_vec_opt() {
let mut v = Vec::with_capacity(64);
v.push(0usize);
let addr = v.as_ptr().cast::<u8>();
let rc: Rc<[_]> = v.into();
unsafe {
assert_eq!(
rc.as_ptr().cast::<u8>().offset_from(addr),
(std::mem::size_of::<usize>() * 2) as isize,
"Vector allocation not reused"
);
}
}