Skip to content

Commit 8424c24

Browse files
committed
Add Vec storage optimization to Arc and add tests
1 parent 1c813c4 commit 8424c24

File tree

3 files changed

+96
-19
lines changed

3 files changed

+96
-19
lines changed

library/alloc/src/sync.rs

+66-19
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,15 @@ struct ArcInner<T: ?Sized> {
333333
data: T,
334334
}
335335

336+
/// Calculate layout for `ArcInner<T>` using the inner value's layout
337+
fn arcinner_layout_for_value_layout(layout: Layout) -> Layout {
338+
// Calculate layout using the given value layout.
339+
// Previously, layout was calculated on the expression
340+
// `&*(ptr as *const ArcInner<T>)`, but this created a misaligned
341+
// reference (see #54908).
342+
Layout::new::<ArcInner<()>>().extend(layout).unwrap().0.pad_to_align()
343+
}
344+
336345
unsafe impl<T: ?Sized + Sync + Send> Send for ArcInner<T> {}
337346
unsafe impl<T: ?Sized + Sync + Send> Sync for ArcInner<T> {}
338347

@@ -1154,11 +1163,7 @@ impl<T: ?Sized> Arc<T> {
11541163
allocate: impl FnOnce(Layout) -> Result<NonNull<[u8]>, AllocError>,
11551164
mem_to_arcinner: impl FnOnce(*mut u8) -> *mut ArcInner<T>,
11561165
) -> *mut ArcInner<T> {
1157-
// Calculate layout using the given value layout.
1158-
// Previously, layout was calculated on the expression
1159-
// `&*(ptr as *const ArcInner<T>)`, but this created a misaligned
1160-
// reference (see #54908).
1161-
let layout = Layout::new::<ArcInner<()>>().extend(value_layout).unwrap().0.pad_to_align();
1166+
let layout = arcinner_layout_for_value_layout(value_layout);
11621167
unsafe {
11631168
Arc::try_allocate_for_layout(value_layout, allocate, mem_to_arcinner)
11641169
.unwrap_or_else(|_| handle_alloc_error(layout))
@@ -1176,11 +1181,7 @@ impl<T: ?Sized> Arc<T> {
11761181
allocate: impl FnOnce(Layout) -> Result<NonNull<[u8]>, AllocError>,
11771182
mem_to_arcinner: impl FnOnce(*mut u8) -> *mut ArcInner<T>,
11781183
) -> Result<*mut ArcInner<T>, AllocError> {
1179-
// Calculate layout using the given value layout.
1180-
// Previously, layout was calculated on the expression
1181-
// `&*(ptr as *const ArcInner<T>)`, but this created a misaligned
1182-
// reference (see #54908).
1183-
let layout = Layout::new::<ArcInner<()>>().extend(value_layout).unwrap().0.pad_to_align();
1184+
let layout = arcinner_layout_for_value_layout(value_layout);
11841185

11851186
let ptr = allocate(layout)?;
11861187

@@ -1246,7 +1247,7 @@ impl<T> Arc<[T]> {
12461247
}
12471248
}
12481249

1249-
/// Copy elements from slice into newly allocated Arc<\[T\]>
1250+
/// Copy elements from slice into newly allocated `Arc<[T]>`
12501251
///
12511252
/// Unsafe because the caller must either take ownership or bind `T: Copy`.
12521253
#[cfg(not(no_global_oom_handling))]
@@ -1260,6 +1261,49 @@ impl<T> Arc<[T]> {
12601261
}
12611262
}
12621263

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+
12631307
/// Constructs an `Arc<[T]>` from an iterator known to be of a certain size.
12641308
///
12651309
/// Behavior is undefined should the size be wrong.
@@ -2571,14 +2615,17 @@ impl<T> From<Vec<T>> for Arc<[T]> {
25712615
/// assert_eq!(&[1, 2, 3], &shared[..]);
25722616
/// ```
25732617
#[inline]
2574-
fn from(mut v: Vec<T>) -> Arc<[T]> {
2575-
unsafe {
2576-
let arc = Arc::copy_from_slice(&v);
2577-
2578-
// Allow the Vec to free its memory, but not destroy its contents
2579-
v.set_len(0);
2580-
2581-
arc
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+
}
25822629
}
25832630
}
25842631
}

library/alloc/tests/arc.rs

+15
Original file line numberDiff line numberDiff line change
@@ -210,3 +210,18 @@ 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,3 +206,18 @@ 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)