Skip to content

process: add process.cpuUsage() - implementation, doc, tests #10796

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

process: add process.cpuUsage() - implementation, doc, tests #10796

wants to merge 1 commit into from

Conversation

@MylesBorins
Copy link
Contributor

@MylesBorins MylesBorins commented Jan 14, 2017 •
edited
Loading

Add process.cpuUsage() method that returns the user and system
CPU time usage of the current process

This is a backport of #6157 to v4.x-staging

/cc @nodejs/lts @pmuellr

notes: the original commit made changes in internal/bootstrap_node and internal/process... in v4.x these changes are all applied to src/node

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

src
@MylesBorins
process: add process.cpuUsage() - implementation, doc, tests
0782a5b 
Add process.cpuUsage() method that returns the user and system CPU time usage of the current process PR-URL: nodejs#6157 Reviewed-By: Robert Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@MylesBorins MylesBorins added lib / src Issues and PRs related to general changes in the lib or src directory. v4.x labels Jan 14, 2017
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. v4.x labels Jan 14, 2017
@MylesBorins MylesBorins mentioned this pull request Jan 14, 2017
Closed
9 tasks
@MylesBorins
Copy link
Contributor Author

MylesBorins commented Jan 14, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/5856/
@mscdex
Copy link
Contributor

mscdex commented Jan 14, 2017

I have thrown these eyes on this PR: 👀

LGTM though.
thefourtheye
thefourtheye reviewed Jan 14, 2017
View reviewed changes
src/node.js
// numbers <= Number.MAX_SAFE_INTEGER.
function previousValueIsValid(num) {
return Number.isFinite(num) &&
num <= Number.MAX_SAFE_INTEGER &&
Copy link
Contributor

@thefourtheye thefourtheye Jan 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about Number.isSafeInteger here?
thefourtheye
thefourtheye reviewed Jan 14, 2017
View reviewed changes
test/parallel/test-process-cpuUsage.js

// Ensure that the return value is the expected shape.
function validateResult(result) {
assert.notEqual(result, null);
Copy link
Contributor

@thefourtheye thefourtheye Jan 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notStrictEqual
thefourtheye
thefourtheye reviewed Jan 14, 2017
View reviewed changes
test/pummel/test-process-cpuUsage.js
@@ -0,0 +1,30 @@
'use strict';
Copy link
Contributor

@thefourtheye thefourtheye Jan 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this test is in pummel?
@thefourtheye
Copy link
Contributor

thefourtheye commented Jan 14, 2017

Ah, never mind the comments. Just noticed that its a backport. LGTM if CI is happy.
jasnell pushed a commit that referenced this pull request Jan 16, 2017
@jasnell
process: add process.cpuUsage() - implementation, doc, tests
151cca6 
Backport to v4.x Original commit message: Add process.cpuUsage() method that returns the user and system CPU time usage of the current process PR-URL: #6157 Reviewed-By: Robert Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> PR-URL: #10796 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@jasnell
Copy link
Member

jasnell commented Jan 16, 2017

Landed in v4.x-staging in 151cca6
@jasnell jasnell closed this Jan 16, 2017
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
@MylesBorins
process: add process.cpuUsage() - implementation, doc, tests
bf3a67c 
Backport to v4.x Original commit message: Add process.cpuUsage() method that returns the user and system CPU time usage of the current process PR-URL: #6157 Reviewed-By: Robert Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> PR-URL: #10796 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Feb 1, 2017
@MylesBorins
process: add process.cpuUsage() - implementation, doc, tests
7a9c8d8 
Backport to v4.x Original commit message: Add process.cpuUsage() method that returns the user and system CPU time usage of the current process PR-URL: #6157 Reviewed-By: Robert Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> PR-URL: #10796 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@MylesBorins MylesBorins added the semver-minor PRs that contain new features and should be released in the next minor version. label Feb 21, 2017
MylesBorins added a commit that referenced this pull request Feb 21, 2017
@MylesBorins
2017-02-21, Version 4.8.0 'Argon' (LTS)
58ab54f 
Notable Changes: * child_process: add shell option to spawn() (cjihrig) #4598 * crypto: * add ALPN Support (Shigeki Ohtsu) #2564 * allow adding extra certs to well-known CAs (Sam Roberts) #9139 * deps: * v8: expose statistics about heap spaces (Ben Ripkens) #4463 * fs: add the fs.mkdtemp() function. (Florian MARGAINE) #5333 * process: * add `externalMemory` to `process` (Fedor Indutny) #9587 * add process.cpuUsage() (Patrick Mueller) #10796
MylesBorins added a commit that referenced this pull request Feb 22, 2017
@MylesBorins
2017-02-21, Version 4.8.0 'Argon' (LTS)
49c5ff4 
Notable Changes: * child_process: add shell option to spawn() (cjihrig) #4598 * crypto: * add ALPN Support (Shigeki Ohtsu) #2564 * allow adding extra certs to well-known CAs (Sam Roberts) #9139 * deps: * v8: expose statistics about heap spaces (Ben Ripkens) #4463 * fs: add the fs.mkdtemp() function. (Florian MARGAINE) #5333 * process: * add `externalMemory` to `process` (Fedor Indutny) #9587 * add process.cpuUsage() (Patrick Mueller) #10796 PR-URL: #10973
imyller added a commit to imyller/meta-nodejs that referenced this pull request Mar 2, 2017
@imyller
2017-02-21, Version 4.8.0 'Argon' (LTS)
bddd5cc 
 Notable Changes: * child_process: add shell option to spawn() (cjihrig) nodejs/node#4598 * crypto: * add ALPN Support (Shigeki Ohtsu) nodejs/node#2564 * allow adding extra certs to well-known CAs (Sam Roberts) nodejs/node#9139 * deps: * v8: expose statistics about heap spaces (Ben Ripkens) nodejs/node#4463 * fs: add the fs.mkdtemp() function. (Florian MARGAINE) nodejs/node#5333 * process: * add `externalMemory` to `process` (Fedor Indutny) nodejs/node#9587 * add process.cpuUsage() (Patrick Mueller) nodejs/node#10796 Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
imyller added a commit to imyller/meta-nodejs that referenced this pull request Mar 2, 2017
@imyller
2017-02-21, Version 4.8.0 'Argon' (LTS)
2ee49f0 
 Notable Changes: * child_process: add shell option to spawn() (cjihrig) nodejs/node#4598 * crypto: * add ALPN Support (Shigeki Ohtsu) nodejs/node#2564 * allow adding extra certs to well-known CAs (Sam Roberts) nodejs/node#9139 * deps: * v8: expose statistics about heap spaces (Ben Ripkens) nodejs/node#4463 * fs: add the fs.mkdtemp() function. (Florian MARGAINE) nodejs/node#5333 * process: * add `externalMemory` to `process` (Fedor Indutny) nodejs/node#9587 * add process.cpuUsage() (Patrick Mueller) nodejs/node#10796 Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
@MylesBorins MylesBorins deleted the backport-cpuUsage-v4.x branch November 14, 2017 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. semver-minor PRs that contain new features and should be released in the next minor version.

7 participants

@MylesBorins @mscdex @thefourtheye @jasnell @ofrobots @addaleax @nodejs-github-bot