Skip to content
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

Tracking Issue for #![feature(async_iterator)] #79024

Open
3 of 9 tasks
Tracked by #7
yoshuawuyts opened this issue Nov 13, 2020 · 19 comments
Open
3 of 9 tasks
Tracked by #7

Tracking Issue for #![feature(async_iterator)] #79024

yoshuawuyts opened this issue Nov 13, 2020 · 19 comments
Labels
A-async-await Area: Async & Await AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@yoshuawuyts
Copy link
Member

yoshuawuyts commented Nov 13, 2020

This is a tracking issue for the RFC "2996" (rust-lang/rfcs#2996).
The feature gate for the issue is #![feature(async_iterator)].

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved Questions

  • Clarify the panic behavior of Stream and Iterator Add core::stream::Stream #79023 (comment)
    • Add a panic section to Iterator, clarifying panic behavior. The panic behavior between Stream and Iterator should be consistent.
  • Restore Stream::next. This was removed from the RFC because it prevents dynamic dispatch, and subsequently removed from the implementation. This should be resolved before stabilizing.
    • Investigate whether we can move the trait from fn poll_next to async fn next once we can use async in traits.
  • Investigate as part of keyword-generics whether we can merge Iterator and AsyncIterator into a single trait which is generic over "asyncness".
  • Should we name this API AsyncIterator instead? Tracking Issue for #![feature(async_iterator)] #79024 (comment)

Implementation history

@yoshuawuyts yoshuawuyts added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Nov 13, 2020
@jonas-schievink jonas-schievink added A-async-await Area: Async & Await T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 13, 2020
@tmandry tmandry added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Dec 3, 2020
@KodrAus KodrAus added the Libs-Tracked Libs issues that are tracked on the team's project board. label Dec 16, 2020
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 30, 2021
Add `core::stream::Stream`

[[Tracking issue: rust-lang#79024](rust-lang#79024)]

This patch adds the `core::stream` submodule and implements `core::stream::Stream` in accordance with [RFC2996](rust-lang/rfcs#2996). The RFC hasn't been merged yet, but as requested by the libs team in rust-lang/rfcs#2996 (comment) I'm filing this PR to get the ball rolling.

## Documentatation

The docs in this PR have been adapted from [`std::iter`](https://doc.rust-lang.org/std/iter/index.html), [`async_std::stream`](https://docs.rs/async-std/1.7.0/async_std/stream/index.html), and [`futures::stream::Stream`](https://docs.rs/futures/0.3.8/futures/stream/trait.Stream.html). Once this PR lands my plan is to follow this up with PRs to add helper methods such as `stream::repeat` which can be used to document more of the concepts that are currently missing. That will allow us to cover concepts such as "infinite streams" and "laziness" in more depth.

## Feature gate

The feature gate for `Stream` is `stream_trait`. This matches the `#[lang = "future_trait"]` attribute name. The intention is that only the APIs defined in RFC2996 will use this feature gate, with future additions such as `stream::repeat` using their own feature gates. This is so we can ensure a smooth path towards stabilizing the `Stream` trait without needing to stabilize all the APIs in `core::stream` at once. But also don't start expanding the API until _after_ stabilization, as was the case with `std::future`.

__edit:__ the feature gate has been changed to `async_stream` to match the feature gate proposed in the RFC.

## Conclusion

This PR introduces `core::stream::{Stream, Next}` and re-exports it from `std` as `std::stream::{Stream, Next}`. Landing `Stream` in the stdlib has been a mult-year process; and it's incredibly exciting for this to finally happen!

---

r? `````@KodrAus`````
cc/ `````@rust-lang/wg-async-foundations````` `````@rust-lang/libs`````
@kaimast
Copy link

kaimast commented Feb 8, 2021

Is there a plan to include a core::stream::StreamExt (similar to the one in the futures crate) as well?

@yoshuawuyts
Copy link
Member Author

Is there a plan to include a core::stream::StreamExt (similar to the one in the futures crate) as well?

The plan is to add methods directly onto Stream much like methods exist on Iterator, but we want to do so in a way that won't cause ambiguities with ecosystem-defined methods in order to not accidentally break existing codebases when upgrading to newer Rust versions.

@joshtriplett
Copy link
Member

Some discussion on Zulip raised the question of naming. With full acknowledgement to the fact that the name Stream has a long history in the async ecosystem, multiple people observed that something like AsyncIterator or similar might be much more evocative for new users. Such a name would allow people to map their existing understanding of iterators. "I understand Iterator, and I understand async, and this is an async version of Iterator".

@brainstorm
Copy link

brainstorm commented Apr 10, 2021

As a new, inexperienced user, I find AsyncIterator way more foreign than Stream, to be honest... perhaps I'm not too involved in compiler discussions and I don't see how the jargon clicks together though :-S ... also, blogposts are already being written about Stream, so changing the name mid-flight will only breed confusion, I reckon?

@yoshuawuyts
Copy link
Member Author

Prior art on "iterator" and "async iterator" naming schemes in other languages:

@bbros-dev
Copy link

Context: We're prototyping some simple CLI app functionality. We're following the mini-redis example code where we can.

We've bumped out head on streams, and the need for the crates async-streams, and parallel-streams.

In our experience some *Iterator terminology would have helped clarify what streams are.

Given the need for the crates we cited, especially the async-streams RFC, we wonder if there isn't a need for:

  • Iterator
  • ConcurrentIterator
  • ParallelIterator

Or is the intention that async-stream and parallel-stream crate efforts all able to converge into a stream that covers the concurrent and parallel use cases?

@benkay86
Copy link

Rayon provides a whole ecosystem of parallel iterators on top of a work-stealing threadpool, and is currently the de facto Rust standard for parallel iteration. But I don't foresee a parallel iterator trait getting into the Rust standard library anytime soon.

Streams are supposed to cover the use case of concurrent iterators only (per my understanding). Hopefully once streams are stabilized into the Rust standard library we can have some syntax to use them in concurrent for loops just like we currently use synchronous iterators in for loops. However, there are still some issues to work out like what to do if a a stream panics or is dropped. Settling on a name (Stream vs AsyncIterator vs ConcurrentIterator) will be the easy part! 😜

Unfortunately, it's difficult to combine parallel and concurrent iteration at the moment. This would require Rayon to support a way to move tasks from worker threads to an async executor thread, or for an async executor like Tokio to support parallel thread pools in a more sophisticated way than tokio::task::spawn_blocking(). Until that happens, most programmers try to get all their data into memory first on a concurrent executor-driven threadpool and then offload the synchronous computation to a parallel threadpool (e.g. managed by Rayon).

@yoshuawuyts
Copy link
Member Author

multiple people observed that something like AsyncIterator or similar might be much more evocative for new users. Such a name would allow people to map their existing understanding of iterators. "I understand Iterator, and I understand async, and this is an async version of Iterator".

I've filed a PR to the RFCs repo, updating the "streams" terminology to "async iterator" instead: rust-lang/rfcs#3208.

@noelzubin
Copy link

why did .next make it. where can i get more info on this ?

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 17, 2022
Move `{core,std}::stream::Stream` to `{core,std}::async_iter::AsyncIterator`

Following amendments in rust-lang/rfcs#3208.

cc rust-lang#79024
cc `@yoshuawuyts` `@joshtriplett`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 18, 2022
Move `{core,std}::stream::Stream` to `{core,std}::async_iter::AsyncIterator`

Following amendments in rust-lang/rfcs#3208.

cc rust-lang#79024
cc ``@yoshuawuyts`` ``@joshtriplett``
@crlf0710 crlf0710 changed the title Tracking Issue for #![feature(async_stream)] Tracking Issue for #![feature(async_iterator)] Feb 22, 2022
@clarfonthey
Copy link
Contributor

One potential concern re: methods on AsyncIterator, which is less of a concern and more of a justification for requiring them, is methods that use internal versus external iteration.

For example, it can be very efficient to perform a fold on an iterator which is composed of several chains, but it can be much less efficient to directly call next on said iterators.

My concern is that without parity between the methods on Iterator and AsyncIterator, much of these optimisations that already exist for Iterator will be lost in the conversion to AsyncIterator. Since, as it stands, there's not really a way to run a for_each or fold on a regular iterator if the loop contains async operations.

I know for a fact that the current async ecosystem with futures::stream::Stream does not have parity with Iterator, with some notable surprises including the fact that try_fold requires the iterator item to also be composed of results, rather than just the return type of the function.

It would be extremely detrimental to the idea that "this is just the async version of an iterator" if there weren't parity there IMHO, since users might notice slowdowns in code that simply calls std::async_iter::from_iter without adding any extra async code at all.

@yoshuawuyts
Copy link
Member Author

@clarfonthey yes, definitely. The concerns you raise are valid, and the working group is currently actively investigating how we can ensure parity between sync and async Rust APIs. We don't yet (but should) have guidelines on how methods on async traits should be translated from sync to async, but that likely needs us to land async closures / async traits first.

@joshtriplett
Copy link
Member

Given the current trend of the async working group, I wouldn't be surprised if this can become async fn next rather than fn poll_next.

@yoshuawuyts
Copy link
Member Author

@joshtriplett ah yes, that's definitely something we've been discussing within the working group, and we're currently working towards enabling that. The other thing we're currently researching is keyword-generics, which may allow us to merge the separate Iterator and AsyncIterator traits into a single Iterator trait which is generic over "asyncness".

I'll update the tracking issue to reflect both these items.

@withoutboats
Copy link
Contributor

withoutboats commented Feb 21, 2023

(NOT A CONTRIBUTION)

Given the current trend of the async working group, I wouldn't be surprised if this can become async fn next rather than fn poll_next.

ah yes, that's definitely something we've been discussing within the working group, and we're currently working towards enabling that.

I think this is not the right direction for this feature. I hold this view very strongly. AsyncIterator::poll_next enables library authors to write explicit, low-level async iteration primitives for unsafe optimizations. Relying on the compiler generated futures of an async function will make the layout of these types much less predictable or controllable and will take this away from users.

For ease of use where this fine-grained laying control is not desirable, generators are the play (async or otherwise), rather than having to write next methods (async or otherwise) at all.

You need to have the low level APIs (Future::poll, Iterator::next and AsyncIterator::poll_next) for hand-rolled optimized code that the compiler can't be relied on to generate. You need to have the high level syntax (async functions, generators, and async generators) for when people just want to get things done and don't care about these kinds of optimizations. You need both. An async next method would be doing each side of it only halfway (low-level iterator, high-level asynchrony), and would basically trap Rust in a local maxima that looks appealing from where we are now but would not be the best final state.

EDIT: What I mean when I say that "generators are the play" and "local maxima" is that I think because Iterator::next has always existed and has a superficially simple API (ie no Pin, no Context), its not as obvious that for ease of use implementing an iterator with a next method is actually an awful experience for users. Yielding from generators would be much easier. So when you want the ease-of-use story, you want generators, and those can be made async just as easily as functions can. But generators can't guarantee the representation that gets the codegen from for loops over slices looking so good, and similarly won't guarantee the optimizations some async code will want as well. You should be thinking of implementing Iterator::next as really as low-level as implementing Future::poll.


EDIT2: I think the counterargument to this is that mixing-and-matching high-level and low-level is also desirable. IE next + async is desirable when you want control over iteration but don't care about control over asynchrony. Analogously, there must be a hypothetical API that's control over asynchrony but compiler generated iteration - a polling generator(??). I think there could be a case to be made that users do want to be able to drop down into fine control over one aspect of their control flow but not the other, but then that should be an additional, third (and fourth?) option in addition to full control or full ease of use, you can't get rid of the full-control option, which is poll_next.

@dtolnay dtolnay added the I-libs-api-nominated The issue / PR has been nominated for discussion during a libs-api team meeting. label Nov 27, 2023
@madsmtm
Copy link
Contributor

madsmtm commented Dec 1, 2023

For the people that haven't closely followed along on the "blogosphere", I'll link to a few excellent blog posts about it (in chronological order) (authored by people in this thread) (by no means exhaustive):


Another argument in favour of fn poll_next that I've not seen explicitly mentioned: we could still provide a default async fn next impl, to somewhat improve the user experience of manually calling next:

async fn next(mut self: Pin<&mut Self>) -> Option<Self::Item> {
    poll_fn(|cx| self.as_mut().poll_next(cx)).await
}

// Or

async fn next(&mut self) -> Option<Self::Item>
where
    Self: Unpin,
{
    poll_fn(|cx| Pin::new(&mut *self).poll_next(cx)).await
}

Certainly, this is not as clean as the simple async fn next(&mut self), see this playground link for an example of how it might be more verbose, because we're allowing the iterator to be self-referential, but might serve to strike enough of a balance?

Whether Rust then goes with self: Pin<&mut Self> or Self: Unpin mostly depends on what the plans are in the future for making Iterator able to be self-referential, which I think is still an open question.

@Amanieu Amanieu removed the I-libs-api-nominated The issue / PR has been nominated for discussion during a libs-api team meeting. label Dec 5, 2023
@the8472
Copy link
Member

the8472 commented Dec 13, 2023

@withoutboats for your latest blog post, can you give an approximate loop desugaring like in #118847 (comment)
I'm not sure if I'm parsing the ascii charts properly.

Anyway, I'm reraising a concern here that I already mentioned in the PR and that's similar to clarfonthey's:

I think the current proposed interface is terrible for performance when iterating on small types (e.g. u8s) because the poll_next interface returns a Poll<Option<Item>> that conveys two states at the same time, readiness and end-of-iteration.
If the loop body contains no await points, just munching some bytes for example, then that loop body would ideally optimize to a single induction variable based on next's internals. I.e. just branching on "do we still have more data to process".
Only when reaching the end of available data it should poll. This also requires a separation of progress information and getting the next item(s), but it tries to solve a different problem than boat's proposal.

One option is to make poll_next not-async (basically just next) but only return items when the iterator has made progress, which would be polled by a separate method. poll_progress would then return a Poll<bool> I guess to indicate more items / end of iteration.

Another approach is returning an I where I: IntoIterator<IntoIter=IN>, IN: ExactSizeIterator from poll_next. Option fulfills these bounds but an async iter that has an internal buffer can instead choose to return some iterable with more than one item, which allows the loop body to process them on a single induction variable of that iterable without polling.

My async understanding is limited, I'm coming from the sync Iterators side. So I may have misunderstood something.

@jmjoy
Copy link

jmjoy commented Dec 30, 2023

Given the current trend of the async working group, I wouldn't be surprised if this can become async fn next rather than fn poll_next.

After rust 1.75 released, trait async func is stable, async fn next is more user friendly.

@tesaguri
Copy link
Contributor

Given the current trend of the async working group, I wouldn't be surprised if this can become async fn next rather than fn poll_next.

After rust 1.75 released, trait async func is stable, async fn next is more user friendly.

In the rest of the thread, withoutboats has argued that we need async generators for ergonomics, rather than async fn next(), which is less fine-grained than fn poll_next() and less ergonomic than async generators in their opinion. Then, what is your rationale for promoting async fn next() over async generators?

@RalfJung
Copy link
Member

Looking at this through the lens of algebraic effects, I would say that

  • Future corresponds to a (suspended ongoing invocation of a) function that can trigger the "pending" effect. The effect has argument and return type () (and eventually the function returns Future::Output).
  • Iterator corresponds to a (suspended ongoing invocation of a) function that can trigger the "yield" effect. The effect has argument type Iterator::Item and return type () (and eventually the function returns ()).

This analogy is not quite perfect: futures can be polled again after returning "ready" (and similar for iterators); futures have this "context" argument; iterators are not pinned. But I would argue all of those are concessions to how we model these kinds of computations in Rust -- the abstract concept we want to model doesn't have them, but the imperfect realization of that concept in Rust does have them.

An AsyncIterator would then be a (suspended ongoing invocation of a) function that can trigger both the "pending" and the "yield" effect, with the same types as above (and eventually the function returns ()).

We don't have algebraic effects in Rust, but futures have shown how we can model one particular algebraic effect. If we follow that same paradigm, then the fn poll_next-based encoding seems to be the most direct way to represent an AyncIterator.

In contrast, the async fn next-based encoding seems to model something different: it represents a function that can trigger a "yield" effect, where the argument type of this effect (i.e., the data being passed from the function to the handler) is a function that can trigger a "pending" effect. If we view an async iterator as triggering a sequence of yield and pending, then this does seem equal in abstract expressivity to fn poll_next (split up the sequence after each yield, to obtain a sequence of subsequences; then each of these subsequences corresponds to one of the functions that is being yielded). However, it is a much less direct encoding of what actually happens, involving a seemingly unnecessary "thunk" (the functions being yielded). Sure, in trivial cases this can be optimized away, but I wouldn't bet much on the claim that such optimizations will always work. It certainly does not seem to fit the usual Rust philosophy of avoiding unnecessary overhead in the most basic abstractions.

So I guess what I am saying is, I tend to agree with boats. Mind you, I'm not an expert in async, I am taking a 10,000 foot view of this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests