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

proposal: require('perf') as API for metric based data #6022

Closed
eljefedelrodeodeljefe opened this issue Apr 2, 2016 · 7 comments
Closed
Labels
discuss Issues opened for discussions and feedbacks.

Comments

@eljefedelrodeodeljefe
Copy link
Contributor

As mentioned on #5565 (cc @mscdex, @evanlucas) I would like to propose a subsystem that would hold perf related data. Currently those are scattered mostly in process, where they might not fit.


Also I am researching C APIs for Linux perf_events / Windows PDH / BSD Dtrace (in C) that could (theoretically) be easily added to the debugger e.g. for stack traces beyond js. libuv already provides APIs like uv_getrusage that can be easily extended into node.

Since the C APIs offer a multitude of information the API could potentially be to bloated for process and could potentially include thread related or other data, that don't fit the semantic context.

Action Items

  • this would be a more global discussion
  • however I am sending in a PR to libuv to fill up some windows metrics of the unix named getrusage (first stab was here util: extends uv_getrusage by majflt on NT and other NT counters libuv/libuv#787). Afterwards I would send in a PR to node to expose uv_getrusage. The question would be whether it should go into a new subsystem, into process or into process and maybe later into a new subsystem.
@mscdex
Copy link
Contributor

mscdex commented Apr 2, 2016

Are you proposing a general performance API (not necessarily related to the current process) or an API as it directly relates to the current process only?

@mscdex mscdex added the discuss Issues opened for discussions and feedbacks. label Apr 2, 2016
@Fishrock123
Copy link
Contributor

Things relating to the current process should probably be on process imo.

If it's across you're OS, wouldn't 'os' be a good fit? That's where we've put some of that before.

@r-52
Copy link
Contributor

r-52 commented Apr 5, 2016

IMO the data that uv_getrusage returns is "process-bound" and belongs more to the process object. (It could match e.g. process.memoryUsage()).

Or would you like to split the process object (one with the current data and a module with new, process related, data)?

(Just as a side note: A long, long time ago, I proposed a PR with uv_getrusage in #1568.)

@rvagg
Copy link
Member

rvagg commented Apr 15, 2016

It seems to me that things within the "perf" scope would apply to the current process so it's not too hard to justify extending the process module.

If we were to introduce a new core module we'd either need to come up with a name that doesn't introduce a conflict with userland or negotiate with the owner of the name we want to take ownership of it to core, currently for "perf" that's https:/reqshark/perf

@jasnell
Copy link
Member

jasnell commented Apr 22, 2016

Yeah, -1 on a new module for this. process seems like the best place to hang this stuff.

@eljefedelrodeodeljefe
Copy link
Contributor Author

Okay. I then I am going to continue this in the scope of process. I just wanted to avoid sementaic collisions, for when you are measuring hardware. But maybe this then could be put into others APIs from the c++ layer. Will work on this next week.

Side note on uv_get_rusage: I'd be in favor for #1568 (@romankl) and offered some help. However I think the unix getrusage is really insufficient and being replaced by the things I would implement in C here also. So it would follow the history on the os'es

@jasnell
Copy link
Member

jasnell commented May 30, 2017

I don't believe this needs to stay open. Closing but we can reopen if necessary

@jasnell jasnell closed this as completed May 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issues opened for discussions and feedbacks.
Projects
None yet
Development

No branches or pull requests

6 participants