Skip to content

Commit ba5ff3d

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 71dc761 commit ba5ff3d

3 files changed

Lines changed: 32 additions & 24 deletions

File tree

library/alloc/src/rc.rs

Lines changed: 13 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,20 @@ 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+
Rc::get_mut_unchecked(&mut rc).write(T::default());
25292532
}
2533+
2534+
// SAFETY: this allocation was just initialized above.
2535+
unsafe { rc.assume_init() }
25302536
}
25312537
}
25322538

library/alloc/src/sync.rs

Lines changed: 13 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,20 @@ 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+
Arc::get_mut_unchecked(&mut arc).write(T::default());
37393738
}
3739+
3740+
// SAFETY: this allocation was just initialized above.
3741+
unsafe { arc.assume_init() }
37403742
}
37413743
}
37423744

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)