Add a new metric for the real cpu usage

[simonepri]

Sorry but I do believe that we are just reintroducing this bug: https://github.com/soyuka/pidusage/issues/58

A node process is not single threaded! The JS code itself runs on a thread generally called Event Loop but a node process is more than that.

Just think at this package. We spawn ps and ps performs some computation (On another thread) that is counted inside the node process.

[soyuka]

A node process is not single threaded! The JS code itself runs on a thread generally called Event Loop but a node process is more than that.

Yes and yes :). I know that, this is why I introduced the value usage that should be used instead of cpu for those who understand that the value can be greater than 100%. I have to take the pm2 userbase into consideration for this patch, and I know we might disagree on this. I was hoping that by having both values, everyone would be free to choose which one he wants to use.

[simonepri]

I have to take the pm2 userbase into consideration for this patch, and I know we might disagree on this.

Actually pm2 MUST report a cpu usage greater than 100. @vmarchaud It should just report the same cpu usage htop and ps would do. @Unitech It would be just wrong not doing so. Just think that a single node process could consume all the 32 cores.(By doing something in c++) What 100% would mean in that situation? Nothing.

I was hoping that by having both values, everyone would be free to choose which one he wants to use.

Add usage would just make everything more confused. The only thing we need is to document how the cpu value works.

[codecov-io]

Codecov Report

Merging #62 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #62   +/-   ##
=======================================
  Coverage   92.57%   92.57%           
=======================================
  Files           9        9           
  Lines         283      283           
=======================================
  Hits          262      262           
  Misses         21       21

Impacted Files	Coverage Δ
lib/wmic.js	89.28% <ø> (ø)	:arrow_up:
lib/procfile.js	100% <ø> (ø)	:arrow_up:
lib/ps.js	95.34% <ø> (ø)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f990f72...d3791e5. Read the comment docs.

[vmarchaud]

I think there is a misunderstanding here, i am in favor of cpu usage being more than 100%, its possible if you have multiple core. I think we should indeed report the same cpu usage as htop

[simonepri]

@soyuka can we formalize better the problems that the current implementation causes before merging this?

Actually I was the one that has introduced the Math.min because I wasn't aware of what a cpu % really was meaning. Before the 2.0 version the Math.min part wasn't here and we were all happy. Then we have realized that my contribution was bugged and we patched it.

Why are we going to reintroduce the bug? Cannot get it, Sorry!

[simonepri]

I think we should indeed report the same cpu usage as htop

Perfect, so this is what I think we need to do: 1) Close this PR 2) Add more unit test to be sure that the current implementation (That should be the correct one) actually reports the correct cpu usage. (And by correct I mean the same reported by top/ps etc)

[soyuka]

Why are we going to reintroduce the bug? Cannot get it, Sorry!

chill out ^^"

I thought that @vmarchaud wanted a cpu usage between 0 and 100. So this is only a misunderstanding.

I'm obviously for having the real value (ie value from 0 to 100*vcore) as cpu.

Before the 2.0 version the Math.min part wasn't here and we were all happy.

Add more unit test to be sure that the current implementation (That should be the correct one) actually reports the correct cpu usage. (And by correct I mean the same reported by top/ps etc)

It is for ps because we use it. procfiles should have the same behavior as cpuUsage in node I guess.

Before the 2.0 version the Math.min part wasn't here and we were all happy.

This is true, let's continue to be happy.

[simonepri]

chill out ^^"

Sorry, I did not want to be rude.

procfiles should have the same behavior as cpuUsage in node I guess.

Yeah

let's continue to be happy.

Sorry for having made all of you sad 😂

I'm obviously for having the real value (ie value from 0 to 100*vcore) as cpu.

I'll take some time as soon as I can to improve the documentation in the readme to justify why the range is not 0-100%

[soyuka]

Sorry for having made all of you sad joy

:laughing: no worries dude this was all a big misunderstanding from my part, I'm doing too much at the same time I guess :).

I'll take some time as soon as I can to improve the documentation in the readme to justify why the range is not 0-100%

:heart: