Skip to content

Use WorkerThread directly in thread-local instead of a pointer#1301

Open
zetanumbers wants to merge 2 commits into
rayon-rs:mainfrom
zetanumbers:inline_worker_thread_state
Open

Use WorkerThread directly in thread-local instead of a pointer#1301
zetanumbers wants to merge 2 commits into
rayon-rs:mainfrom
zetanumbers:inline_worker_thread_state

Conversation

@zetanumbers

Copy link
Copy Markdown

This is a simplified version of rust-lang/rust#155688 for the upstream. This PR changes WORKER_THREAD_STATE type to store WorkerThread directly in the thread-local to avoid indirection.

@zetanumbers

Copy link
Copy Markdown
Author

I've tried to measure it, but sadly I couldn't get reliable numbers on my machine for some reason. I'll try using my laptop instead but until then I dunno.

F: FnOnce(&WorkerThread) -> R,
{
WORKER_THREAD_STATE.with(|t| {
struct Guard(*mut Option<WorkerThread>);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to introduce this helper using an internal guard instead of keeping the WorkerThread as the guard itself?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is as long as we want to keep WorkerThread contained in the thread-local and drop it at the end of this function.

#[inline]
pub(super) fn current() -> *const WorkerThread {
WORKER_THREAD_STATE.get()
WORKER_THREAD_STATE.with(|t| unsafe { (&*t.get()).as_ref().map_or(ptr::null(), |r| r) })

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looks like UB as the provenance you get from the reference won't be valid outside with.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I don't see any problem with the provenance, I didn't write this code to follow tree-borrows or stack-borrows. It's a temporary simplification from rust-lang/rust#155688 and it was intended to allow us to quickly measure performance impact and decide to go for it or not with more changes, as currently tree/stack-borrows would not impact this code AFAIK. And maybe there was a rule allowing this behavior in tree borrows but I'm not sure.

@cuviper

cuviper commented Apr 27, 2026

Copy link
Copy Markdown
Member

Here's a microbenchmark:

#![feature(test)]
extern crate test;

#[bench]
fn global_num_threads(b: &mut test::Bencher) {
    // prime the global thread pool first
    _ = rayon::current_num_threads();
    b.iter(|| rayon::current_num_threads());
}

#[bench]
fn local_num_threads(b: &mut test::Bencher) {
    rayon::scope(|_| {
        b.iter(|| rayon::current_num_threads());
    });
}

#[bench]
fn global_thread_index(b: &mut test::Bencher) {
    assert!(rayon::current_thread_index().is_none());
    b.iter(|| rayon::current_thread_index());
}

#[bench]
fn local_thread_index(b: &mut test::Bencher) {
    rayon::scope(|_| {
        assert!(rayon::current_thread_index().is_some());
        b.iter(|| rayon::current_thread_index());
    });
}

I get nearly identical results on my machine (Linux 9800X3D), within noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants