Auto merge of rust-lang#123550 - GnomedDev:remove-initial-arc, r=Nilstrieb

Remove last rt::init allocation for thread info

Removes the last allocation pre-main by just not storing anything in std::thread::Thread for the main thread.
- The thread name can just be a hard coded literal, as was done in rust-lang#123433.
- The ThreadId is always the `1` value, so `ThreadId::new` now starts at `2` and can fabricate the `1` value when needed.
- Storing Parker in a static that is initialized once at startup. This uses SyncUnsafeCell and MaybeUninit as this is quite performance critical and we don't need synchronization or to store a tag value and possibly leave in a panic.

This also adds a UI test to make sure that allocations do not occur before main ever again.

try-job: dist-x86_64-linux

Author: bors

library/std/src/lib.rs

@@ -352,6 +352,7 @@
 #![feature(str_internals)]
 #![feature(strict_provenance)]
 #![feature(strict_provenance_atomic_ptr)]
+#![feature(sync_unsafe_cell)]
 #![feature(ub_checks)]
 // tidy-alphabetical-end
 //

library/std/src/thread/mod.rs

@@ -159,12 +159,14 @@
 mod tests;
 
 use crate::any::Any;
+use crate::cell::SyncUnsafeCell;
 use crate::cell::{OnceCell, UnsafeCell};
 use crate::env;
 use crate::ffi::{CStr, CString};
 use crate::fmt;
 use crate::io;
 use crate::marker::PhantomData;
+use crate::mem::MaybeUninit;
 use crate::mem::{self, forget};
 use crate::num::NonZero;
 use crate::panic;
@@ -530,7 +532,7 @@ impl Builder {
 
         let f = MaybeDangling::new(f);
         let main = move || {
-            if let Some(name) = their_thread.cname() {
+            if let Some(name) = their_thread.0.name() {
                 imp::Thread::set_name(name);
             }
 
@@ -1168,7 +1170,7 @@ pub fn park_timeout(dur: Duration) {
     let guard = PanicGuard;
     // SAFETY: park_timeout is called on the parker owned by this thread.
     unsafe {
-        current().inner.as_ref().parker().park_timeout(dur);
+        current().0.parker().park_timeout(dur);
     }
     // No panic occurred, do not abort.
     forget(guard);
@@ -1207,7 +1209,12 @@ pub fn park_timeout(dur: Duration) {
 pub struct ThreadId(NonZero<u64>);
 
 impl ThreadId {
-    // Generate a new unique thread ID.
+    /// Generate a new unique thread ID.
+    ///
+    /// The current implementation starts at 2 and increments from there.
+    ///
+    /// This is as `1` is the value for the main thread, so std::thread::Thread does not
+    /// have to store this value when creating the main thread's information.
     fn new() -> ThreadId {
         #[cold]
         fn exhausted() -> ! {
@@ -1218,7 +1225,7 @@ impl ThreadId {
             if #[cfg(target_has_atomic = "64")] {
                 use crate::sync::atomic::AtomicU64;
 
-                static COUNTER: AtomicU64 = AtomicU64::new(0);
+                static COUNTER: AtomicU64 = AtomicU64::new(1);
 
                 let mut last = COUNTER.load(Ordering::Relaxed);
                 loop {
@@ -1234,7 +1241,7 @@ impl ThreadId {
             } else {
                 use crate::sync::{Mutex, PoisonError};
 
-                static COUNTER: Mutex<u64> = Mutex::new(0);
+                static COUNTER: Mutex<u64> = Mutex::new(1);
 
                 let mut counter = COUNTER.lock().unwrap_or_else(PoisonError::into_inner);
                 let Some(id) = counter.checked_add(1) else {
@@ -1251,6 +1258,11 @@ impl ThreadId {
         }
     }
 
+    /// Creates a ThreadId with the ID of the main thread.
+    fn new_main() -> Self {
+        Self(NonZero::<u64>::MIN)
+    }
+
     /// This returns a numeric identifier for the thread identified by this
     /// `ThreadId`.
     ///
@@ -1270,23 +1282,59 @@ impl ThreadId {
 // Thread
 ////////////////////////////////////////////////////////////////////////////////
 
-/// The internal representation of a `Thread`'s name.
-enum ThreadName {
-    Main,
-    Other(CString),
-    Unnamed,
-}
+/// The parker for the main thread. This avoids having to allocate an Arc in `fn main() {}`.
+static MAIN_PARKER: SyncUnsafeCell<MaybeUninit<Parker>> =
+    SyncUnsafeCell::new(MaybeUninit::uninit());
 
-/// The internal representation of a `Thread` handle
-struct Inner {
-    name: ThreadName, // Guaranteed to be UTF-8
+/// The internal representation of a `Thread` that is not the main thread.
+struct OtherInner {
+    name: Option<CString>, // Guaranteed to be UTF-8
     id: ThreadId,
     parker: Parker,
 }
 
+/// The internal representation of a `Thread` handle.
+#[derive(Clone)]
+enum Inner {
+    /// Represents the main thread. May only be constructed by Thread::new_main.
+    Main,
+    /// Represents any other thread.
+    Other(Pin<Arc<OtherInner>>),
+}
+
 impl Inner {
-    fn parker(self: Pin<&Self>) -> Pin<&Parker> {
-        unsafe { Pin::map_unchecked(self, |inner| &inner.parker) }
+    fn id(&self) -> ThreadId {
+        match self {
+            Self::Main => ThreadId::new_main(),
+            Self::Other(other) => other.id,
+        }
+    }
+
+    fn name(&self) -> Option<&CStr> {
+        match self {
+            Self::Main => Some(c"main"),
+            Self::Other(other) => other.name.as_deref(),
+        }
+    }
+
+    fn parker(&self) -> Pin<&Parker> {
+        match self {
+            Self::Main => {
+                // Safety: MAIN_PARKER is only ever read in this function, which requires access
+                // to an existing `&Inner` value, which can only be accessed via the main thread
+                // giving away such an instance from `current()`, implying that initialization,
+                // the only write to `MAIN_PARKER`, has been completed.
+                let static_ref: &'static MaybeUninit<Parker> = unsafe { &*MAIN_PARKER.get() };
+
+                // Safety: MAIN_PARKER is initialised when Inner::Main is initialised.
+                let parker_ref = unsafe { static_ref.assume_init_ref() };
+
+                Pin::static_ref(parker_ref)
+            }
+            Self::Other(inner) => unsafe {
+                Pin::map_unchecked(inner.as_ref(), |inner| &inner.parker)
+            },
+        }
     }
 }
 
@@ -1310,46 +1358,56 @@ impl Inner {
 /// docs of [`Builder`] and [`spawn`] for more details.
 ///
 /// [`thread::current`]: current
-pub struct Thread {
-    inner: Pin<Arc<Inner>>,
-}
+pub struct Thread(Inner);
 
 impl Thread {
     /// Used only internally to construct a thread object without spawning.
     ///
     /// # Safety
     /// `name` must be valid UTF-8.
     pub(crate) unsafe fn new(name: CString) -> Thread {
-        unsafe { Self::new_inner(ThreadName::Other(name)) }
+        unsafe { Self::new_inner(Some(name)) }
     }
 
     pub(crate) fn new_unnamed() -> Thread {
-        unsafe { Self::new_inner(ThreadName::Unnamed) }
+        unsafe { Self::new_inner(None) }
     }
 
-    // Used in runtime to construct main thread
-    pub(crate) fn new_main() -> Thread {
-        unsafe { Self::new_inner(ThreadName::Main) }
+    /// Used in runtime to construct main thread
+    ///
+    /// # Safety
+    ///
+    /// This must only ever be called once, and must be called on the main thread.
+    pub(crate) unsafe fn new_main() -> Thread {
+        // Safety: As this is only called once and on the main thread, nothing else is accessing MAIN_PARKER
+        // as the only other read occurs in Inner::parker *after* Inner::Main has been constructed,
+        // and this function is the only one that constructs Inner::Main.
+        //
+        // Pre-main thread spawning cannot hit this either, as the caller promises that this is only called on the main thread.
+        unsafe { Parker::new_in_place(MAIN_PARKER.get().cast()) }
+
+        Self(Inner::Main)
     }
 
     /// # Safety
-    /// If `name` is `ThreadName::Other(_)`, the contained string must be valid UTF-8.
-    unsafe fn new_inner(name: ThreadName) -> Thread {
+    ///
+    /// If `name` is `Some(_)`, the contained string must be valid UTF-8.
+    unsafe fn new_inner(name: Option<CString>) -> Thread {
         // We have to use `unsafe` here to construct the `Parker` in-place,
         // which is required for the UNIX implementation.
         //
         // SAFETY: We pin the Arc immediately after creation, so its address never
         // changes.
         let inner = unsafe {
-            let mut arc = Arc::<Inner>::new_uninit();
+            let mut arc = Arc::<OtherInner>::new_uninit();
             let ptr = Arc::get_mut_unchecked(&mut arc).as_mut_ptr();
             addr_of_mut!((*ptr).name).write(name);
             addr_of_mut!((*ptr).id).write(ThreadId::new());
             Parker::new_in_place(addr_of_mut!((*ptr).parker));
             Pin::new_unchecked(arc.assume_init())
         };
 
-        Thread { inner }
+        Self(Inner::Other(inner))
     }
 
     /// Like the public [`park`], but callable on any handle. This is used to
@@ -1358,7 +1416,7 @@ impl Thread {
     /// # Safety
     /// May only be called from the thread to which this handle belongs.
     pub(crate) unsafe fn park(&self) {
-        unsafe { self.inner.as_ref().parker().park() }
+        unsafe { self.0.parker().park() }
     }
 
     /// Atomically makes the handle's token available if it is not already.
@@ -1394,7 +1452,7 @@ impl Thread {
     #[stable(feature = "rust1", since = "1.0.0")]
     #[inline]
     pub fn unpark(&self) {
-        self.inner.as_ref().parker().unpark();
+        self.0.parker().unpark();
     }
 
     /// Gets the thread's unique identifier.
@@ -1414,7 +1472,7 @@ impl Thread {
     #[stable(feature = "thread_id", since = "1.19.0")]
     #[must_use]
     pub fn id(&self) -> ThreadId {
-        self.inner.id
+        self.0.id()
     }
 
     /// Gets the thread's name.
@@ -1457,15 +1515,7 @@ impl Thread {
     #[stable(feature = "rust1", since = "1.0.0")]
     #[must_use]
     pub fn name(&self) -> Option<&str> {
-        self.cname().map(|s| unsafe { str::from_utf8_unchecked(s.to_bytes()) })
-    }
-
-    fn cname(&self) -> Option<&CStr> {
-        match &self.inner.name {
-            ThreadName::Main => Some(c"main"),
-            ThreadName::Other(other) => Some(&other),
-            ThreadName::Unnamed => None,
-        }
+        self.0.name().map(|s| unsafe { str::from_utf8_unchecked(s.to_bytes()) })
     }
 }
 

tests/debuginfo/thread.rs

@@ -12,9 +12,10 @@
 // cdb-check:join_handle,d    [Type: std::thread::JoinHandle<tuple$<> >]
 // cdb-check:    [...] __0              [Type: std::thread::JoinInner<tuple$<> >]
 //
-// cdb-command:dx t,d
+// cdb-command:dx -r3 t,d
 // cdb-check:t,d              : [...] [Type: std::thread::Thread *]
-// cdb-check:[...] inner [...][Type: core::pin::Pin<alloc::sync::Arc<std::thread::Inner,alloc::alloc::Global> >]
+// cdb-check:    [...] __0              : Other [Type: enum2$<std::thread::Inner>]
+// cdb-check:         [...] __0              [Type: core::pin::Pin<alloc::sync::Arc<std::thread::OtherInner,[...]> >]
 
 use std::thread;
 

tests/rustdoc/demo-allocator-54478.rs

@@ -40,6 +40,7 @@
 //! }
 //!
 //! fn main() {
+//!     drop(String::from("An allocation"));
 //!     assert!(unsafe { HIT });
 //! }
 //! ```

tests/ui/runtime/aborting-alloc.rs

@@ -0,0 +1,40 @@
+//! Helper for 'no-allocation-before-main'.
+//!
+//! This also contains a meta-test to make sure that the AbortingAllocator does indeed abort.
+//!
+//! -Cprefer-dynamic=no is required as otherwise #[global_allocator] does nothing.
+//@ run-fail
+//@ compile-flags: -Cprefer-dynamic=no
+
+use std::{sync::atomic::{AtomicBool, Ordering}, alloc::System};
+
+static ABORT: AtomicBool = AtomicBool::new(true);
+
+pub struct AbortingAllocator(System);
+
+unsafe impl std::alloc::GlobalAlloc for AbortingAllocator {
+    unsafe fn alloc(&self, layout: std::alloc::Layout) -> *mut u8 {
+        if ABORT.swap(false, Ordering::SeqCst) {
+            println!("{}", std::backtrace::Backtrace::force_capture());
+            std::process::abort();
+        }
+
+        self.0.alloc(layout)
+    }
+
+    unsafe fn dealloc(&self, ptr: *mut u8, layout: std::alloc::Layout) {
+        if ABORT.swap(false, Ordering::SeqCst) {
+            println!("{}", std::backtrace::Backtrace::force_capture());
+            std::process::abort();
+        }
+
+        self.0.dealloc(ptr, layout)
+    }
+}
+
+#[global_allocator]
+static ALLOCATOR: AbortingAllocator = AbortingAllocator(System);
+
+fn main() {
+    std::hint::black_box(String::from("An allocation"));
+}

tests/ui/runtime/no-allocation-before-main.rs

@@ -0,0 +1,16 @@
+//! Tests that a program with no body does not allocate.
+//!
+//! The initial runtime should not allocate for performance/binary size reasons.
+//!
+//! -Cprefer-dynamic=no is required as otherwise #[global_allocator] does nothing.
+//! We only test linux-gnu as other targets currently need allocation for thread dtors.
+//@ run-pass
+//@ compile-flags: -Cprefer-dynamic=no -Cdebuginfo=full
+//@ only-linux
+//@ only-gnu
+
+#[allow(dead_code)]
+#[path = "aborting-alloc.rs"]
+mod aux;
+
+fn main() {}