Unbounded spinning in `WakerSet`

[matklad]

WakerSet contains a spin lock which never falls back to OS-provided blocking facilities:

https://github.com/async-rs/async-std/blob/6d69a3e368869bb7fb6933baad3b0bc1cd5ef8f8/src/sync/waker_set.rs#L181-L182

I think this might lead to a pretty bad behavior in pathological cases, due to priority inversion.

Not sure what's the best solution here is though: std::sync::Mutex is not the fastest one in town, parking_lot is an external dependency.

See also https://matklad.github.io/2020/01/02/spinlocks-considered-harmful.html and https://probablydance.com/2019/12/30/measuring-mutexes-spinlocks-and-how-bad-the-linux-scheduler-really-is/.

[matklad]

Not sure what's the best solution here is though

Actually, I think there's a decent solution. We provide two implementations, std::sync::Mutex based one and a parking_lot::Mutex (or even raw park/unpark) based one, under a feature flag. Neither implementation suffers from pathalogical spinlocks behaviors, default implementation doesns't have extra dependencies (and is fast enough for most practical purposes), but, if you do benchmarking and/or need utmost performance for your workload, you can opt-in into an extra dependency. Beacause the feature flag does not unlock new API, it shouldn't add a lot to public API surface. That is, if/when std migrates to parking lot implementation, we can just migrate the flag to do nothing instead.

[yoshuawuyts]

@matklad thanks so much for the analysis! Organizationally it sounds like we could probably split this up into two parts:

• use the std::sync::Mutex impl to solve the primary issue
• add parking_lot::Mutex behind a feature flag as a follow-up optimization

This makes it so we can solve the issue at hand first without increasing the API surface, and later decide on how/if to add parking_lot. Also performance has not been my area of focus, so pinging @stjepang to weigh in here.