Add fairness to futures_util::{select, try_select, select_ok}

[Xaeroxe]

Fixes #2135 by making these three functions fair, and then documenting that they are fair.

[Xaeroxe]

Hmm so I'm only just now realizing this repo has its own implementation of randomness. I'm willing to change to use that one, but I also like mine so I'm going to wait for a code reviewer to tell me what to do.

[taiki-e]

futures-rs prefers to use our own PRNG implementation because rand has caused maintenance pains (#1664 (comment), #1425, #1804, #1489, etc.) in the past.

[Xaeroxe]

@taiki-e Okay, this PR has been updated to omit rand.

[Xaeroxe]

FYI, these CI failures are not related to this PR.

[Xaeroxe]

CI passes now!

[goffrie]

If we want to do this, can we also add deterministic (biased) versions of these combinators? In our codebase the randomized versions are undesirable as we want our execution to be completely deterministic.

[Xaeroxe]

@goffrie There is select_biased. Does that meet your needs?

[goffrie]

No, because that's only usable in macro form and the combinator form is often simpler to use.

[Xaeroxe]

@goffrie alright, though it's not too difficult to adapt it into combinator form outside of the futures-rs repo. Something like

pub async fn select_biased<Fut1, Fut2>(
    mut a: Fut1,
    mut b: Fut2,
) -> Either<Fut1::Output, Fut2::Output>
where
    Fut1: FusedFuture + Unpin,
    Fut2: FusedFuture + Unpin,
{
    futures::select_biased! {
        a_res = a => Either::Left(a_res),
        b_res = b => Either::Right(b_res),
    }
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=ff6bea6682902ab3eaf781738e182368

[Xaeroxe]

I have resolved the merge conflicts.

[Xaeroxe]

CI failures seem to be unrelated.

[Xaeroxe]

@taiki-e just to clarify, who needs to make a decision on this?

[taiki-e]

who needs to make a decision on this?

The maintainer needs to decide whether to accept this, and if so, whether to accept this API as is, whether to provide a biased variant, etc.

Opinions and summaries on the advantages and disadvantages of these would be helpful in making a decision.

[Xaeroxe]

Fairness by default is preferable in my opinion, because the alternative is a foot gun. In particular it is common to call select in a loop. If the first argument is providing output at a rate which the loop can keep up with, there is no problem. However once the first argument starts outputting at a rate beyond the loop's capacity, in an unfair system, now the second argument is completely starved. It is not processed at all because all of the attention is going to the first argument. This can be very confusing. Why is the machinery tied to the second argument not running? We're selecting over both of them right? I personally have spent far too much time debugging such systems only to realize that the select machinery is not fair.

So, in an unloaded system, both futures are processed as they become available, and in a loaded system, only one future is processed ever. Some may see this behavior as desirable, but in my opinion this is surprising enough that a bias in select should be opt in.

Now, should the biased variant be included? I don't really have a compelling reason to exclude it, especially because I've already authored the biased variant, and it ended up being pretty easy to maintain. It just forwards to a pre-existing macro.

[Xaeroxe]

I pushed a commit which adds the biased variant as well as the tests for it. I didn't use my original implementation though, because I realized that from the standpoint of futures-util having a mandatory dependency on futures-macro wasn't ideal. So, I created a copy of select and then simplified the behavior.

[Xaeroxe]

I realized I had two other functions I needed to write still, and that this code was getting repetitive fast. So I changed tactic and added a biased boolean to each of the future structs, and then branched on that if the std feature was enabled.

[Xaeroxe]

I have a stash where I converted this to rely on const generics in order to cut back on branching for the biased functions. However I can't assign a default value to a const generic until Rust 1.59. The current MSRV is 1.56. It's not really feasible to do this without a default const generic due to the large amount of places where this type is specified explicitly in the futures code base (and, let's be honest, probably elsewhere in the rust ecosystem)

[Xaeroxe]

The most recent commit provides yet another option. Rather than using const generics, use a generic type with a default value of Fair. The alternative being Biased. Both structures implement a simple trait called IsBiased which contains a constant boolean determining if the parent future is biased or not.

[Xaeroxe]

This PR has been updated to resolve the merge conflicts.