Replace parking with std::thread::park

smol-rs / futures-lite

Futures, streams, and async I/O combinators.

Apache License 2.0

439 stars 25 forks source link

Replace parking with std::thread::park #54

Closed james7132 closed 2 years ago

james7132 commented 2 years ago

parking adds extra compile time for something that's already available in the std lib. This PR replaces parking with std::thread::park and Thread::unpark when using future::block_on. It has also gotten signfigant platform-specific improvements that parking lacks. This might add one additional thread local lookup when the thread is parked, but if you're parking the thread anyway, I don't think that overhead really matters.

notgull commented 2 years ago

Doesn't park() add spurious wakeups, which parking avoids? I don't know, I'm still waiting on an answer for smol-rs/parking#7

sbarral commented 2 years ago

AFAIU the reason for using parking is not to avoid spurious wakeups but to prevent the risk of deadlock when the future also makes use of park/unpark. See this issue: https://github.com/rust-lang/futures-rs/pull/2010

taiki-e commented 2 years ago

Yeah, the use of parking should be correct here. Also, parking is enough small and I don't think it would cause an increase in compile time in practice. (Try building with the --timings flag.)