Skip to content

Commit 62c627c

Browse files
committed
Auto merge of #104571 - clubby789:remove-vec-rc-opt, r=the8472
Revert Vec/Rc storage reuse opt Remove the optimization for using storage added by #104205. The perf wins were pretty small, and it relies on non-guarenteed behaviour. On platforms that don't implement shrinking in place, the performance will be significantly worse. While it could be gated to platforms that do this (such as GNU), I don't think it's worth the overhead of maintaining it for very small gains. (#104565, #104563) cc `@RalfJung` `@matthiaskrgr` Fixes #104565 Fixes #104563
2 parents 5e6de23 + 27019f1 commit 62c627c

File tree

4 files changed

+12
-137
lines changed

4 files changed

+12
-137
lines changed

library/alloc/src/rc.rs

+6-53
Original file line numberDiff line numberDiff line change
@@ -1441,48 +1441,6 @@ impl<T> Rc<[T]> {
14411441
}
14421442
}
14431443

1444-
/// Create an `Rc<[T]>` by reusing the underlying memory
1445-
/// of a `Vec<T>`. This will return the vector if the existing allocation
1446-
/// is not large enough.
1447-
#[cfg(not(no_global_oom_handling))]
1448-
fn try_from_vec_in_place(mut v: Vec<T>) -> Result<Rc<[T]>, Vec<T>> {
1449-
let layout_elements = Layout::array::<T>(v.len()).unwrap();
1450-
let layout_allocation = Layout::array::<T>(v.capacity()).unwrap();
1451-
let layout_rcbox = rcbox_layout_for_value_layout(layout_elements);
1452-
let mut ptr = NonNull::new(v.as_mut_ptr()).expect("`Vec<T>` stores `NonNull<T>`");
1453-
if layout_rcbox.size() > layout_allocation.size()
1454-
|| layout_rcbox.align() > layout_allocation.align()
1455-
{
1456-
// Can't fit - calling `grow` would involve `realloc`
1457-
// (which copies the elements), followed by copying again.
1458-
return Err(v);
1459-
}
1460-
if layout_rcbox.size() < layout_allocation.size()
1461-
|| layout_rcbox.align() < layout_allocation.align()
1462-
{
1463-
// We need to shrink the allocation so that it fits
1464-
// https://doc.rust-lang.org/nightly/std/alloc/trait.Allocator.html#memory-fitting
1465-
// SAFETY:
1466-
// - Vec allocates by requesting `Layout::array::<T>(capacity)`, so this capacity matches
1467-
// - `layout_rcbox` is smaller
1468-
// If this fails, the ownership has not been transferred
1469-
if let Ok(p) = unsafe { Global.shrink(ptr.cast(), layout_allocation, layout_rcbox) } {
1470-
ptr = p.cast();
1471-
} else {
1472-
return Err(v);
1473-
}
1474-
}
1475-
// Make sure the vec's memory isn't deallocated now
1476-
let v = mem::ManuallyDrop::new(v);
1477-
let ptr: *mut RcBox<[T]> = ptr::slice_from_raw_parts_mut(ptr.as_ptr(), v.len()) as _;
1478-
unsafe {
1479-
ptr::copy(ptr.cast::<T>(), &mut (*ptr).value as *mut [T] as *mut T, v.len());
1480-
ptr::write(&mut (*ptr).strong, Cell::new(1));
1481-
ptr::write(&mut (*ptr).weak, Cell::new(1));
1482-
Ok(Self::from_ptr(ptr))
1483-
}
1484-
}
1485-
14861444
/// Constructs an `Rc<[T]>` from an iterator known to be of a certain size.
14871445
///
14881446
/// Behavior is undefined should the size be wrong.
@@ -2008,17 +1966,12 @@ impl<T> From<Vec<T>> for Rc<[T]> {
20081966
/// assert_eq!(vec![1, 2, 3], *shared);
20091967
/// ```
20101968
#[inline]
2011-
fn from(v: Vec<T>) -> Rc<[T]> {
2012-
match Rc::try_from_vec_in_place(v) {
2013-
Ok(rc) => rc,
2014-
Err(mut v) => {
2015-
unsafe {
2016-
let rc = Rc::copy_from_slice(&v);
2017-
// Allow the Vec to free its memory, but not destroy its contents
2018-
v.set_len(0);
2019-
rc
2020-
}
2021-
}
1969+
fn from(mut v: Vec<T>) -> Rc<[T]> {
1970+
unsafe {
1971+
let rc = Rc::copy_from_slice(&v);
1972+
// Allow the Vec to free its memory, but not destroy its contents
1973+
v.set_len(0);
1974+
rc
20221975
}
20231976
}
20241977
}

library/alloc/src/sync.rs

+6-54
Original file line numberDiff line numberDiff line change
@@ -1261,49 +1261,6 @@ impl<T> Arc<[T]> {
12611261
}
12621262
}
12631263

1264-
/// Create an `Arc<[T]>` by reusing the underlying memory
1265-
/// of a `Vec<T>`. This will return the vector if the existing allocation
1266-
/// is not large enough.
1267-
#[cfg(not(no_global_oom_handling))]
1268-
fn try_from_vec_in_place(mut v: Vec<T>) -> Result<Arc<[T]>, Vec<T>> {
1269-
let layout_elements = Layout::array::<T>(v.len()).unwrap();
1270-
let layout_allocation = Layout::array::<T>(v.capacity()).unwrap();
1271-
let layout_arcinner = arcinner_layout_for_value_layout(layout_elements);
1272-
let mut ptr = NonNull::new(v.as_mut_ptr()).expect("`Vec<T>` stores `NonNull<T>`");
1273-
if layout_arcinner.size() > layout_allocation.size()
1274-
|| layout_arcinner.align() > layout_allocation.align()
1275-
{
1276-
// Can't fit - calling `grow` would involve `realloc`
1277-
// (which copies the elements), followed by copying again.
1278-
return Err(v);
1279-
}
1280-
if layout_arcinner.size() < layout_allocation.size()
1281-
|| layout_arcinner.align() < layout_allocation.align()
1282-
{
1283-
// We need to shrink the allocation so that it fits
1284-
// https://doc.rust-lang.org/nightly/std/alloc/trait.Allocator.html#memory-fitting
1285-
// SAFETY:
1286-
// - Vec allocates by requesting `Layout::array::<T>(capacity)`, so this capacity matches
1287-
// - `layout_arcinner` is smaller
1288-
// If this fails, the ownership has not been transferred
1289-
if let Ok(p) = unsafe { Global.shrink(ptr.cast(), layout_allocation, layout_arcinner) }
1290-
{
1291-
ptr = p.cast();
1292-
} else {
1293-
return Err(v);
1294-
}
1295-
}
1296-
// Make sure the vec's memory isn't deallocated now
1297-
let v = mem::ManuallyDrop::new(v);
1298-
let ptr: *mut ArcInner<[T]> = ptr::slice_from_raw_parts_mut(ptr.as_ptr(), v.len()) as _;
1299-
unsafe {
1300-
ptr::copy(ptr.cast::<T>(), &mut (*ptr).data as *mut [T] as *mut T, v.len());
1301-
ptr::write(&mut (*ptr).strong, atomic::AtomicUsize::new(1));
1302-
ptr::write(&mut (*ptr).weak, atomic::AtomicUsize::new(1));
1303-
Ok(Self::from_ptr(ptr))
1304-
}
1305-
}
1306-
13071264
/// Constructs an `Arc<[T]>` from an iterator known to be of a certain size.
13081265
///
13091266
/// Behavior is undefined should the size be wrong.
@@ -2615,17 +2572,12 @@ impl<T> From<Vec<T>> for Arc<[T]> {
26152572
/// assert_eq!(&[1, 2, 3], &shared[..]);
26162573
/// ```
26172574
#[inline]
2618-
fn from(v: Vec<T>) -> Arc<[T]> {
2619-
match Arc::try_from_vec_in_place(v) {
2620-
Ok(rc) => rc,
2621-
Err(mut v) => {
2622-
unsafe {
2623-
let rc = Arc::copy_from_slice(&v);
2624-
// Allow the Vec to free its memory, but not destroy its contents
2625-
v.set_len(0);
2626-
rc
2627-
}
2628-
}
2575+
fn from(mut v: Vec<T>) -> Arc<[T]> {
2576+
unsafe {
2577+
let rc = Arc::copy_from_slice(&v);
2578+
// Allow the Vec to free its memory, but not destroy its contents
2579+
v.set_len(0);
2580+
rc
26292581
}
26302582
}
26312583
}

library/alloc/tests/arc.rs

-15
Original file line numberDiff line numberDiff line change
@@ -210,18 +210,3 @@ fn weak_may_dangle() {
210210
// `val` dropped here while still borrowed
211211
// borrow might be used here, when `val` is dropped and runs the `Drop` code for type `std::sync::Weak`
212212
}
213-
214-
#[test]
215-
fn arc_from_vec_opt() {
216-
let mut v = Vec::with_capacity(64);
217-
v.push(0usize);
218-
let addr = v.as_ptr().cast::<u8>();
219-
let arc: Arc<[_]> = v.into();
220-
unsafe {
221-
assert_eq!(
222-
arc.as_ptr().cast::<u8>().offset_from(addr),
223-
(std::mem::size_of::<usize>() * 2) as isize,
224-
"Vector allocation not reused"
225-
);
226-
}
227-
}

library/alloc/tests/rc.rs

-15
Original file line numberDiff line numberDiff line change
@@ -206,18 +206,3 @@ fn weak_may_dangle() {
206206
// `val` dropped here while still borrowed
207207
// borrow might be used here, when `val` is dropped and runs the `Drop` code for type `std::rc::Weak`
208208
}
209-
210-
#[test]
211-
fn rc_from_vec_opt() {
212-
let mut v = Vec::with_capacity(64);
213-
v.push(0usize);
214-
let addr = v.as_ptr().cast::<u8>();
215-
let rc: Rc<[_]> = v.into();
216-
unsafe {
217-
assert_eq!(
218-
rc.as_ptr().cast::<u8>().offset_from(addr),
219-
(std::mem::size_of::<usize>() * 2) as isize,
220-
"Vector allocation not reused"
221-
);
222-
}
223-
}

0 commit comments

Comments
 (0)