Don't use cpus count to compute pcpu revert #59

[soyuka]

58

https://github.com/Unitech/pm2/issues/3708

[simonepri]

You need to remove: https://github.com/soyuka/pidusage/blob/master/test/ps.js#L45 https://github.com/soyuka/pidusage/blob/master/test/ps.js#L110

[simonepri]

How these behave on multi-threaded process? https://github.com/soyuka/pidusage/blob/master/lib/procfile.js#L48 https://github.com/soyuka/pidusage/blob/master/lib/wmic.js#L105

[soyuka]

You need to remove: https://github.com/soyuka/pidusage/blob/master/test/ps.js#L45 https://github.com/soyuka/pidusage/blob/master/test/ps.js#L110

Not really, let's keep them for later they're not used anymore.

How these behave on multi-threaded process? https://github.com/soyuka/pidusage/blob/master/lib/procfile.js#L48 https://github.com/soyuka/pidusage/blob/master/lib/wmic.js#L105

I'll find time to investigate on this. First guess is joins what you said earlier - nothing is telling us if the process is using more then 1 core. This means that we can't really divide by the number of cores... I don't know which value we can use, especially that utime/stime values that we're using are not telling us anything about cores. I'm not sure that it's as easier as totaltime / cpus number in the end.

[codecov-io]

Codecov Report

Merging #60 into master will decrease coverage by 0.02%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
- Coverage    92.6%   92.57%   -0.03%     
==========================================
  Files           9        9              
  Lines         284      283       -1     
==========================================
- Hits          263      262       -1     
  Misses         21       21

Impacted Files	Coverage Δ
lib/ps.js	95.34% <100%> (-0.11%)	:arrow_down:

---

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 ed9f81d...e00f66e. Read the comment docs.

[simonepri]

"nothing is telling us if the process is using more then 1 core" Probably that is not completely true.

Let's assume that you do 2 measurements m1 and m2 at two different moments t1 and t2 If the cpu time used by the process m2-m1 in the interval is greater than t2-t1 then you must have used more than 1 cpu.

The idea is the following: If it's elapsed 1s but the readings reports 4s cpu time consumed in the last 1s then we must have used 1s time on 4 core parallelly. That make sense for you?

These are just supposition, I'm not actually aware on how the utime /stime are actually counted

[simonepri]

// process usage since last call
var total = (kerneltime + usertime - hst.ctime) / 1000
// time elapsed between calls in seconds
var seconds = uptime - hst.uptime
var cpu = seconds > 0 ? (total / seconds) * 100 : 0

If my suppositions are true, then total / seconds may be greater than 1 and our code just works as is and the only thing to do, is to remove these lines: https://github.com/soyuka/pidusage/blob/master/lib/procfile.js#L53 https://github.com/soyuka/pidusage/blob/master/lib/wmic.js#L110 https://github.com/soyuka/pidusage/pull/60/files#diff-88c126b24fe4510cfe06437cb9045a0eL104

[simonepri]

Probably we need to get rid of Math.min everywhere.

[soyuka]

Yes, my guess is that you're probably right!

Anyway, it's always tricky to represent CPU usage because it's hard to tell what it actually is IMO. The current value is "representative" without being perfectly exact. We definitely need to add more data in the results, maybe with something closer to the truth :D.

I'm merging as-is for now!