You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I've added a number of folks for editing on the agenda doc, and on the newly created Hangouts On Air event. Let me know if you didn't see notice of this, and if the various links in the description don't work for you.
clearTimeout(setInterval()) never notify asyncWrap. Will particularly be an issue when setTimeout "is fixed".
I belive net.connect() has a similar issue, the before and after event is fired multiply times, but you don’t known when the handle is no longer in use (connection closed).
(I need to revalidate these, but I'm quite sure they still hold)
Guessing that @trevnorris has a fairly definitive list of async_wrap issues, but he unfortunately won't be able to make the call.
I don't know how specific the meeting should be and some of these have already been discussed. So I will let someone wiser decide.
I'm not wiser, but have some comments. :-)
At this point it looks like the "tracing" story has a few different angles - async_wrap, v8-tracing.h, platform specific tracing w/ETW/etc.
Seems like v8-tracing.h / platform-specific tracing are kinda at the same level, and async_wrap is kinda different. I guess I see it as async_wrap will be a great place to put v8-tracing.h / platform-specific trace points. Does that seem right?
If so, it would be good to come up with different "names" for these things, so we can keep straight what we're talking about, for folks coming to this as n00bs, like me.
In any case, w/r/t async_wrap, using it as an exemplar of using v8/platform tracing seems ideal. We clearly want tracing hooks there. So let's talk about it some.
Would also like to get an update on v8-tracing, from @ofrobots . @billti also provided some info on work to add ETW support for tracing to node, in #10, ICYMI.
Want to make sure everyone's on the same page about AsyncWrap. It is a class where I centralized the creation of all class constructors that have the possibility of performing I/O. The current API exposed via process.binding('async_wrap') was only created as a way to utilize the new class structure. That API could be removed easily without affecting the functionality of the AsyncWrap class.
It requires monkey patching to catch errors without changing the throw origin
All error handling abilities were removed in the PR because of conflicts with the domain module and uncaughtException. Took me about a month to get it right, but was then considered to big of an alteration to merge at the time.
nextTick requires money patching
setTimeout, setImmediate, setInterval: shares the handle which results in an unexpected context
My original PR addressed these cases, but unfortunately they conflicted with domains and were eventually removed because I wanted to get the native code changes merged. This can be done, but requires making calls similar to how domains work today.
A handle ondestructed event would be nice
This callback could not guarantee being called while the object was being destructed. Which means you would no longer have access to the object. I've toyed with an implementation in the past, but never reached a point of being useful.
clearTimeout(setInterval()) never notify asyncWrap. Will particularly be an issue when setTimeout "is fixed".
This is because for performance there is a single TimerWrap instance with a JS linked list object attached that contains all the timers that will be fired in the future. So when a timer is cleared it's simply removed from the linked list.
I belive net.connect() has a similar issue, the before and after event is fired multiply times
Probably because they are also creating a ShutdownWrap (src/stream_wrap.h). All *Wrap have a unique id, which could allow filtering of these calls.
Also note there are cases like net.createServer() that will not trigger AsyncWrap until .listen() is called. This is because we don't know whether the user wants a Pipe or TCP until that time. It was a serious pain to work around when I was writing the initial error handling implementation.
I haven't done much with AsyncWrap for a while because this is the type of conversation I wanted to have before proceeding. When should events be fires, in what order, etc. are things that need to be discussed.
I did not. I need to figure out how to maintain an email list from GH ids. And presumably Ben has a brand-new shiny IBM email id :-) Will send now. Both have email addresses in their GH profiles, so will send them a note.
I was trying to get @fmeawad to join the meeting. He's the chrome dev working on getting trace-event into V8, but I don't think he can make this particular time-slot.
@Qard I don't think tracing integration should be blocked by nodejs/node#2546. Once https://codereview.chromium.org/988893003/ lands into v8 it should be fairly easy to collect trace logs for v8 in Node and save them to a file. The file can be viewed with the trace-viewer by e.g. loading it into chrome:tracing
We use tracing instrumentation to populate Timeline panel in DevTools. All the instrumentation is provided by tracing. DevTools only implements a fairly simple tracing agent (100 loc) that allows to convert traces via the protocol. This agent may well be integrated into v8-inspector as long as tracing support is landed in v8. Timeline panel is targeted at web developers and tries to represent tracing data in a way understandable by people not familiar with the browser guts while tracing provides more sophisticated UI and exposes way more internal details. I believe trace-viewer may be more useful for Node developers. In any case as I said v8-inspector should not block the work on integration with tracing.
For anyone reading this, that's not in the list of attendees in the description of this issue (top of the web page), but wants to attend - or things they may be able to attend - please send me your email so I can add you to the Hangout.
I've updated the invite list on the hangout to include folks I noticed on previous calls, and mentioned in this issue; you can see the invite list here:
If you're not on that list, I either missed you, or couldn't find an email address for you. Either case, please send me an email you can use with Hangouts.
@ofrobots@natduca until the next meeting could you please link any code/specs implementations or examples that would allow us to get a better understanding of what we'd have to implement in order to use the tracing facilities you described?
One of the topics discussed in the meeting was setting up some spots in the repo to drop some documentation/diagrams/etc of all the various components that have been under discussion. I'll prime this with some structure this week, and then hopefully folks who understand the components can start filling in details.
@thlorenz: For the 'spec', the best starting point would be this comment from @natduca, and then following the linked documents from there.
Here are the main components, as per my understanding:
trace-event.h and trace-event-common.h. These define the low-level C macros that provide a low-overhead mechanism to insert traces into C++ code. @fmeawad is working on getting these files committed into V8 under this change-list.
As per the current CL, there a few methods that are getting added to v8-platform.h. An embedder needs to provide an implementation of these. Chromium provides the implementation of these in trace_log.h and trace_log.cc.
To implement these functions, an embedder needs a buffer that gathers the traces. The buffer also is responsible to periodically & asynchronously flushing the contents to whatever 'sink' the traces need to go to. The Chrome implementation of the buffer is in trace_buffer.h and trace_buffer.cc.
To implement the above in Node.js, I think we probably need a separate thread handling the flushing of the trace-buffer to keep it off the main thread. Checking if a particular trace is enabled needs to be extremely cheap, and putting something into the trace-buffer synchronously needs to be cheap.
Basically this trace-event buffer becomes the 'single pipe' through all trace-events gathered by different parts of the software stack.
Once the above linked CL lands, traces being gathered in V8 will show up there. Node.js can start putting its own traces into the same pipe. I would imagine we would also need an API in Node.js to be exposed to JavaScript to allow JS code to start sending the trace events.
Activity
matthewloring commentedon Sep 20, 2015
@pmuellr I would like to join in on this meeting
AndreasMadsen commentedon Sep 20, 2015
I'm not sure I will be able to make it, so I will just take an observer seat.
lucamaraschi commentedon Sep 20, 2015
IN!
pmuellr commentedon Sep 20, 2015
@matthewloring, @AndreasMadsen - send an email for the google doc / hangout, to pmuellr@gmail.com
pmuellr commentedon Sep 21, 2015
I've added a number of folks for editing on the agenda doc, and on the newly created Hangouts On Air event. Let me know if you didn't see notice of this, and if the various links in the description don't work for you.
AndreasMadsen commentedon Sep 23, 2015
This is the list of async_wrap issues that I could remember:
I don't know how specific the meeting should be and some of these have already been discussed. So I will let someone wiser decide.
pmuellr commentedon Sep 23, 2015
Guessing that @trevnorris has a fairly definitive list of async_wrap issues, but he unfortunately won't be able to make the call.
I'm not wiser, but have some comments. :-)
At this point it looks like the "tracing" story has a few different angles - async_wrap, v8-tracing.h, platform specific tracing w/ETW/etc.
Seems like v8-tracing.h / platform-specific tracing are kinda at the same level, and async_wrap is kinda different. I guess I see it as async_wrap will be a great place to put v8-tracing.h / platform-specific trace points. Does that seem right?
If so, it would be good to come up with different "names" for these things, so we can keep straight what we're talking about, for folks coming to this as n00bs, like me.
In any case, w/r/t async_wrap, using it as an exemplar of using v8/platform tracing seems ideal. We clearly want tracing hooks there. So let's talk about it some.
Would also like to get an update on v8-tracing, from @ofrobots . @billti also provided some info on work to add ETW support for tracing to node, in #10, ICYMI.
trevnorris commentedon Sep 23, 2015
Want to make sure everyone's on the same page about
AsyncWrap. It is a class where I centralized the creation of all class constructors that have the possibility of performing I/O. The current API exposed viaprocess.binding('async_wrap')was only created as a way to utilize the new class structure. That API could be removed easily without affecting the functionality of theAsyncWrapclass.All error handling abilities were removed in the PR because of conflicts with the domain module and
uncaughtException. Took me about a month to get it right, but was then considered to big of an alteration to merge at the time.My original PR addressed these cases, but unfortunately they conflicted with domains and were eventually removed because I wanted to get the native code changes merged. This can be done, but requires making calls similar to how domains work today.
This callback could not guarantee being called while the object was being destructed. Which means you would no longer have access to the object. I've toyed with an implementation in the past, but never reached a point of being useful.
This is because for performance there is a single
TimerWrapinstance with a JS linked list object attached that contains all the timers that will be fired in the future. So when a timer is cleared it's simply removed from the linked list.Probably because they are also creating a
ShutdownWrap(src/stream_wrap.h). All*Wraphave a unique id, which could allow filtering of these calls.Also note there are cases like
net.createServer()that will not triggerAsyncWrapuntil.listen()is called. This is because we don't know whether the user wants a Pipe or TCP until that time. It was a serious pain to work around when I was writing the initial error handling implementation.I haven't done much with
AsyncWrapfor a while because this is the type of conversation I wanted to have before proceeding. When should events be fires, in what order, etc. are things that need to be discussed.Qard commentedon Sep 23, 2015
Did you invite @bnoordhuis? He expressed interest in attending over in #19.
Qard commentedon Sep 23, 2015
It'd be good to get @yury-s in there too, so we can discuss what needs to happen to keep nodejs/node#2546 moving forward.
pmuellr commentedon Sep 23, 2015
I did not. I need to figure out how to maintain an email list from GH ids. And presumably Ben has a brand-new shiny IBM email id :-) Will send now. Both have email addresses in their GH profiles, so will send them a note.
ofrobots commentedon Sep 24, 2015
I was trying to get @fmeawad to join the meeting. He's the chrome dev working on getting trace-event into V8, but I don't think he can make this particular time-slot.
yury-s commentedon Sep 24, 2015
@Qard I don't think tracing integration should be blocked by nodejs/node#2546. Once https://codereview.chromium.org/988893003/ lands into v8 it should be fairly easy to collect trace logs for v8 in Node and save them to a file. The file can be viewed with the trace-viewer by e.g. loading it into chrome:tracing
We use tracing instrumentation to populate Timeline panel in DevTools. All the instrumentation is provided by tracing. DevTools only implements a fairly simple tracing agent (100 loc) that allows to convert traces via the protocol. This agent may well be integrated into v8-inspector as long as tracing support is landed in v8. Timeline panel is targeted at web developers and tries to represent tracing data in a way understandable by people not familiar with the browser guts while tracing provides more sophisticated UI and exposes way more internal details. I believe trace-viewer may be more useful for Node developers. In any case as I said v8-inspector should not block the work on integration with tracing.
2 remaining items
bnoordhuis commentedon Sep 24, 2015
Got the invite now (thanks @pmuellr) but I may have a conflicting schedule. Will try to join Friday but don't wait for me.
AndreasMadsen commentedon Sep 24, 2015
I'm quite sure I will be able to make it. So see you on Friday.
pmuellr commentedon Sep 25, 2015
For anyone reading this, that's not in the list of attendees in the description of this issue (top of the web page), but wants to attend - or things they may be able to attend - please send me your email so I can add you to the Hangout.
pmuellr commentedon Sep 25, 2015
I've updated the invite list on the hangout to include folks I noticed on previous calls, and mentioned in this issue; you can see the invite list here:
If you're not on that list, I either missed you, or couldn't find an email address for you. Either case, please send me an email you can use with Hangouts.
known email-challenged folks: @fmeawad @paulirish @chiniforooshan
lucamaraschi commentedon Sep 25, 2015
Guys I think I cannot attend the call as I am in bed sick! I will try to
attend and watch but I cannot guarantee...is it going to be recorded?
pmuellr commentedon Sep 25, 2015
@lucamaraschi yes, the Hangout should be recorded, if I don't screw up :-)
thlorenz commentedon Sep 28, 2015
@ofrobots @natduca until the next meeting could you please link any code/specs implementations or examples that would allow us to get a better understanding of what we'd have to implement in order to use the tracing facilities you described?
I'd greatly appreciate it.
pmuellr commentedon Sep 28, 2015
One of the topics discussed in the meeting was setting up some spots in the repo to drop some documentation/diagrams/etc of all the various components that have been under discussion. I'll prime this with some structure this week, and then hopefully folks who understand the components can start filling in details.
pmuellr commentedon Sep 28, 2015
I've created an issue to track the "add docs to repo" idea - #22
ofrobots commentedon Sep 28, 2015
@thlorenz: For the 'spec', the best starting point would be this comment from @natduca, and then following the linked documents from there.
Here are the main components, as per my understanding:
Basically this trace-event buffer becomes the 'single pipe' through all trace-events gathered by different parts of the software stack.
Once the above linked CL lands, traces being gathered in V8 will show up there. Node.js can start putting its own traces into the same pipe. I would imagine we would also need an API in Node.js to be exposed to JavaScript to allow JS code to start sending the trace events.
pmuellr commentedon Sep 28, 2015
I've got the meeting minutes in a PR - #23
Someone want to merge it? And give me commit access? :-)
Qard commentedon Sep 28, 2015
@pmuellr Request to join https://github.com/orgs/nodejs/teams/tracing and you should get in.
thlorenz commentedon Sep 29, 2015
Thanks @ofrobots will find some time to digest this all.
Qard commentedon Oct 7, 2015
I'm closing this, as the meeting has passed and the important comments have been copied or linked to in other issues now.