[conradludgate]

I don't think I'd recommend using this in production. The benchmarks look good, but by immediately waking the waker[0], you've effectively created a spin-lock. They may work in some very specific circumstances, but they will most likely in practice be more costly to your scheduler (which likely uses locks btw) than just using locks

[0]: https://github.com/fortress-build/whirlwind/blob/0e4ae5a2aba...

[sujayakar]

+1. I'd be curious how much of a pessimization to uncontended workloads it'd be to just use `tokio::sync::RwLock`.

and, if we want to keep it as a spinlock, I'm curious how much the immediate wakeup compares to using `tokio::task::yield_now`: https://docs.rs/tokio/latest/tokio/task/fn.yield_now.html

[willothy]

This is an interesting idea. I am gonna try this out - especially with dashmap, I think that could perform very well.

[zamalek]

You could also look into shamelessly "taking inspiration" from async-lock.

[willothy]

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]

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]

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/8897885425bf3d89053f8... [1]: https://github.com/tokio-rs/tokio/blob/8897885425bf3d89053f8...

[willothy]

Oh this is really good to know, thank you!

[jkelleyrtp]

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]

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]

Another offender is AtomicWaker [1] which does the same on contention.

[1] https://docs.rs/atomic-waker/latest/atomic_waker/

[SabrinaJewson]

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]

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).

[willothy]

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.