smol-rs / futures-lite

Futures, streams, and async I/O combinators.
Apache License 2.0
439 stars 25 forks source link

join! macro to join more than two futures? #6

Open joshtriplett opened 4 years ago

joshtriplett commented 4 years ago

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.

ghost commented 4 years ago

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 commented 4 years ago

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

zicklag commented 4 years ago

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.

joshtriplett commented 4 years ago

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.

ghost commented 4 years ago

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

Look, it makes sense:

joshtriplett commented 4 years ago

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

zicklag commented 4 years ago

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

stackinspector commented 1 year ago

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

notgull commented 1 year ago

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.

stackinspector commented 1 year ago

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.

notgull commented 1 year ago

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;
    }
});
brandonros commented 1 week ago

not sure who this may help but I was able to get really far with just futures-lite block_on and then realized I needed to add async_executor (as called out thanks to @notgull) in order to do more than 2 futures in parallel at once

use std::future::Future;

use async_executor::{LocalExecutor, Task};

pub async fn join_all<F, T, E>(futures: Vec<F>) -> Result<Vec<T>, E>
where
    F: Future<Output = Result<T, E>>,
{
    let exec = LocalExecutor::new();

    // Spawn each future onto the executor and collect the task handles
    let handles: Vec<Task<Result<T, E>>> = futures.into_iter()
        .map(|future| exec.spawn(future))
        .collect();

    // Run the executor and join all the tasks
    exec.run(async move {
        let mut results = Vec::with_capacity(handles.len());
        for handle in handles {
            results.push(handle.await?);
        }
        Ok(results)
    }).await
}
let mut futures = vec![];
for chunk in &chunks {
    futures.push(Robinhood::get_marketdata_options(chunk));
}
let results = utilities::join_all(futures).await?;