Auto merge of #153108 - alexcrichton:revert, r=joboet

Revert "Simplify internals of `{Rc,Arc}::default`"

This reverts #152591 following a [perf run being done](#152591 (comment)). I'm not really positioned at this time to dive in further and understand the performance regressions, so I'll back away from `Rc`/`Arc` and leave them as-is.

Author: bors

library/alloc/src/rc.rs

@@ -289,7 +289,6 @@ struct RcInner<T: ?Sized> {
 }
 
 /// Calculate layout for `RcInner<T>` using the inner value's layout
-#[inline]
 fn rc_inner_layout_for_value_layout(layout: Layout) -> Layout {
     // Calculate layout using the given value layout.
     // Previously, layout was calculated on the expression
@@ -2519,25 +2518,15 @@ impl<T: Default> Default for Rc<T> {
     /// ```
     #[inline]
     fn default() -> Self {
-        // First create an uninitialized allocation before creating an instance
-        // of `T`. This avoids having `T` on the stack and avoids the need to
-        // codegen a call to the destructor for `T` leading to generally better
-        // codegen. See #131460 for some more details.
-        let mut rc = Rc::new_uninit();
-
-        // SAFETY: this is a freshly allocated `Rc` so it's guaranteed there are
-        // no other strong or weak pointers other than `rc` itself.
         unsafe {
-            let raw = Rc::get_mut_unchecked(&mut rc);
-
-            // Note that `ptr::write` here is used specifically instead of
-            // `MaybeUninit::write` to avoid creating an extra stack copy of `T`
-            // in debug mode. See #136043 for more context.
-            ptr::write(raw.as_mut_ptr(), T::default());
+            Self::from_inner(
+                Box::leak(Box::write(
+                    Box::new_uninit(),
+                    RcInner { strong: Cell::new(1), weak: Cell::new(1), value: T::default() },
+                ))
+                .into(),
+            )
         }
-
-        // SAFETY: this allocation was just initialized above.
-        unsafe { rc.assume_init() }
     }
 }
 

library/alloc/src/sync.rs

@@ -392,7 +392,6 @@ struct ArcInner<T: ?Sized> {
 }
 
 /// Calculate layout for `ArcInner<T>` using the inner value's layout
-#[inline]
 fn arcinner_layout_for_value_layout(layout: Layout) -> Layout {
     // Calculate layout using the given value layout.
     // Previously, layout was calculated on the expression
@@ -3725,25 +3724,19 @@ impl<T: Default> Default for Arc<T> {
     /// assert_eq!(*x, 0);
     /// ```
     fn default() -> Arc<T> {
-        // First create an uninitialized allocation before creating an instance
-        // of `T`. This avoids having `T` on the stack and avoids the need to
-        // codegen a call to the destructor for `T` leading to generally better
-        // codegen. See #131460 for some more details.
-        let mut arc = Arc::new_uninit();
-
-        // SAFETY: this is a freshly allocated `Arc` so it's guaranteed there
-        // are no other strong or weak pointers other than `arc` itself.
         unsafe {
-            let raw = Arc::get_mut_unchecked(&mut arc);
-
-            // Note that `ptr::write` here is used specifically instead of
-            // `MaybeUninit::write` to avoid creating an extra stack copy of `T`
-            // in debug mode. See #136043 for more context.
-            ptr::write(raw.as_mut_ptr(), T::default());
+            Self::from_inner(
+                Box::leak(Box::write(
+                    Box::new_uninit(),
+                    ArcInner {
+                        strong: atomic::AtomicUsize::new(1),
+                        weak: atomic::AtomicUsize::new(1),
+                        data: T::default(),
+                    },
+                ))
+                .into(),
+            )
         }
-
-        // SAFETY: this allocation was just initialized above.
-        unsafe { arc.assume_init() }
     }
 }
 

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

@@ -10,19 +10,18 @@ use std::sync::Arc;
 pub fn new_from_array(x: u64) -> Arc<[u64]> {
     // Ensure that we only generate one alloca for the array.
 
-    // CHECK: %[[A:.+]] = alloca
+    // CHECK: alloca
     // CHECK-SAME: [8000 x i8]
-    // CHECK-NOT: %[[B:.+]] = alloca
+    // CHECK-NOT: alloca
     let array = [x; 1000];
     Arc::new(array)
 }
 
 // CHECK-LABEL: @new_uninit
 #[no_mangle]
 pub fn new_uninit(x: u64) -> Arc<[u64; 1000]> {
-    // CHECK: %[[A:.+]] = alloca
-    // CHECK-SAME: [8000 x i8]
-    // CHECK-NOT: %[[B:.+]] = alloca
+    // CHECK: call alloc::sync::arcinner_layout_for_value_layout
+    // CHECK-NOT: call alloc::sync::arcinner_layout_for_value_layout
     let mut arc = Arc::new_uninit();
     unsafe { Arc::get_mut_unchecked(&mut arc) }.write([x; 1000]);
     unsafe { arc.assume_init() }
@@ -31,7 +30,8 @@ pub fn new_uninit(x: u64) -> Arc<[u64; 1000]> {
 // CHECK-LABEL: @new_uninit_slice
 #[no_mangle]
 pub fn new_uninit_slice(x: u64) -> Arc<[u64]> {
-    // CHECK-NOT: %[[B:.+]] = alloca
+    // CHECK: call alloc::sync::arcinner_layout_for_value_layout
+    // CHECK-NOT: call alloc::sync::arcinner_layout_for_value_layout
     let mut arc = Arc::new_uninit_slice(1000);
     for elem in unsafe { Arc::get_mut_unchecked(&mut arc) } {
         elem.write(x);