Skip to content

Commit

Permalink
Auto merge of rust-lang#123550 - GnomedDev:remove-initial-arc, r=<try>
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Apr 30, 2024
2 parents 47314eb + 519fe58 commit 9e56cf1
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 42 deletions.
1 change: 1 addition & 0 deletions library/std/src/lib.rs
Expand Up @@ -356,6 +356,7 @@
#![feature(str_internals)]
#![feature(strict_provenance)]
#![feature(strict_provenance_atomic_ptr)]
#![feature(sync_unsafe_cell)]
#![feature(ub_checks)]
// tidy-alphabetical-end
//
Expand Down
130 changes: 90 additions & 40 deletions library/std/src/thread/mod.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -1162,7 +1164,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);
Expand Down Expand Up @@ -1201,7 +1203,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() -> ! {
Expand All @@ -1212,7 +1219,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 {
Expand All @@ -1228,7 +1235,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 {
Expand All @@ -1245,6 +1252,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`.
///
Expand All @@ -1264,23 +1276,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)
},
}
}
}

Expand All @@ -1304,46 +1352,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
Expand All @@ -1352,7 +1410,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.
Expand Down Expand Up @@ -1388,7 +1446,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.
Expand All @@ -1408,7 +1466,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.
Expand Down Expand Up @@ -1451,15 +1509,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()) })
}
}

Expand Down
5 changes: 3 additions & 2 deletions tests/debuginfo/thread.rs
Expand Up @@ -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;

Expand Down
1 change: 1 addition & 0 deletions tests/rustdoc/demo-allocator-54478.rs
Expand Up @@ -40,6 +40,7 @@
//! }
//!
//! fn main() {
//! drop(String::from("An allocation"));
//! assert!(unsafe { HIT });
//! }
//! ```
40 changes: 40 additions & 0 deletions 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"));
}
16 changes: 16 additions & 0 deletions 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() {}

0 comments on commit 9e56cf1

Please sign in to comment.