1

Amend RFC 2996 to replace `Stream` with `AsyncIterator` by yoshuawuyts · Pull R...

 2 years ago
source link: https://github.com/rust-lang/rfcs/pull/3208
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.

Conversation

Copy link

Member

yoshuawuyts commented on Dec 14, 2021

edited by scottmcm

RENDERED
Tracking Issue

This PR updates RFC 2996: Async Stream to use "async iterator" terminology instead. This was brought up on the async_stream tracking issue by @joshtriplett, capturing a conversation that happened on Zulip. The proposal has since seen quite a bit of support both from the ecosystem and async foundations working group.

Additionally there is also strong precedence in other languages for using an "iterator"/"async iterator" naming scheme:

Hence I'd like to propose this amendment to RFC 2996: Async Stream to use "async iterator" terminology instead. Thanks!


cc/ @rust-lang/libs-api, @rust-lang/wg-async-foundations @nellshamrell

This seems to reflect the API shift we've been talking about for a while and have been assuming would happen. Thanks for writing this up! I'm going to start an FCP to confirm consensus on this renaming.

@rfcbot merge

Copy link

rfcbot commented on Dec 14, 2021

edited by m-ou-se

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

Copy link

Member

scottmcm commented on Dec 14, 2021

This does make sense to me; when I hear "stream" I think something different.

That said, should this add a section to the Rationale & Alternatives saying why this name was chosen?

bellThis is now entering its final comment period, as per the review above. bell

Copy link

Member

Author

yoshuawuyts commented on Dec 14, 2021

That said, should this add a section to the Rationale & Alternatives saying why this name was chosen?

That's a good point. I think we should. Would it be okay if I file a follow-up PR so we can discuss the exact wording there without holding up the FCP in this PR?

Copy link

Member

scottmcm commented on Dec 14, 2021

I think it's perfectly fine to add more "why" details during the FCP; they're not like bors where a new commit requires a new approval. (Changing the design can sometimes need a new FCP, but if it's just recording rationale and alternatives that doesn't invalidate checkboxes IMHO.)

So since it'll take 10 days for the FCP to complete anyway, I'd suggest just updating this PR. But later is totally fine too.

(And I'm not on libs-api, so nothing I say here is authoritative.)

Copy link

Member

Author

yoshuawuyts commented on Dec 14, 2021

So since it'll take 10 days for the FCP to complete anyway, I'd suggest just updating this PR. But later is totally fine too.

Ah, okay - I'll do that then!

This PR seems like a good change to me. While I like "Streams" better as a name (it's shorter and sounds better to my ears), it also conveys to me very little about what Streams actually are. I think AsyncIterator is much clearer in that it immediately gives some idea what it refers to, so it makes sense to prioritize clarity in my opinion.

Copy link

Contributor

@tmandry tmandry left a comment

Thanks for putting this up, @yoshuawuyts.

This reflects the general approach we've been talking about for awhile to stick to AsyncFoo naming for any trait Foo which has a direct synchronous analogue in the standard library. While Stream is definitely nicer to type than AsyncIterator, I agree with @eholk that having clarity is important, and I think internal consistency and predictability are important too.

Copy link

Contributor

nikomatsakis commented 28 days ago

Big fan.

Copy link

Contributor

workingjubilee commented 26 days ago

...Does this mean to say the iterator's elements are iterated over in a non-serial "out of order" fashion, so we are no longer required to iterate in a "eager" or "sync" fashion in order to execute an item, or that the iterator is iterated over serially, but the items returned are Futures?

Copy link

Member

Author

yoshuawuyts commented 26 days ago

@workingjubilee the iterator is iterated over serially, but the items returned are Futures. From the summary section of the RFC:

Async iterators are a core async abstraction. These behave similarly to Iterator, but rather than blocking between each item yield, it allows other tasks to run while it waits.

Copy link

Member

Nemo157 commented 26 days ago

...Does this mean to say the iterator's elements are iterated over in a non-serial "out of order" fashion, so we are no longer required to iterate in a "eager" or "sync" fashion in order to execute an item, or that the iterator is iterated over serially, but the items returned are Futures?

Those actually describe the same thing to my understanding, Iterator<Item = impl Future>, this would allow you to iterate over the items and wait for/compute them out-of-order since the futures are decoupled. AsyncIterator is different in that it serially but asynchronously waits for/computes the next item, you cannot skip over some items to a future item without waiting for those items to be computed and discarded (and there is another step in between this and the out-of-order version, StreamingIterator<Item<'a> = impl Future + 'a>, this allows you to skip items without waiting, but not resolve them out-of-order).

Copy link

Contributor

tmandry commented 25 days ago

You must consume the items of an AsyncIterator serially (just like with Iterator); the Async only controls whether those items are produced synchronously or not. Similarly, the items can be produced out of order (with respect to some reference order), see e.g. FuturesUnordered which yields task results as they complete.

To restate what @Nemo157 said (partly for my own understanding): It's true that in both Iterator and AsyncIterator the item could be a Future which would allow you to poll them independently. Alternatively, you might want the hypothetical Streaming versions of each trait which would allow the yielded values to borrow from the iterator (or data it borrows from), but would prevent you from holding onto previously yielded items after you call next again.

I feel like there should be a table for all this :)

Copy link

Contributor

workingjubilee commented 24 days ago

Yes, I think the "other interpretation" I had in mind was either StreamingIterator or FuturesUnordered, where there's even less rules on what is happening when, it's just everybody hitting the dance floor (but notionally, at least in my mind, there was still a "sequence that would be valid to iterate over").

StreamingIterator well and truly is a mouthful and also can break some intuitions about what "iteration" means, so I think I support this name change, if nothing else, to reserve Stream as a namespace for that.

Copy link

rfcbot commented 24 days ago

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

joshtriplett

merged commit c76963d into

rust-lang:master 24 days ago

yoshuawuyts

deleted the async-iterator branch

17 days ago

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK