Introduce biased argument for select!
#3603
Conversation
Similar to `futures::select_biased!`, it is sometimes beneficial to provide developers with strict control over the polling order of futures. Most of the time, the fairness guarantees that `select!` provides through RNG can be thought out by a careful developer to ensure that no one future will monopolize all of the CPU time. In exchange for this, the developer gets a performance boost as they do not need to spend time generating endless random numbers. Fixes: tokio-rs#2181
Darksonn
added
A-tokio
| $crate::select!(@{ () } $p = $($t)*) | ||
| // Randomly generate a starting point. This makes `select!` a bit more | ||
| // fair and avoids always polling the first future. | ||
| $crate::select!(@{ start={ $crate::macros::support::thread_rng_n(BRANCHES) }; () } $p = $($t)*) |
There was a problem hiding this comment.
Huh, it surprises me that we can access BRANCHES here.
There was a problem hiding this comment.
It surprised me too, but I was very happy that my intuition that we couldn't was wrong 😄
|
I think the PR looks reasonable, and this is a feature I want. To me, the main question is the syntax choice. |
|
LGTM, but could we add some tests to |
|
Besides that, it's a very well documented PR. I appreciate the detailed description. Thanks! |
|
This is really nice to have — our codebase mainly uses |
|
@carllerche thanks for the review, but what additional test coverage are you looking for? I added the doc test on the polling order because I saw that doc comments are preferred in the contributor guide, and I think what's tested here is really the only novel difference with the normal |
|
It would probably be nice with a test where the |
|
@Darksonn feel free to merge this if/when you feel it is ready. |
|
Thank you @Darksonn and @carllerche! 🎉 |
| /// are always ready. | ||
| /// | ||
| /// This behavior can be overridden by adding `biased;` to the beginning of the | ||
| /// macro usage. See the exmples for details. This will cause `select` to poll |
Similar to
futures::select_biased!, it is sometimes beneficial toprovide developers with strict control over the polling order of
futures. Most of the time, the fairness guarantees that
select!provides through RNG can be thought out by a careful developer to ensure
that no one future will monopolize all of the CPU time. In exchange for
this, the developer gets a performance boost as they do not need to
spend time generating endless random numbers.
Fixes: #2181
Motivation
I want to control future polling order - both for more determinism and to decrease CPU burden from generating random numbers.
Solution
By adding a
biased;argument toselect!, we can disable the RNG that select performs by default. If this is omitted, we continue to do RNG and behavior is unchanged.Alternatives
There are a myriad of syntax choices that can be taken to activate this code path. There's a bit of discussion in the linked issue, but I can summarize here.
select_biased!macro could be introduced, but Carl is against thisbiased, we could allow developers to include the index to start polling at directly. I'm against this because it would make the polling order very hard to understand. As-is, you can read from top to bottom and not worry about skipping around, very straightforward.biased;because it matches withselect_biased!from futures. If others have alternatives that are more clear, I'm very open to changing this.