-
Notifications
You must be signed in to change notification settings - Fork 78
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
trace_event.h tracking issue #30
Comments
We can add it now if there is a good reason to.
It would be nice if there was a common library that we could use without pulling in chromium base as a dependency. It's big and painful to build outside the chromium tree. /cc @ofrobots
I imagine you'd start with a (public) C++ API, that the JS API ends up calling. The API itself should probably be callback-based, where node.js calls the callback with the raw trace data. TBD if callbacks are made on the main thread. That's mandatory for the JS API but not for the C++ API and would be a performance drag. Apropos C++ streams, that seems like a bad fit, never mind that they're not API-stable. |
The callbacks thing is why I was wondering about C++ streams. It might be possible to get decent performance just writing it to a file or the network directly in C++, rather than constantly doing the hop to JS. I don't know too much about the stability of C++ streams, it was just a thought. As for the Chromium bits, it probably makes the most sense to just pull the trace_log and trace_buffer bits out into its own small project that we can dump in the deps folder. @fmeawad |
My feeling is that it would simpler to copy (and fork) trace_log and trace_buffer files from Chromium. There is a big dependency on chromium base in the file currently and extracting it out to a library would require extricating those dependencies anyway. I personally don't think it is worth the effort required to share these bits between Node.js and Chromium. |
FYI, the trace-event CL has since landed into V8 4.9. If anyone wants to play with this, I have Node.js + V8 4.9 branch here: https://github.com/ofrobots/node/tree/vee-eight-4.9. |
Note that @matthewloring, @kjin and @misterpoe have been working on prototyping TraceEvent for Node on this branch here: https://github.com/matthewloring/node/commits/tracing. This is dependent on V8 5.4. We can open this is as a PR after V8 5.4 lands. Collaboration would be most welcome! |
I'll try to take a look soon, thanks for the update @ofrobots! /cc @digitalinfinity |
#53 is discussion of integration strategies for this, perhaps we should merge threads. |
conversation continues now in #84 |
This is a tracking issue for trace_event.h integration. If I've missed anything, please add it.
TODO:
TRACE_EVENT_BEGIN0
,TRACE_EVENT_END0
,TRACE_EVENT_ASYNC_BEGIN0
,TRACE_EVENT_ASYNC_END0
macros in node codebase (AsyncWrap might be a good place to start)The text was updated successfully, but these errors were encountered: