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

Async/Await implementation in V8 #4

Closed
benjamingr opened this issue Feb 15, 2016 · 69 comments
Closed

Async/Await implementation in V8 #4

benjamingr opened this issue Feb 15, 2016 · 69 comments

Comments

@benjamingr
Copy link
Member

Async await is coming our way with v8 slated to implement it in Q2 2016.

Node needs to be prepared :)

Possible areas of impact:

  • V8's implementation needs to be tracked so we make sure we have our right hooks when it lands. Things like unhandledRejection might possibly be impacted as well as the way promises are currently queued and run in Node.
  • We need to consider the whole operational vs. programmer error issue here.
  • Core dumps should work properly in async methods.

Probably many others, just starting the discussion.

@chrisdickinson
Copy link
Contributor

This issue is a bit open-ended. Would you mind scoping it in, or closing it if it can't be scoped in?

@petkaantonov
Copy link
Contributor

Core dumps should work properly in async methods.

This is discussed in #8 , (point 2. in OP)

@benjamingr
Copy link
Member Author

Basically - to track https://bugs.chromium.org/p/v8/issues/detail?id=4483 and make sure we're keeping up with the implementation from the node side and until then to discuss desirable behavior we'd like async functions to have in Node. Namely that none of the hooks (unhandledRejection, rejectionHandled) break.

This is non-blocking for the promises-in-core PR just so we're clear. I'll scope it in.

@benjamingr benjamingr changed the title Async/Await Async/Await implementation in V8 Feb 15, 2016
@benjamingr
Copy link
Member Author

I changed the name is this better @chrisdickinson ?

@chrisdickinson
Copy link
Contributor

Excellent, thank you!

@littledan
Copy link

To clarify, V8 intends to implement async/await but we don't have any particular schedule announced. async/await is a lot like using Promises directly. I'm not sure if anything special is needed for Node's needs here. But if there are other requirements, I'd really like to hear about them. About the points here:

  • Unhandled rejections will be handled, according to the spec, just like Promise unhandled rejections, and invoke the handlers as appropriate.
  • For operational/programmer errors, I like the idea of a convention to either return some kind of error code or passing an error callback as opposed to using Promise rejection within Node. Promise rejection is equivalent to throwing an exception, and async/await makes this very clear, so if exceptions are to be avoided in Node, then error callbacks or codes would work better.
  • For core dumps, seems like the important thing is that the unhandled rejection callback can do a core dump. Does anyone know if there are any issues with these callbacks and core dumps at the moment? I don't see async/await changing a lot about them here vs how things are today with Promises.

@petkaantonov
Copy link
Contributor

@littledan

Errors with promise users are always through one channel even in node and any exceptions node tries to throw for promise users can be caught or listened with domains and turned into rejections. This way the consumer decides whether an error is operational/programmer error because the producer of the error simply cannot know this. Adding additional error channel doesn't add value to this as it is the producer who will choose the channel at error production time and therefore the consumer would be forced to duplicate code for the different channels. There are plenty of examples how the same error can be programmer error and operational error depending on what the consumer is doing.

Glossary:

  • producer means producer of the error
  • consumer means consumer of the error

@chrisdickinson
Copy link
Contributor

@littledan There are two problems with core dumps and promises at present:

  1. Unhandled rejection at the end of a chain — that is, rejectAPromise().then(A).then(B).then(C) will catch and rethrow the error three times, and the abort will happen at C's PromiseIdRejectHandler, instead at the source of the rejection in rejectAPromise as desired. @misterdjules has some work put towards this — primarily around running handlers and executors without try / catch if a --abort-on-uncaught-exception flag is set and PromiseHasUserDefinedRejectHandler is false.
  2. (This is primarily on the Node end, but!) Right now we wait until the end of the microtask queue to fire the unhandledRejection listener, so that we can check that the promises rejected during microtask execution have not been synchronously handled. This means that we have to unwind the stack, which produces less useful cores. We may be able to switch our behavior using flags here, but it means all synchronous rejections will immediately abort the program (i.e., we may have to patch Promise.reject to return a pending promise that rejects on next tick instead of immediately.)

@petkaantonov
Copy link
Contributor

The stack can be saved and restored later like. Chrome has to do something like this for the async debugger to work. Theoretically the mutable objects that a stack points to might have been destructively changed but there is a high chance this isn't a practical concern.

@benjamingr
Copy link
Member Author

@littledan like @petkaantonov and @chrisdickinson said - any help with porting the async stack traces like functionality to node that would allow us to save and restore the stack for inspection would be greatly appreciated.

Clarification: The feature in Chrome is called async stack traces but it actually saves the actual stack and not just the trace.

@benjamingr
Copy link
Member Author

@CrabDude Awesome. That would be great!

I think the first step is to figure out and map how Chrome's async stack traces feature works - how we can keep track of the stack, this value and arguments and how things work.

This post contains the PRs node-inspector/node-inspector#340

The challenging part isn't just getting the trace - it's getting the stack frame - in the devtools I can access the closure variables, this value and more. If we can produce and then consume similar output that would be awesome for debugging.

Here is an HTML5Rocks article about how to use the feature in case you are not already familiar http://www.html5rocks.com/en/tutorials/developertools/async-call-stack/

@benjamingr
Copy link
Member Author

@CrabDude any progress? Anything you need help with?

@littledan
Copy link

@chrisdickinson Good to know about these technical issues! Seems like some of them might be solved on the V8 side. Let me know if you or anyone else (@CrabDude @benjamingr) wants to get involved on the V8 side, and I can provide technical help and explain the technical context, and we can discuss designs. Another issue with async stack tracking is that it has significant overhead and keeps more things alive with respect to GC, so that might need to be addressed if you want to leave it always on.

@pekaantonov: no disagreement on the basic points, except there are some arguments I've heard in favor of using another channel, e.g. better integration with the core dump tool and that conditional catching in Node is unergonomic. I just wanted to mainly emphasize that rejection === exceptions, and async/await doesn't change this relationship.

@ghost
Copy link

ghost commented May 3, 2016

@benjamingr are you sure async/await v8 support is going to land in Q2? From where should we know that? What is the estimated schedule to shipping for node.js?

@PlasmaPower
Copy link

Async/await support just landed in v8! https://chromium.googlesource.com/v8/v8.git/+/d08c0304c5779223d6c468373af4815ec3ccdb84

Still a couple of bugs being worked out, but the foundation is there. Hopefully we can get this in Node soon.

@benjamingr
Copy link
Member Author

@hasyee yes 😅

@caitp
Copy link

caitp commented May 25, 2016

Still a couple of bugs being worked out, but the foundation is there. Hopefully we can get this in Node soon.

If you know about bugs that we don't, please file them (or at least @-spam me on github or Twitter), because I'm actually not aware of much else that is left (other than my wish for putting async callstacks in V8 itself, rather than in Blink, but that may not necessarily happen)

@PlasmaPower
Copy link

@caitp Nope! I just wrote that looking at patches to resolve various small issues.

@naixia
Copy link

naixia commented Jun 1, 2016

@benjamingr When will we get this feature in node.js ? 😄

@chrisveness
Copy link

chrisveness commented Jun 1, 2016

Async/await should arrive – behind an 'experimental' flag – in Chrome 53, which is scheduled to be released into stable channel on 6 September, so will hopefully arrive (behind a flag) in Node v7, in October.

@littledan
Copy link

To clarify, async/await is not yet finished. I'm working on fixing bugs and writing tests. If 53 branched today, we would not turn on the async/await flag on. I would not recommend using experimental features in production--they are often behind flags because they are not working 100% correctly.

@benjamingr
Copy link
Member Author

@littledan I definitely see value in shipping it behind a flag in Node as soon as possible so people can test it.

@DJviolin
Copy link

DJviolin commented Oct 8, 2016

Thank You! Sorry, I missed this one.

@Okoyl
Copy link

Okoyl commented Oct 12, 2016

any chance of this feature being backported to nodejs 6.x.x?

@benjamingr
Copy link
Member Author

@Okoyl probably not since it requires an updated version of v8 and would break native modules which is part of why we need a major version number anyway.

You can use a transpiler though

@leodutra
Copy link

Maybe Node should wait for modules in V8 for a v7.0?
Chromium guys are over modules right now

@unilynx
Copy link

unilynx commented Oct 17, 2016

@leodutra that would pretty much eliminate the need for babel for a lot of use cases, at least for me. The other missing features I can live with having to work around, but not import & await..

We can dream, can't we ?

@kyrylkov
Copy link

@unilynx I'd say the need for Babel will stay, until native V8 performance of Promises and consequently async / await improves.

@littledan
Copy link

I'm glad to hear so much excitement about async/await. However, Node v7 is based on V8 54, whose async/await implementation has some bugs in it (including a nasty memory leak). I would not recommend using --harmony-async-await in production until upgrading to V8 55; the easiest way to achieve that would be to wait for Node 8. A backport of everything to Node v6 would be non-trivial; there would be a lot of merge conflicts. Until Node 8 comes out, I'd recommend using a transpiler.

@billinghamj
Copy link

So why aren't we waiting for V8 55? How long is it expected to be until release?

@ilkkao
Copy link

ilkkao commented Oct 17, 2016

Node releases are time boxed, but there is a possibility that V8 gets upgraded in a minor release (e.g. 7.1 or 7.2) when V8 55 is stable. This has happened once before.

@ghost
Copy link

ghost commented Oct 18, 2016

Looks like V8 55 stable release is planned to be before Dec. 6th: https://www.chromium.org/developers/calendar

@ofrobots
Copy link

One key component that would be needed for async functions to be properly debuggable/traceable for Node.js would be the promise/microtask hooks currently being worked on (see issue and design).

While a mid-stream, ABI-neutral upgrade to V8 5.5 is theoretically possible on the Node.js v7.x branch, I personally think that we should hold off until promise hook support is available in V8 (i.e. until Node.js v8.x).

@pierophp
Copy link

V8 55 released:
http://v8project.blogspot.com.br/2016/10/v8-release-55.html

@ofrobots
Copy link

That is not a release announcement. 'V8 5.5 has branched and this is what
it contains'
On Tue, Oct 25, 2016 at 5:28 AM Piero notifications@github.com wrote:

V8 55 released:
http://v8project.blogspot.com.br/2016/10/v8-release-55.html


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#4 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAE0qbLdoPmHglDCL4jyP2wL1MB0qyGVks5q3fXTgaJpZM4Hahze
.

@chrisveness
Copy link

... "will be in beta until it is released in coordination with Chrome 55 Stable in several weeks"

@rvagg
Copy link
Member

rvagg commented Oct 30, 2016

While a mid-stream, ABI-neutral upgrade to V8 5.5 is theoretically possible on the Node.js v7.x branch, I personally think that we should hold off until promise hook support is available in V8 (i.e. until Node.js v8.x).

Can you clarify this a little more @ofrobots? I've been working under the assumption that a 5.5 mid-current wouldn't be a big deal, are you thinking that our lack of ability to interact properly with Promise internals should hold that back because of how widely async/await will be adopted?

@leodutra
Copy link

leodutra commented Oct 30, 2016

I have another worry about this. I can't stop thinking how nice would be
each promisse running in a libuv thread. I don't know if it is implemented
this way today, but if the cost of these threads are cheap... Maybe using
stream internally to run the hidden promise resolutions could make node
even more powerful.

It's the point of view of a passing guy, MHO.

@ljharb
Copy link
Member

ljharb commented Oct 30, 2016

@leodutra it would have to be unobservable to the underlying JS to be spec-compliant - JS is single-threaded.

@leodutra
Copy link

leodutra commented Oct 30, 2016 via email

@ljharb
Copy link
Member

ljharb commented Oct 30, 2016

@leodutra that's certainly one opinion. There's quite a number of people, like myself, who consider threading a legacy programming model. None the less, this is the way JS is and will stay.

@ghost
Copy link

ghost commented Nov 1, 2016

I've been using Node 7 since the beta with --harmony-async-await. Maybe there are advanced debugging use cases that I am not exercising, but debugging support in Chrome DevTools is good. I haven't had any issue debugging code that uses async/await. It's worked surprisingly well.

From my perspective as someone updating a module to use async/await, it would be nice to have v8 55 make it into Node 7. Waiting until Node 8 could mean 3 extra months delaying a beta release of the software (assuming 55 goes stable Dec. 6th).

@nickserv
Copy link

nickserv commented Nov 1, 2016

Also, isn't the async/await implementation still using Promises or something similar internally? If so, Promises have already been stable in V8/Node, so I see less risk in pushing prereleases with async/await support.

@adnan-kamili
Copy link

adnan-kamili commented Dec 2, 2016

Chrome 55 stable is out, @littledan are there any API breaking changes, if no can we expect it to part of next Node.js 7.x release and get rid of harmony flag for async await

@billinghamj
Copy link

@adnan-kamili The breaking changes would be at the native API level, and there almost certainly are some. Unfortunately this is unlikely before Node v8 :(

@ofrobots
Copy link

ofrobots commented Dec 5, 2016

You can track the status of the effort to integrate V8 5.5 here: nodejs/node#9618

@billinghamj
Copy link

nodejs/node#9618 (comment) - so should be coming to v7 :)

@AMorgaut
Copy link

@nickmccurdy async functions are also based internally on generators, so they also need them to be stable, not just the Promises. But I think that's the case ;-)

@adnan-kamili
Copy link

Issue tracking the backport of v8 5.5 into 7.x branch

nodejs/node#11029

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

No branches or pull requests