-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add an "async" session #496
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
NOTE: `mdbook test` does not allow code samples to reference other crates, so they must be marked as `compile_fail`; see #175.
These concepts are closely related, and there's not much else to know about runtimes other than "they exist". This removes the bit about futures being "inert" because it doesn't really lead anywhere.
* Add async channels chapter * Async control flow * Async pitfalls * Separate in multiple chapters + add daemon section * Merge reentering threads in blocking-executor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this together.
My general impression is that this is more like a chapter on tokio
than async
. Is that the intention? In particular, I think most of the basic concepts can be demonstrated using the futures crate, and with less boilerplate code.
Also, I remember there were some discussions about whether the content is for a full-day or a half-day session. Looks like we are aiming for the latter (i.e., day 4 afternoon after a morning on concurrency). This means we have only 2 hours for teaching this content. I feel like the content is too heavy for only 2 hours.
- [async/await](async/async-await.md) | ||
- [Async Blocks](async/async-blocks.md) | ||
- [Futures](async/futures.md) | ||
- [Runtimes](async/runtimes.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is better to talk about select & join earlier. I am guessing a question about that will come up rather early.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how or where.. they are simple to understand but a little more complicated to demonstrate.
``` | ||
|
||
<details> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps add a note on why this is not supported (i.e., the resulting type not having a known type at compile time), and the overhead of using async_trait (because it does heap allocation).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By "supported", do you mean why this isn't stabilized?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know enough about that to summarize - perhaps you can add something? I read a lot of blog posts on the topic!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I can add something once you've merged #556. I think I learnt about it in an episode of Crust of Rust. We could also leave it out, as it might be too much detail.
I think the instructors can choose to go into more depth as they see fit when teaching the course. We don't need to cover everything in the notes, but since the speaker notes for this section are are already so comprehensive, I was tempted to suggest adding this too :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#556 is merged. It can't hurt to include in the notes - maybe with an "If a student asks.." preface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If think I know why but I would have a hard time explaining, so let me provide a pointer and you can explain better than me:
If you had a trait with fn fetch_data_count(&self) -> usize
, then the compiler knows that it needs to push 64 bits to the stack for the return value, whatever the type of the caller.
Now if we had async fn fetch_data_count(&self) -> usize
, since we actually return a future, the stack of the future needs to also be allocated, so the size of the return value depends on the actual method.
Consider both implementations:
async fn fetch_data_count(&self) -> usize {
let data: [i32; 1000] = fetch().await;
let filter: i32 = fetch_filter().await;
data.iter().filter(|&item| item != filter).count()
}
async fn fetch_data_count(&self) -> usize {
let data: Vec<i32> = fetch().await;
let filter: i32 = fetch_filter().await;
data.iter().filter(|&item| item != filter).count()
}
Example one, with the data on the stack probably has a higher size than the second one with the Vec. Using dynamic dispatch, it would be impossible for the compiler to know ahead of time the size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heap allocating fixes the size issue. Note that Box does not always allocate and the compiler is often able to omit allocations if it can figure out the size
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Exactly. And I just noticed that I have a typo in my original comment; it should be "the resulting type not having a known size at compile time".
We cannot add all the details in the notes, so I was thinking of just a two sentence summary. I'll create a PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sizing is a difficulty, but not the only one. Offhand, there's also:
- Bounds on the future (most notably
Send
) - Bounds on the associated types (most notably lifetime)
- Specifying associated type values to allow use as
dyn Trait
- (Formerly) each method (under the hood) needs an associated type, which must be generic and therefore needs GATs
- Other design hairiness such as duplication vs parameterization (keyword generics, trait transformers, etc.)
It doesn't seem worth diving into all of these, but it feels misleading to suggest that Sized
is the only one.
I think Niko Matsakis's 2019 post remains the best overview of the challenges. It's what I'd send to anyone wondering "why not...?". There's been a lot written since then about potential designs and the tradeoffs, but none of it seems "settled enough" to incorporate (at least to me, a person who follows progress from a distance).
Thanks for taking a comprehensive look at the assembled afternoon! I've not gotten a chance to do so myself but I will do so shortly. I agree that this is far too much content for 2 hours (and yes, that's been the intended timeframe). In that time I think we can only remove some of the mystery around async Rust. I suspect the situation is similar with Android + Rust -- students have the context to begin working with Rust in Android, but will still need to learn the details. As for whether it's a |
I was thinking mostly about the first few slides. I think it is too much to talk about |
That's fair - let's see what you can come up with. I felt pretty constrained by having to write something that ran in the playground, but maybe I missed something, or maybe that constraint can be relaxed. |
* Simplify the async-await slide * Shorten futures and move it up * Add a page on Tokio
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK! I've made a PR addressing most of these comments: #556.
src/async/control-flow/select.md
Outdated
|
||
* In this example, we have a race between a cat and a dog. `first_animal_to_finish_race` listens to both channels and will pick whichever arrives first. Since the dog takes 5 seconds, it wins against the cat that take 10 seconds. | ||
* You can use `oneshot` channels in this example as the channels are supposed to receive only one `send`. | ||
* You can try adding more contestants to the race and return a leaderboard. Also, you can add a deadline after which contestants get eliminated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the deadline part of this "Try .." feels less contrived, so I'll stick with that.
``` | ||
|
||
<details> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By "supported", do you mean why this isn't stabilized?
- [async/await](async/async-await.md) | ||
- [Async Blocks](async/async-blocks.md) | ||
- [Futures](async/futures.md) | ||
- [Runtimes](async/runtimes.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how or where.. they are simple to understand but a little more complicated to demonstrate.
* Modifications to the async section * Remove the "Daemon" slide, as it largely duplicates the "Tasks" slide. The introduction to the "Control Flow" section mentions tasks as a kind of control flow. * Reorganize the structure in SUMMARY.md to correspond to the directory structure. * Simplify the "Pin" and "Blocking the Executor" slides with steps in the speaker notes to demonstrate / fix the issues. * Rename "join_all" to "Join". * Simplify some code samples to shorten them, and to print output rather than asserting. * Clarify speaker notes and include more "Try.." suggestions. * Be consistent about where `async` blocks are introduced (in the "Tasks" slide). * Explain `join` and `select` in prose. * Fix formatting of section-header slides.
I want to use some new stuff that's in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Thanks.
``` | ||
|
||
<details> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Exactly. And I just noticed that I have a typo in my original comment; it should be "the resulting type not having a known size at compile time".
We cannot add all the details in the notes, so I was thinking of just a two sentence summary. I'll create a PR.
* Try adding a deadline to the race, demonstrating selecting different sorts of | ||
futures. | ||
|
||
* Note that `select!` consumes the futures it is given, and is easiest to use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: maybe add "by default"?
select!
doesn't have to consume the future (in the fn(s: Foo)
sense). It works with mutable references too. Maybe this isn't worth bothering with since the "pin" section demonstrates this; I just found the wording confusing/misleading on first read.
``` | ||
|
||
<details> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sizing is a difficulty, but not the only one. Offhand, there's also:
- Bounds on the future (most notably
Send
) - Bounds on the associated types (most notably lifetime)
- Specifying associated type values to allow use as
dyn Trait
- (Formerly) each method (under the hood) needs an associated type, which must be generic and therefore needs GATs
- Other design hairiness such as duplication vs parameterization (keyword generics, trait transformers, etc.)
It doesn't seem worth diving into all of these, but it feels misleading to suggest that Sized
is the only one.
I think Niko Matsakis's 2019 post remains the best overview of the challenges. It's what I'd send to anyone wondering "why not...?". There's been a lot written since then about potential designs and the tradeoffs, but none of it seems "settled enough" to incorporate (at least to me, a person who follows progress from a distance).
|
||
println!("connection from {addr:?}"); | ||
|
||
tokio::spawn(async move { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the first time async blocks appear (also alluded to in async-await and pin sections). They probably merit a sentence somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's mentioned in the speaker notes. In general, it's a non-goal to put all explanation on the slide itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this was a failure in my search. I agree the note in speaker notes is good and sufficient.
For context, I noticed async move
later and was trying to find where async blocks was explained/mentioned. Both "async" and "block" appear a lot, so I searched for async block
, async {
, or async move
. (I don't think there's anything actionable here.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @djmitche, @rbehjati, and @sakex for all your work on this!
I really like the notes, they helped me a lot in understanding the slides.
I've left a few small comments which can be fixed after merging the PR — I think it'll be great to get this published so that we can get some more real-world feedback on it.
|
||
If you chose Async for Day 4 afternoon, you will need a fresh crate set up and | ||
the dependencies downloaded and ready to go. You can then copy/paste the | ||
examples into `src/main.rs` to experiment with them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest adding the large async examples to the repository as full Cargo projects (similar to how the Android chapter handles things).
That makes it trivial for an instructor to find known-good versions of the code, and we also get coverage from cargo build
when you add those projects to the workspace for the repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Thanks! I'll make up some new PRs and/or issues to address these items. |
These address some comments in google#496.
These address some comments in #496.
* beginning of an Async section * address review comments * Add futures page (google#497) NOTE: `mdbook test` does not allow code samples to reference other crates, so they must be marked as `compile_fail`; see google#175. * Add Runtimes & Tasks (google#522) These concepts are closely related, and there's not much else to know about runtimes other than "they exist". This removes the bit about futures being "inert" because it doesn't really lead anywhere. * Async chapter (google#524) * Add async channels chapter * Async control flow * Async pitfalls * Separate in multiple chapters + add daemon section * Merge reentering threads in blocking-executor * async_trait * Async fixes (google#546) * Async: some ideas for simplifying the content (google#550) * Simplify the async-await slide * Shorten futures and move it up * Add a page on Tokio * Modifications to the async section (google#556) * Modifications to the async section * Remove the "Daemon" slide, as it largely duplicates the "Tasks" slide. The introduction to the "Control Flow" section mentions tasks as a kind of control flow. * Reorganize the structure in SUMMARY.md to correspond to the directory structure. * Simplify the "Pin" and "Blocking the Executor" slides with steps in the speaker notes to demonstrate / fix the issues. * Rename "join_all" to "Join". * Simplify some code samples to shorten them, and to print output rather than asserting. * Clarify speaker notes and include more "Try.." suggestions. * Be consistent about where `async` blocks are introduced (in the "Tasks" slide). * Explain `join` and `select` in prose. * Fix formatting of section-header slides. * Add a note on async trait (google#558) --------- Co-authored-by: sakex <[email protected]> Co-authored-by: rbehjati <[email protected]>
These address some comments in google#496.
This is a continuation of #492, but against a branch in the upstream repo so that @rbehjati and I can both make PRs against it. Once the session is complete we can merge this PR to put it into
main
.