Skip to content
This repository has been archived by the owner on Mar 3, 2020. It is now read-only.

RFC: Let's make await a method and possibly deprecate wait #20

Closed
rushmorem opened this issue Sep 5, 2017 · 2 comments
Closed

RFC: Let's make await a method and possibly deprecate wait #20

rushmorem opened this issue Sep 5, 2017 · 2 comments

Comments

@rushmorem
Copy link

I know I have argued for totally getting rid of await!(...)? before (note that I was against await!(...)? not the await! macro itself, which would still be useful) but in light of @Nemo157's heroic exploits in #15 which I like for various reasons and the technical argument for using await!(...)? if we are to eventually merge his work I would like to propose that we adopt /u/exxplicit's idea of using .await() instead of await!(...). If we do this, async code will probably end up looking like so

// Just putting this here so you can see where the Ok and Err in the body are coming
// from but this will be imported by the `async` macro so these won't interfere with
// your other code 
use futures::future::{ok as Ok, err as Err};

#[async]
fn fetch_rust_lang(client: hyper::Client) -> impl Future<Item=String, Error=io::Error> {
    let response = client.get("https://www.rust-lang.org").await()?;
    if !response.status().is_success() {
        return Err(io::Error::new(io::ErrorKind::Other, "request failed"))
    }
    let body = response.body().concat().await()?;
    let string = String::from_utf8(body)?;
    Ok(string)
}

(Yeah, strictly speaking we still need to seriously consider this whether or not we end up merging @Nemo157's work but I couldn't resist the opportunity to advertise his work as well 😄)

We can easily experiment with this in this repo instead of adding it in the upstream futures library by making the Future and Stream traits that this library exports extend the upstream traits to add the await() method.

I know having both .wait() and .await() might be confusing to new users. However

  • .wait() and await!() will probably still confuse new users as well
  • we can try to mitigate this by clearly stating that await() is the asynchronous version of wait() in the documentation
  • I would also like to propose that we deprecate wait()

Here is why I think we should deprecate wait() besides the potential confusion that it causes with either await!() or await()

It's too easy to use incorrectly

I have seen a lot of people, myself included, use it incorrectly either out of ignorance or just because it's so convenient to use (sometimes both) ending up with code that block the entire thread unnecessarily. IMHO, the situation has just gotten worse with async/await. People who don't understand the difference between wait() and await!() might use wait() in the async macro (probably because it makes code easier to read) which will work. They will wonder why their programs are slow (it blocks the entire event loop), worse still they might not even realise this.

It makes task execution confusing

According to the documentation we have 3 methods of executing futures; wait(), the thread pool and the event loop. I have seen some people get confused about when to use which. The documentation tries to explain this but the fact that the thread pool returns a future and that all futures have a wait method doesn't help. If we deprecate wait() we remain with only 2, the thread pool and the event loop. However, considering that the thread pool simply returns a future, we will be forced to run that future on the event loop which brings us down to only one way to drive futures to completion.

Questions that have also come up include

  • when to use the thread pool
  • when to use the event loop
  • how to use both

In my experience there is only one ideal way to drive futures to completion. That is, using an event loop. If you have tasks that are CPU bound, throw them on the thread pool and grab the CPU futures you get back. If you have tasks that are IO bound construct them as needed and grab the IO futures you get back. Multiplex your CPU futures with your IO futures as needed (you can join, select them etc, all that good stuff) and grab the resulting future. Throw that resulting future on the event loop and you are done. In my experience, this is the most efficient way to use futures and deprecating wait() helps guide users towards this.

It's easy to implement

Those who really need it can easily implement it on top of await().

/cc @alexcrichton

@rushmorem
Copy link
Author

Also see the discussion going on in this Reddit thread.

@rushmorem
Copy link
Author

This RFC has been replaced by #22.

As pointed out by /u/MalenaErnman using await() as a method will not work well with std::ops::Try.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant