rust-lang / libs-team

The home of the library team
Apache License 2.0
127 stars 19 forks source link

ACP: add `Thread::into_raw` and `Thread::from_raw` #200

Closed h33p closed 2 months ago

h33p commented 1 year ago

Proposal

Problem statement

It should be possible to convert a Thread from/to a raw pointer for passing thread handles across API boundaries in an opaque manner.

Motivation, use-cases

Primary use case would be optimizing small futures executors. pollster - a minimalistic futures executor with over 1 million downloads would be a direct beneficiary of this. Currently, pollster needs to heap allocate a signal on every future being executed. Alternative would be to use a thread_local! store of static signals/thread handles, but why bother, when a Thread is already that?

As of today, Thread is merely a newtype around opaque Arc, and I was able to abuse this knowledge to remove heap allocations from pollster and speed it up by up to 2x (https://github.com/zesterer/pollster/pull/19). Thus, it would be trivial to add methods to convert the thread handle into a raw pointer, and go the other way around.

Additional use cases would be passing thread handles across FFI-boundary, which would enable better interoperability with other languages and dynamically loaded Rust libraries.

Solution sketches

We would add these functions to Thread implementation:

#[must_use]
pub fn into_raw(self) -> *const () {
    let inner = unsafe { Pin::into_inner_unchecked(self.inner) };
    Arc::into_raw(inner) as *const ()
}

pub unsafe fn from_raw(raw: *const ()) -> Thread {
    Thread {
        inner: Pin::new_unchecked(Arc::from_raw(raw as *const Inner))
    }
}

Links and related work

Original PR to pollster: https://github.com/zesterer/pollster/pull/14 Prior implementation of this ACP: https://github.com/rust-lang/rust/pull/97524 Implementation of this ACP (I closed it as duplicate): https://github.com/rust-lang/rust/pull/109794

joboet commented 1 year ago

Just for reference: this has already been proposed, with the same API: rust-lang/rust#97524

h33p commented 1 year ago

Oh, my bad! Appears that I forgot to check for the most basic of things 🙃

joboet commented 1 year ago

Not to worry, it was quite buried. I just had a déjà vu. Anyway, it's probably best that this now gets a proper ACP...

joshtriplett commented 2 months ago

We reviewed this (long-delayed) ACP in today's @rust-lang/libs-api meeting, and approved it. Thank you!