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

Lessons learned from async listener #28

Open
domenic opened this issue Jan 8, 2015 · 10 comments
Open

Lessons learned from async listener #28

domenic opened this issue Jan 8, 2015 · 10 comments

Comments

@domenic
Copy link

domenic commented Jan 8, 2015

In my opinion, the lack of "async listener"-esque functionality is a bug in the JavaScript language. There should be something built in to the language, and the engine, that allows you to intercept any entries or exits from the event loop. Then, on top of that, you could build domains, async try/catch, long stack traces, etc.

Currently in browsers people do this by patching every async API. Node's async listener was, if I understand it, an attempt at a more principled approach. However, it was removed.

I'd love to understand the "lessons learned" from the work on async listener so far. What worked? What didn't? What were the problems? Was there a fatal flaw that prevented the API from shipping, or was it just an overall feeling that it wasn't quite right, or...? Hopefully we can take this information and work out something that could eventually be baked into the JS language itself, across all environments. Informed, of course, by careful prototypes by us people :).

/cc @trevnorris @piscisaureus @othiym23 @creationix @btford plus lots more I am presumably forgetting.

@ijroth
Copy link

ijroth commented Jan 8, 2015

@Qard
Copy link
Member

Qard commented Jan 9, 2015

This might be a useful read: https://gist.github.com/groundwater/942dad5c0c4cfae21af9

That is from a Tracing Summit meeting from several months ago. The general consensus seemed to be that AsyncListener was awkward and unreliable for our use case.

I'll gladly help in any way I can with defining an AsyncListener-like thing for JS. :)

@othiym23
Copy link

othiym23 commented Jan 9, 2015

The API was unwieldy and came in at an awkward layer of abstraction. It was perfect for continuation-local storage's purposes, kinda, but at (what we presumed was) the end of the 0.12 release cycle, it felt like the design of the API needed some more time to bake. The intention was always to land a more fully-baked tracing API before Node hit 1.0, and the notes that @groundwater pulled together are meant as a jumping-off point for future work. It's not a dead letter!

@domenic I think all of us who were at the meeting that @Qard linked to were in agreement that better access to instrumentation and operational reflection from the Node runtime would be hugely useful, but I'm not sure how well that meshes the concerns I see within TC39 regarding security and preventing inadvertent information disclosure. asyncListener presumes a trusted environment, and a lot of its power comes from it having visibility into parts of the execution cycle that are normally obscured by design in JS. But I would love to see better instrumentation available in JS, so I'll let TC39 types decide what is and isn't appropriate wrt those criteria.

@creationix
Copy link

I always wanted a simple API for node, long before we had domains or async-listner. The core idea is:

  • a hook for when a new potential callback is added to the event loop (for example, during setTimeout). This hook would need to run some arbitrary code and return a context value if it wanted to track this event. Some extra information like event type and possibly libuv handle or timer object or something would be nice to have.
function onSetup(type, handle) {
  // Here I can do things like look at current globals to know what domain I'm in or what http request I'm tracing, etc.
  return {};
}
  • a frame inserted at the top of every event source. This will give me the context I created earlier and a function to dispatch the actual event.
function onEvent(context, event) {
  // Do things before the event, like setting globals or tracing stuff
  event();
  // Do things after as well
}

This simple, but powerful API can do all sorts of crazy things. It could implement domains using try..catch..finally in the onEvent handler. It could silence certain types of events entirely, it could give http code globals like req and res and swap them out on every event source.

The main concern I've heard with this approach is it would kill performance. But after seeing the issues with the other approaches, I'm not convinced performance is so killer in such a simple API.

I had this interface in luvit 1.0 and it was still considerably faster than node.js. But that may be because luajit has much lower overhead switching between lua and C compared to V8 switching between JS and C++.

@creationix
Copy link

Btw, when I worked on async-listener, I quickly realized that backwards compat with the original domains was quite complex and treated event listeners as an async source when they were a purely sync action.

As others have noted, the real complexities are when you have multi emitters like intervals, tcp servers, and low-level streams. The libuv team has ideas to unify this all into single event type APIs (more like requestAnimationFrame and less like setInterval). If all event sources APIs were converted to this style, even the connection pooling problem would be less of an issue (though still needs app-level intervention for multiplexed responses over a single channel).

@domenic
Copy link
Author

domenic commented Jan 9, 2015

One thing that seems clear to me is that we want to focus on async entry points, and not special-case EventEmitter at all. If code enters the event loop by firing an event, that hits some hook, just like if code enters the event loop by calling a callback. (In both cases, "C++ is calling JS.") But if JS code fires an event itself, that doesn't hit the hook.

@bjouhier
Copy link

bjouhier commented Jan 9, 2015

There are several drivers behind these hooks. I see two that really justify the hooks:

  • TLS (Thead Local Storage) equivalent.
  • Instrumentation, for example to get histograms of tick times.

But we don't need additional APIs for exception handling or long stack traces. These issues will be solved by the async/await + promises combination in ES7 and if people want them today, they can use one of the libraries/tools that provide async/await emulation. The good ones support structured EH with long stack traces.

Ideally, I think that Local Storage should also be handed at the language level rather than through special APIs. This is important for interop between libraries. For example, it would be good to have some kind of TLS locale as part of the Intl library.

Otherwise, +1 on @domenic's comment that the focus should be on async entry points rather than on event emitters. BTW, this is what I have put in streamline.js: a pair of yield/resume events that fire when code enters/exits the event loop, with a context that is preserved across await and accessible from these events. If you can capture the stack (long one of course) at the yielding point (the await call that caused the yield), then you have a very powerful instrumentation hook, and for example you can capture flamegraphs of async calls (see https://github.com/Sage/streamline-flamegraph).

@trevnorris
Copy link

I'll reply to this in more detail later, but want to make sure everyone
understands that the hooks async listener used are still there. Technically
async listener could ship as a module, which was the point. Didn't want to
have another domains issue where we find out after the fact the API has
issues. So I wanted the community to write their own implementations and
come to a consensus of what the best API would be.

@piscisaureus
Copy link
Member

I'll write up my experience with it too; right now I'm busy with the io.js release but that should be over next week.

@kraman
Copy link

kraman commented Jan 10, 2015

I am working on an API that would meet the goals of async-listener as well as other APIs that require tracking context across the C boundary. You can take a look here https://github.com/strongloop/async-tracker.

Once implemented this API will provide events for core functions which should satisfy @domenic 's use case. It would also let non-core node programmers instrument their own code if they would like additional events tracked. It also handles one-show callbacks as well as repeat-callbacks in the same way.

I haven't thought about how the async/await style would affect this as yet and the implementation is still very inefficient but I am interested in learning if the API satisfies what is expect from async-listener like APIs.

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

No branches or pull requests

9 participants