Skip to content

Commit 666bbff

Browse files
committed
Auto merge of rust-lang#123550 - GnomedDev:remove-initial-arc, r=<try>
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 currently does not have a regression test to prevent future changes from re-adding allocations pre-main as I'm [having trouble](GnomedDev@6f7be53) implementing it, but if wanted I can draft this PR until that test is ready.
2 parents 83d0a94 + d5b8b00 commit 666bbff

File tree

3 files changed

+79
-46
lines changed

3 files changed

+79
-46
lines changed

library/std/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,7 @@
353353
#![feature(str_internals)]
354354
#![feature(strict_provenance)]
355355
#![feature(strict_provenance_atomic_ptr)]
356+
#![feature(sync_unsafe_cell)]
356357
// tidy-alphabetical-end
357358
//
358359
// Library features (alloc):

library/std/src/thread/mod.rs

+77-46
Original file line numberDiff line numberDiff line change
@@ -159,11 +159,13 @@
159159
mod tests;
160160

161161
use crate::any::Any;
162+
use crate::cell::SyncUnsafeCell;
162163
use crate::cell::{OnceCell, UnsafeCell};
163164
use crate::ffi::{CStr, CString};
164165
use crate::fmt;
165166
use crate::io;
166167
use crate::marker::PhantomData;
168+
use crate::mem::MaybeUninit;
167169
use crate::mem::{self, forget};
168170
use crate::num::NonZero;
169171
use crate::panic;
@@ -511,7 +513,7 @@ impl Builder {
511513

512514
let f = MaybeDangling::new(f);
513515
let main = move || {
514-
if let Some(name) = their_thread.cname() {
516+
if let Some(name) = their_thread.0.name() {
515517
imp::Thread::set_name(name);
516518
}
517519

@@ -1143,7 +1145,7 @@ pub fn park_timeout(dur: Duration) {
11431145
let guard = PanicGuard;
11441146
// SAFETY: park_timeout is called on the parker owned by this thread.
11451147
unsafe {
1146-
current().inner.as_ref().parker().park_timeout(dur);
1148+
current().0.parker().park_timeout(dur);
11471149
}
11481150
// No panic occurred, do not abort.
11491151
forget(guard);
@@ -1182,7 +1184,12 @@ pub fn park_timeout(dur: Duration) {
11821184
pub struct ThreadId(NonZero<u64>);
11831185

11841186
impl ThreadId {
1185-
// Generate a new unique thread ID.
1187+
/// Generate a new unique thread ID.
1188+
///
1189+
/// The current implementation starts at 2 and increments from there.
1190+
///
1191+
/// This is as `1` is the value for the main thread, so std::thread::Thread does not
1192+
/// have to store this value when creating the main thread's information.
11861193
fn new() -> ThreadId {
11871194
#[cold]
11881195
fn exhausted() -> ! {
@@ -1193,7 +1200,7 @@ impl ThreadId {
11931200
if #[cfg(target_has_atomic = "64")] {
11941201
use crate::sync::atomic::{AtomicU64, Ordering::Relaxed};
11951202

1196-
static COUNTER: AtomicU64 = AtomicU64::new(0);
1203+
static COUNTER: AtomicU64 = AtomicU64::new(1);
11971204

11981205
let mut last = COUNTER.load(Relaxed);
11991206
loop {
@@ -1209,7 +1216,7 @@ impl ThreadId {
12091216
} else {
12101217
use crate::sync::{Mutex, PoisonError};
12111218

1212-
static COUNTER: Mutex<u64> = Mutex::new(0);
1219+
static COUNTER: Mutex<u64> = Mutex::new(1);
12131220

12141221
let mut counter = COUNTER.lock().unwrap_or_else(PoisonError::into_inner);
12151222
let Some(id) = counter.checked_add(1) else {
@@ -1226,6 +1233,11 @@ impl ThreadId {
12261233
}
12271234
}
12281235

1236+
/// Creates a ThreadId with the ID of the main thread.
1237+
fn new_main() -> Self {
1238+
Self(NonZero::<u64>::MIN)
1239+
}
1240+
12291241
/// This returns a numeric identifier for the thread identified by this
12301242
/// `ThreadId`.
12311243
///
@@ -1245,23 +1257,54 @@ impl ThreadId {
12451257
// Thread
12461258
////////////////////////////////////////////////////////////////////////////////
12471259

1248-
/// The internal representation of a `Thread`'s name.
1249-
enum ThreadName {
1250-
Main,
1251-
Other(CString),
1252-
Unnamed,
1253-
}
1260+
/// The parker for the main thread. This avoids having to allocate an Arc in `fn main() {}`.
1261+
static MAIN_PARKER: SyncUnsafeCell<MaybeUninit<Parker>> =
1262+
SyncUnsafeCell::new(MaybeUninit::uninit());
12541263

1255-
/// The internal representation of a `Thread` handle
1256-
struct Inner {
1257-
name: ThreadName, // Guaranteed to be UTF-8
1264+
/// The internal representation of a `Thread` that is not the main thread.
1265+
struct OtherInner {
1266+
name: Option<CString>, // Guaranteed to be UTF-8
12581267
id: ThreadId,
12591268
parker: Parker,
12601269
}
12611270

1271+
/// The internal representation of a `Thread` handle.
1272+
#[derive(Clone)]
1273+
enum Inner {
1274+
Main,
1275+
Other(Pin<Arc<OtherInner>>),
1276+
}
1277+
12621278
impl Inner {
1263-
fn parker(self: Pin<&Self>) -> Pin<&Parker> {
1264-
unsafe { Pin::map_unchecked(self, |inner| &inner.parker) }
1279+
fn id(&self) -> ThreadId {
1280+
match self {
1281+
Self::Main => ThreadId::new_main(),
1282+
Self::Other(other) => other.id,
1283+
}
1284+
}
1285+
1286+
fn name(&self) -> Option<&CStr> {
1287+
match self {
1288+
Self::Main => Some(c"main"),
1289+
Self::Other(other) => other.name.as_deref(),
1290+
}
1291+
}
1292+
1293+
fn parker(&self) -> Pin<&Parker> {
1294+
match self {
1295+
Self::Main => {
1296+
// Safety: MAIN_PARKER only ever has a mutable reference when Inner::Main is initialised.
1297+
let uninit_ref: &'static MaybeUninit<Parker> = unsafe { &*MAIN_PARKER.get() };
1298+
1299+
// Safety: MAIN_PARKER is initialised when Inner::Main is initialised.
1300+
let parker_ref = unsafe { uninit_ref.assume_init_ref() };
1301+
1302+
Pin::static_ref(parker_ref)
1303+
}
1304+
Self::Other(inner) => unsafe {
1305+
Pin::map_unchecked(inner.as_ref(), |inner| &inner.parker)
1306+
},
1307+
}
12651308
}
12661309
}
12671310

@@ -1285,41 +1328,37 @@ impl Inner {
12851328
/// docs of [`Builder`] and [`spawn`] for more details.
12861329
///
12871330
/// [`thread::current`]: current
1288-
pub struct Thread {
1289-
inner: Pin<Arc<Inner>>,
1290-
}
1331+
pub struct Thread(Inner);
12911332

12921333
impl Thread {
12931334
// Used only internally to construct a thread object without spawning
12941335
pub(crate) fn new(name: Option<CString>) -> Thread {
1295-
if let Some(name) = name {
1296-
Self::new_inner(ThreadName::Other(name))
1297-
} else {
1298-
Self::new_inner(ThreadName::Unnamed)
1299-
}
1300-
}
1301-
1302-
// Used in runtime to construct main thread
1303-
pub(crate) fn new_main() -> Thread {
1304-
Self::new_inner(ThreadName::Main)
1305-
}
1306-
1307-
fn new_inner(name: ThreadName) -> Thread {
13081336
// We have to use `unsafe` here to construct the `Parker` in-place,
13091337
// which is required for the UNIX implementation.
13101338
//
13111339
// SAFETY: We pin the Arc immediately after creation, so its address never
13121340
// changes.
13131341
let inner = unsafe {
1314-
let mut arc = Arc::<Inner>::new_uninit();
1342+
let mut arc = Arc::<OtherInner>::new_uninit();
13151343
let ptr = Arc::get_mut_unchecked(&mut arc).as_mut_ptr();
13161344
addr_of_mut!((*ptr).name).write(name);
13171345
addr_of_mut!((*ptr).id).write(ThreadId::new());
13181346
Parker::new_in_place(addr_of_mut!((*ptr).parker));
13191347
Pin::new_unchecked(arc.assume_init())
13201348
};
13211349

1322-
Thread { inner }
1350+
Self(Inner::Other(inner))
1351+
}
1352+
1353+
/// # Safety
1354+
///
1355+
/// This must only ever be called once, and must be called on the main thread.
1356+
pub(crate) unsafe fn new_main() -> Thread {
1357+
// Safety: Caller responsible for holding the mutable invariant and
1358+
// Parker::new_in_place does not read from the uninit value.
1359+
unsafe { Parker::new_in_place(MAIN_PARKER.get().cast()) }
1360+
1361+
Self(Inner::Main)
13231362
}
13241363

13251364
/// Like the public [`park`], but callable on any handle. This is used to
@@ -1328,7 +1367,7 @@ impl Thread {
13281367
/// # Safety
13291368
/// May only be called from the thread to which this handle belongs.
13301369
pub(crate) unsafe fn park(&self) {
1331-
unsafe { self.inner.as_ref().parker().park() }
1370+
unsafe { self.0.parker().park() }
13321371
}
13331372

13341373
/// Atomically makes the handle's token available if it is not already.
@@ -1364,7 +1403,7 @@ impl Thread {
13641403
#[stable(feature = "rust1", since = "1.0.0")]
13651404
#[inline]
13661405
pub fn unpark(&self) {
1367-
self.inner.as_ref().parker().unpark();
1406+
self.0.parker().unpark();
13681407
}
13691408

13701409
/// Gets the thread's unique identifier.
@@ -1384,7 +1423,7 @@ impl Thread {
13841423
#[stable(feature = "thread_id", since = "1.19.0")]
13851424
#[must_use]
13861425
pub fn id(&self) -> ThreadId {
1387-
self.inner.id
1426+
self.0.id()
13881427
}
13891428

13901429
/// Gets the thread's name.
@@ -1427,15 +1466,7 @@ impl Thread {
14271466
#[stable(feature = "rust1", since = "1.0.0")]
14281467
#[must_use]
14291468
pub fn name(&self) -> Option<&str> {
1430-
self.cname().map(|s| unsafe { str::from_utf8_unchecked(s.to_bytes()) })
1431-
}
1432-
1433-
fn cname(&self) -> Option<&CStr> {
1434-
match &self.inner.name {
1435-
ThreadName::Main => Some(c"main"),
1436-
ThreadName::Other(other) => Some(&other),
1437-
ThreadName::Unnamed => None,
1438-
}
1469+
self.0.name().map(|s| unsafe { str::from_utf8_unchecked(s.to_bytes()) })
14391470
}
14401471
}
14411472

tests/rustdoc/demo-allocator-54478.rs

+1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
//! }
4141
//!
4242
//! fn main() {
43+
//! drop(std::env::var("PWD"));
4344
//! assert!(unsafe { HIT });
4445
//! }
4546
//! ```

0 commit comments

Comments
 (0)