futures-lite icon indicating copy to clipboard operation
futures-lite copied to clipboard

join! macro to join more than two futures?

Open joshtriplett opened this issue 5 years ago • 12 comments

futures-lite implements the join function to join two futures. I'd love to have the join! macro to join more than two futures, if that'd be a reasonable addition to futures-lite.

joshtriplett avatar Jul 27 '20 04:07 joshtriplett

This is easy to add, but right now I've stumbled on an open design question.

In the futures crate, join(a, b) returns a future that outputs (A::Output, B::Output). However, join!(a, b) desugars to join(a, b).await, which means the macro evaluates to the final output.

So the macro version join!() is not just join() that accepts an arbitrary number of arguments -- it also contains the .await.

Speaking from experience, this is sometimes confusing. Reading code, it can be hard to tell whether a macro does or does not contain an await, so you have to think extra hard to remember or check every time.

I wonder if join!() macro here should omit the .await and be used as in join!(a, b).await.

ghost avatar Jul 27 '20 11:07 ghost

Related: https://github.com/rust-lang/futures-rs/issues/2127

ghost avatar Jul 27 '20 12:07 ghost

I think that it makes sense to have to .await join!. It seems to make it easier to understand, and it isn't at all a lot of extra syntax to add one .await. The only negative I imagine is that some people are probably already used to join! returning outputs and not futures.

Still it seems worth making the it functionally similar to the join function I think.

zicklag avatar Jul 27 '20 14:07 zicklag

I agree that it makes sense to not hide the await inside the macro. However, it also seems potentially confusing to have two join! macros that do subtly different things, depending on if you use futures or futures-lite. "Potentially confusing" on a scale of "this would become the most frequent issue by far when migrating from one to the other".

Perhaps it would help to use a different name for this macro in futures-lite? That gives the opportunity for someone to see documentation.

joshtriplett avatar Jul 27 '20 16:07 joshtriplett

What if we called this zip!/zip() rather than join!/join(), taking inspiration from Option::zip() and Iterator::zip()?

  • https://doc.rust-lang.org/nightly/std/iter/trait.Iterator.html#method.zip
  • https://doc.rust-lang.org/nightly/std/option/enum.Option.html#method.zip

Look, it makes sense:

  • fn zip<A, B>(a: Option<A>, b: Option<B>) -> Option<(A, B)>
  • fn zip<A, B>(a: impl Iterator<Item = A>, b: impl Iterator<Item = B>) -> impl Iterator<Output = (A, B)>
  • fn zip<A, B>(a: impl Future<Output = A>, b: impl Future<Output = B>) -> impl Future<Output = (A, B)>

ghost avatar Jul 27 '20 18:07 ghost

@stjepang Sold. That makes perfect sense to me.

That would allow futures to also add zip!, since it wouldn't conflict with the existing join!. And futures-lite can just only add zip!, and mention join! in the documentation for zip! so that people searching for join! as part of porting will find what to use instead.

joshtriplett avatar Jul 29 '20 07:07 joshtriplett

:+1: for me. I think that makes a lot of sense.

zicklag avatar Jul 29 '20 15:07 zicklag

Why not add a zip_all() and/or something like FuturesUnordered?

stackinspector avatar Nov 27 '22 10:11 stackinspector

FuturesUnordered is a bit of a footgun. If you want to implement this pattern with a substantial number of futures, you should probably use an executor like async-executor instead of this combinator.

notgull avatar Nov 27 '22 15:11 notgull

I just wanted to simply wait for several futures in parallel, without even required for the results to be returned. Then I looked at the implementation of join_all() and found that it uses FuturesUnordered internally.

stackinspector avatar Nov 27 '22 20:11 stackinspector

The join_all function, for a large number of futures, tends to be inefficient. You'd do better to use async_executor instead. For instance:

use async_executor::LocalExecutor;

let exec = LocalExecutor::new();
let mut handles = vec![];

for future in futures_to_run() {
    handles.push(exec.spawn(future));
}

exec.run(async move {
    for handle in handles {
        handle.await;
    }
});

notgull avatar Nov 27 '22 21:11 notgull