7 ms·

I don't believe that waking the waker in `poll` synchronously waits / runs poll again immediately. I think it is more likely just adding the future back to the

I don't believe that waking the waker in `poll` synchronously waits / runs poll again immediately. I think it is more likely just adding the future back to the global queue to be polled. I could be wrong though, I'll look into this more. Thanks for the info!

conradludgate 2y ago
It does immediately put itself into the queue to be polled again. But that's no different in effect to a spin-lock. If you have other tasks in your runtime, this will be putting excess pressure on your scheduler
- conradludgate 2y ago
  Expanding on this. If you have a lot of concurrent tasks, you will overflow[0] the task local queue and be bottlenecked by the global queue mutex[1] [0]: https://github.com/tokio-rs/tokio/blob/8897885425bf3d89053f896319eeb8777cf255fc/tokio/src/runtime/scheduler/multi_thread/queue.rs#L63 https://github.com/tokio-rs/tokio/blob/8897885425bf3d89053f8... [1]: https://github.com/tokio-rs/tokio/blob/8897885425bf3d89053f896319eeb8777cf255fc/tokio/src/runtime/scheduler/inject/rt_multi_thread.rs#L77 https://github.com/tokio-rs/tokio/blob/8897885425bf3d89053f8...
  - willothy 2y ago
    Oh this is really good to know, thank you!
- willothy 2y ago
  Hmm, imo it's definitely better than directly spinlocking to have many spinlocks running cooperatively, but you're right that it may not be ideal. Thanks for pointing this out. I'll see if I can find a better way to coordinate the polling/waking of lock acquisition futures.
- jkelleyrtp 2y ago
  Tokio has a task budget that will cap at 32 or 256 polls of the same task before switching to another task. So, yes a spinlock, but not likely to deadlock.
  - willothy 2y ago
    Yeah, after looking into this more I think this was a big oversight on my part. Working on a (hopefully) better way of doing this right now - I'm thinking per-shard waker queues and only falling back to spinlocking like this if the queues are full.
- binarycoffee 2y ago
  Another offender is AtomicWaker [1] which does the same on contention. [1] https://docs.rs/atomic-waker/latest/atomic_waker/ https://docs.rs/atomic-waker/latest/atomic_waker/
  - SabrinaJewson 2y ago
    AtomicWaker is much less bad, because it will only spin in the case that another thread has spuriously called `wake` – i.e. called `wake` despite that fact that the future it’s waking is in fact unable to progress. In the common case, the waker side will operate some logic like: set_flag(); atomic_waker.wake(); and the future will run: atomic_waker.register(cx.waker()); if flag_is_set() { Poll::Ready(()) } else { Poll::Pending } Thus, even if `register` decides to “spin”, the flag will already be set, and so the future will not spin in reality (it may be polled unnecessarily, but this will only happen once). I can’t immediately think of examples where support for spurious waking is necessary.
    - binarycoffee 2y ago
      The producers of an MPSC queue may use an AtomicWaker::wake to signal to the consumer that a new item is ready. In this case all wake-ups are necessary (not spurious).

conradludgate 2y ago

It does immediately put itself into the queue to be polled again. But that's no different in effect to a spin-lock. If you have other tasks in your runtime, this will be putting excess pressure on your scheduler

conradludgate 2y ago
Expanding on this. If you have a lot of concurrent tasks, you will overflow[0] the task local queue and be bottlenecked by the global queue mutex[1] [0]: https://github.com/tokio-rs/tokio/blob/8897885425bf3d89053f896319eeb8777cf255fc/tokio/src/runtime/scheduler/multi_thread/queue.rs#L63 https://github.com/tokio-rs/tokio/blob/8897885425bf3d89053f8... [1]: https://github.com/tokio-rs/tokio/blob/8897885425bf3d89053f896319eeb8777cf255fc/tokio/src/runtime/scheduler/inject/rt_multi_thread.rs#L77 https://github.com/tokio-rs/tokio/blob/8897885425bf3d89053f8...
- willothy 2y ago
  Oh this is really good to know, thank you!
willothy 2y ago
Hmm, imo it's definitely better than directly spinlocking to have many spinlocks running cooperatively, but you're right that it may not be ideal. Thanks for pointing this out. I'll see if I can find a better way to coordinate the polling/waking of lock acquisition futures.
jkelleyrtp 2y ago
Tokio has a task budget that will cap at 32 or 256 polls of the same task before switching to another task. So, yes a spinlock, but not likely to deadlock.
- willothy 2y ago
  Yeah, after looking into this more I think this was a big oversight on my part. Working on a (hopefully) better way of doing this right now - I'm thinking per-shard waker queues and only falling back to spinlocking like this if the queues are full.
binarycoffee 2y ago
Another offender is AtomicWaker [1] which does the same on contention. [1] https://docs.rs/atomic-waker/latest/atomic_waker/ https://docs.rs/atomic-waker/latest/atomic_waker/
- SabrinaJewson 2y ago
  AtomicWaker is much less bad, because it will only spin in the case that another thread has spuriously called `wake` – i.e. called `wake` despite that fact that the future it’s waking is in fact unable to progress. In the common case, the waker side will operate some logic like: set_flag(); atomic_waker.wake(); and the future will run: atomic_waker.register(cx.waker()); if flag_is_set() { Poll::Ready(()) } else { Poll::Pending } Thus, even if `register` decides to “spin”, the flag will already be set, and so the future will not spin in reality (it may be polled unnecessarily, but this will only happen once). I can’t immediately think of examples where support for spurious waking is necessary.
  - binarycoffee 2y ago
    The producers of an MPSC queue may use an AtomicWaker::wake to signal to the consumer that a new item is ready. In this case all wake-ups are necessary (not spurious).