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

process.cpuUsage accuracy #8728

Closed
nickstanish opened this issue Sep 22, 2016 · 5 comments
Closed

process.cpuUsage accuracy #8728

nickstanish opened this issue Sep 22, 2016 · 5 comments
Labels
process Issues and PRs related to the process subsystem. question Issues that look for answers.

Comments

@nickstanish
Copy link
Contributor

  • v6.3.1:
  • Darwin Kernel Version 15.6.0 root:xnu-3248.60.11~1/RELEASE_X86_64 x86_64:
  • process:

I'm trying to find cpu utilization of the process as a percentage of the system as a whole - similar to how top, activity monitor, and similar tools work, but from within the program itself. It appears that process.cpuUsage is a relatively new feature, and I haven't been able to find a way to get the cpu percentage and have it accurately match values found in system monitors. I created a repository of some of the different methods I have tried: https://github.com/nickstanish/node-test-process-cpuusage

It appears that there is discussion on an approach to get the percentage, but uses the wrong precision and seems to not account for number of cpu cores.

Is there a recommended approach in order to find this value?

I'd also like to point out that the tests for this feature have the wrong precision as well which makes the possible value range huge. The process usage values are in microseconds and it multiplies by 1e6
https://github.com/nodejs/node/blob/master/test/pummel/test-process-cpuUsage.js#L27

Original Issue:
nodejs/help#283

@mscdex mscdex added the process Issues and PRs related to the process subsystem. label Sep 23, 2016
@tniessen tniessen added the question Issues that look for answers. label Jun 2, 2017
@Trott
Copy link
Member

Trott commented Aug 7, 2017

/ping @pmuellr

@pmuellr
Copy link

pmuellr commented Aug 8, 2017

Hi Nick, did you have any luck finding a good recipe? I tried your test1.js and test2.js, and got the following results:

$ node test1.js
elapsed time ms:   1504.27047
elapsed user ms:   NaN
elapsed system ms: NaN
cpu percent:       NaN
elapsed time ms:   3015.270453
elapsed user ms:   NaN
elapsed system ms: NaN
cpu percent:       NaN
^C

$ node test2.js
elapsed time ms:   1503.639829
elapsed user ms:   501.57
elapsed system ms: 16.713
cpu percent:       34.5%

elapsed time ms:   1520.0664040000001
elapsed user ms:   508.175
elapsed system ms: 18.608
cpu percent:       34.7%
^C

test1.js seems to be treating the cpuUsage() values like hrtime() structures, which they aren't, hence the NaNs. test2.js seems to be calculating a reasonable results, for the first round, but I would have expected the second and subsequent ones to have an elapsed time of 1000ms (not 1500ms) - it seems like setInterval() schedules the next timer to go off AFTER the current one has finished, rather than a fixed "every XXX ms"? That's interesting, didn't realize it did that! (or forgotten it)

seems to not account for number of cpu cores

Correct, it does not account for cores, as far as I know. The underlying API measures the amount of CPU used across all cores, so if your program uses 100% CPU in the main JavaScript thread, but also does work in other threads, you should expect the resultant % value to be > 100%.

I think CPU usage values like this are typically based on a single CPU; that's what I seem to see in Mac's Activity Monitor, for instance. Same sort of thing, where a process using most of two cores will show CPU % values like 200%.

If you want to have a value of % of total machine CPUs used, you could of course divide by the number of CPUs.

the tests for this feature have the wrong precision

I believe you are right, and we should fix this.

Seems like the following bits:

const MICROSECONDS_PER_SECOND = 1000 * 1000;
...
assert(diff.user <= SLOP_FACTOR * RUN_FOR_MS * MICROSECONDS_PER_SECOND);

should be changed to:

const MICROSECONDS_PER_MILLISECOND = 1000
...
assert(diff.user <= SLOP_FACTOR * RUN_FOR_MS * MICROSECONDS_PER_MILLISECOND);

@nickstanish
Copy link
Contributor Author

Thanks for taking a look. After trying some more, I created a new example similar to test2 that is having results much more similar to what you would see in activity monitor.
The trick here was to keep the elapsed time as close as possible to the refresh rate in activity monitor (based on a 2 second update frequency).

I think that resolves any questions I had around process.cpuUsage, but we should still update the test to use the correct multiplier.

nickstanish added a commit to nickstanish/node that referenced this issue Aug 9, 2017
@pmuellr
Copy link

pmuellr commented Aug 9, 2017

Great!

Agree that we should fix the test; would you like the "honors"? Otherwise I'll try to get around to it this week or next ...

@nickstanish
Copy link
Contributor Author

Yeah, here it is: #14706

@Trott Trott closed this as completed in a439cf4 Aug 10, 2017
addaleax pushed a commit that referenced this issue Aug 10, 2017
PR-URL: #14706
Fixes: #8728
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 19, 2017
PR-URL: #14706
Fixes: #8728
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Issues and PRs related to the process subsystem. question Issues that look for answers.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants