Support Symbol.asyncIterator

[benjamingr]

Async iterators are a stage 3 proposal for asynchronous pull streams (link) with a v8 implementation arriving very soon.

A new Symbol.asyncIterator is introduced. I believe stream.Readable should support Symbol.asyncIteator which would allow it to be iterated with for...await.

Supporting Symbol.asyncIterator means that readable streams are iterable through the new for await loops. ref.

Meaning we'll be able to do

let body = '';
for await(const data of req) body += data;
const parsed = JSON.parse(body); // note that we can let errors propagate, since we're in an async fn
console.log("got", parsed);

Semantics

The async iterator protocol looks like:

interface AsyncIterator {
    next(value) : Promise<IteratorResult>
    [optional] throw(value) : Promise<IteratorResult>
    [optional] return(value) : Promise<IteratorResult>
}
interface IteratorResult {
    value : any
    done : bool
}

Which is similar to the regular JavaScript iteration protocol except that it returns promises. Since it is a pull stream supporting back pressure is pretty simple (since the consumer asks for values rather than being pushed values).

You .next the async iterator to get a promise for the next value in the iteration. You also can build async iterators with async generators. Here are some examples for both from a talk I gave: https://docs.google.com/presentation/d/1r2V1sLG8JSSk8txiLh4wfTkom-BoOsk52FgPBy8o3RM

Implementation

We'll add a Symbol.asyncIterable to readable streams that returns an object with next throw and return methods.

• next is called by the for await loop to get the next value, it expects a promise to be returned that resolves with a done/value pair. This should likely
• return is called when a user abruptly completes the loop by calling break. This is our chance to do cleanup - like what you'd use finally for and it marks completion (like done: true).
• throw is on the consumer's side to propagate errors to the async iterator. I'm not sure what we should do here.

With regards to backpressure - the "easiest" thing would be to pause the stream whenever we just gave a value and unpause it on next - I'm concerend if that might affect performance even further.

Note that while there are interesting problems to solve with async iterators in general - I think our initial focus should be users of the new for await loop.

Performance

Allocating a promise for each value in next sounds prohibitively expensive. However, I am optimistic this will be solved in V8 land and the promise resolution will be inlined.

I recommend we start with a simple implementation and work our way towards a more optimized one.

[benjamingr]

I'd also like to point out I'm far from being a NodeJS streams expert :)

[caitp]

However, I am optimistic this will be solved in V8 land and the promise resolution will be inlined.

Note that this idea isn't guaranteed to work :) it could generate fatter code that simply isn't worthwhile.

Best case, you'd still have a property load for then, and allocation of two different microtask entries, depending on which path is taken, which might not be worth it to elide the Promise allocation.

It's something to try in 2017 though.

[mcollina]

Before adding this to core, I think we should experiment with this in userland, even through the nodejs org. It's simple to polyfill, and I can think 2-3 different ways to implement it.

I recommend we experiment with the idea, and come up with a benchmark. This will help everyone assess the situation, before adding something to core that we will have to support forever.

I'm happy to produce that impl, if someone can help set it up the whole transpiling chain to try it.

[calvinmetcalf]

I think I may have written an implementation of this lemme check

[calvinmetcalf]

false alarm, I have not, yet

[calvinmetcalf]

though more generally I'd vote for waiting and seeing what whatwg streams do re async iterators before we do anything

[mcollina]

I generally agree. However, we should check if that API works for us before its finalized. After which, we do not have much chance to change things.

[calvinmetcalf]

• next this is strait forward
• return, should probably cover similar stuff as the destroy proposal
• throw, just throws an error in the stream?

[benjamingr]

@calvinmetcalf sounds good to me!

I think we should implement now (maybe behind a flag) and see how it works for us.

[calvinmetcalf]

more likely make a separate package to add it for now

…

On Fri, Jan 6, 2017 at 11:37 AM Benjamin Gruenbaum ***@***.***> wrote: @calvinmetcalf <https://github.com/calvinmetcalf> sounds good to me! I think we should implement now (maybe behind a flag) and see how it works for us. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#254 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABE4n5JFFqbOfnBmGTQ7NIt28qmNg3L9ks5rPm3ngaJpZM4LYlAl> .

[mcollina]

@calvinmetcalf yes. We can prototype it there with transpilers, and see how things are going. Then, import it here or within core.

[calvinmetcalf]

prototype version, also this issue was originally nodejs/CTC#53

[benjamingr]

@calvinmetcalf can you upload to NPM? I want to make sure I play with it and have feedback before the meeting.

[mcollina]

I've opened up a couple of issues we have to discuss.

[calvinmetcalf]

@benjamingr https://www.npmjs.com/package/async-iter-stream

[mcollina]

Closing for now, feel free to reopen @benjamingr if there is something more for us to do!

[benjamingr]

I'm pretty content with the resolution and async-iter-stream is satisfactory in the meantime and I'm enjoying it.

I think we can close this until async iterators land in Node at which point we'll probably want to revisit this and add support to core :)

Thanks!

[vsemozhetbyt]

A use case that will gain from iterable streams:
puppeteer/puppeteer#412

[addaleax]

Fwiw this is available behind a harmony flag in Node 7+, so if we want to, we can start implementing things in core.

[benjamingr]

See also discussion in nodejs/promises#31 regarding optimizing async iterators.

I think we should start implementing - but can't expect it to be fast any time soon.

[mcollina]

When will async-iterators be outside of the harmony flag?

We have a working prototype https://github.com/calvinmetcalf/async-iter-stream, can you check if it works with --harmony? I think it can help driving the discussion on how it can be made fast. However, I think it might be a bit too early for adding it into core IMHO.

[targos]

it was staged in V8 just a few days ago. That means the implementation is complete, which is great, but it can stay behind a flag for some time.

/cc @caitp Is there an intent to ship this soon?

[targos]

@mcollina async-iter-stream's test suite passes with both v8.4.0 and canary with --harmony-async-iteration (I executed test.js without babel-register)

[mcollina]

@targos this is amazing news.

[caitp]

/cc @caitp Is there an intent to ship this soon?

I've been asked to send one out recently.

I think we should start implementing - but can't expect it to be fast any time soon.

This would give us some useful data for profiling, so the sooner the better :)

[benjamingr]

When we ship, I don't mind writing a benchmark for readable streams as async iterators (vs callbacks with event emitters) in a real world app example (opening several streams and reading from there to fulfill a user request).

[calvinmetcalf]

@benjamingr i'd be interested to see that, though they are going to have different perf characteristics because async-iterators support back pressure, though any help making async-iter-stream better would be very appreciated, I've got a few issues created in that repo

[vsemozhetbyt]

Refs: https://medium.com/netscape/async-iterators-these-promises-are-killing-my-performance-4767df03d85b

[mcollina]

@vsemozhetbyt I have just verified that https://github.com/mafintosh/csv-parser is as fast as the python alternative.

[vsemozhetbyt]

Discussion in the proposal repository.