Skip to content

Commit ce4c17f

Browse files
committed
Simplify internals of {Rc,Arc}::default
This commit simplifies the internal implementation of `Default` for these two pointer types to have the same performance characteristics as before (a side effect of changes in 131460) while avoid use of internal private APIs of Rc/Arc. To preserve the same codegen as before some non-generic functions needed to be tagged as `#[inline]` as well, but otherwise the same IR is produced before/after this change. The motivation of this commit is I was studying up on the state of initialization of `Arc` and `Rc` and figured it'd be nicer to reduce the use of internal APIs and instead use public stable APIs where possible, even in the implementation itself.
1 parent d7daac0 commit ce4c17f

File tree

3 files changed

+42
-24
lines changed

3 files changed

+42
-24
lines changed

library/alloc/src/rc.rs

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@ struct RcInner<T: ?Sized> {
289289
}
290290

291291
/// Calculate layout for `RcInner<T>` using the inner value's layout
292+
#[inline]
292293
fn rc_inner_layout_for_value_layout(layout: Layout) -> Layout {
293294
// Calculate layout using the given value layout.
294295
// Previously, layout was calculated on the expression
@@ -2518,15 +2519,25 @@ impl<T: Default> Default for Rc<T> {
25182519
/// ```
25192520
#[inline]
25202521
fn default() -> Self {
2522+
// First create an uninitialized allocation before creating an instance
2523+
// of `T`. This avoids having `T` on the stack and avoids the need to
2524+
// codegen a call to the destructor for `T` leading to generally better
2525+
// codegen. See #131460 for some more details.
2526+
let mut rc = Rc::new_uninit();
2527+
2528+
// SAFETY: this is a freshly allocated `Rc` so it's guaranteed there are
2529+
// no other strong or weak pointers other than `rc` itself.
25212530
unsafe {
2522-
Self::from_inner(
2523-
Box::leak(Box::write(
2524-
Box::new_uninit(),
2525-
RcInner { strong: Cell::new(1), weak: Cell::new(1), value: T::default() },
2526-
))
2527-
.into(),
2528-
)
2531+
let raw = Rc::get_mut_unchecked(&mut rc);
2532+
2533+
// Note that `ptr::write` here is used specifically instead of
2534+
// `MaybeUninit::write` to avoid creating an extra stack copy of `T`
2535+
// in debug mode. See #136043 for more context.
2536+
ptr::write(raw.as_mut_ptr(), T::default());
25292537
}
2538+
2539+
// SAFETY: this allocation was just initialized above.
2540+
unsafe { rc.assume_init() }
25302541
}
25312542
}
25322543

library/alloc/src/sync.rs

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,7 @@ struct ArcInner<T: ?Sized> {
392392
}
393393

394394
/// Calculate layout for `ArcInner<T>` using the inner value's layout
395+
#[inline]
395396
fn arcinner_layout_for_value_layout(layout: Layout) -> Layout {
396397
// Calculate layout using the given value layout.
397398
// Previously, layout was calculated on the expression
@@ -3724,19 +3725,25 @@ impl<T: Default> Default for Arc<T> {
37243725
/// assert_eq!(*x, 0);
37253726
/// ```
37263727
fn default() -> Arc<T> {
3728+
// First create an uninitialized allocation before creating an instance
3729+
// of `T`. This avoids having `T` on the stack and avoids the need to
3730+
// codegen a call to the destructor for `T` leading to generally better
3731+
// codegen. See #131460 for some more details.
3732+
let mut arc = Arc::new_uninit();
3733+
3734+
// SAFETY: this is a freshly allocated `Arc` so it's guaranteed there
3735+
// are no other strong or weak pointers other than `arc` itself.
37273736
unsafe {
3728-
Self::from_inner(
3729-
Box::leak(Box::write(
3730-
Box::new_uninit(),
3731-
ArcInner {
3732-
strong: atomic::AtomicUsize::new(1),
3733-
weak: atomic::AtomicUsize::new(1),
3734-
data: T::default(),
3735-
},
3736-
))
3737-
.into(),
3738-
)
3737+
let raw = Arc::get_mut_unchecked(&mut arc);
3738+
3739+
// Note that `ptr::write` here is used specifically instead of
3740+
// `MaybeUninit::write` to avoid creating an extra stack copy of `T`
3741+
// in debug mode. See #136043 for more context.
3742+
ptr::write(raw.as_mut_ptr(), T::default());
37393743
}
3744+
3745+
// SAFETY: this allocation was just initialized above.
3746+
unsafe { arc.assume_init() }
37403747
}
37413748
}
37423749

tests/codegen-llvm/issues/issue-111603.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,19 @@ use std::sync::Arc;
1010
pub fn new_from_array(x: u64) -> Arc<[u64]> {
1111
// Ensure that we only generate one alloca for the array.
1212

13-
// CHECK: alloca
13+
// CHECK: %[[A:.+]] = alloca
1414
// CHECK-SAME: [8000 x i8]
15-
// CHECK-NOT: alloca
15+
// CHECK-NOT: %[[B:.+]] = alloca
1616
let array = [x; 1000];
1717
Arc::new(array)
1818
}
1919

2020
// CHECK-LABEL: @new_uninit
2121
#[no_mangle]
2222
pub fn new_uninit(x: u64) -> Arc<[u64; 1000]> {
23-
// CHECK: call alloc::sync::arcinner_layout_for_value_layout
24-
// CHECK-NOT: call alloc::sync::arcinner_layout_for_value_layout
23+
// CHECK: %[[A:.+]] = alloca
24+
// CHECK-SAME: [8000 x i8]
25+
// CHECK-NOT: %[[B:.+]] = alloca
2526
let mut arc = Arc::new_uninit();
2627
unsafe { Arc::get_mut_unchecked(&mut arc) }.write([x; 1000]);
2728
unsafe { arc.assume_init() }
@@ -30,8 +31,7 @@ pub fn new_uninit(x: u64) -> Arc<[u64; 1000]> {
3031
// CHECK-LABEL: @new_uninit_slice
3132
#[no_mangle]
3233
pub fn new_uninit_slice(x: u64) -> Arc<[u64]> {
33-
// CHECK: call alloc::sync::arcinner_layout_for_value_layout
34-
// CHECK-NOT: call alloc::sync::arcinner_layout_for_value_layout
34+
// CHECK-NOT: %[[B:.+]] = alloca
3535
let mut arc = Arc::new_uninit_slice(1000);
3636
for elem in unsafe { Arc::get_mut_unchecked(&mut arc) } {
3737
elem.write(x);

0 commit comments

Comments
 (0)