Fix Process cpu_percent on Windows

[PierreF]

Windows cpu_percent of Process is wrong. On a mostly idle system, a process take few percent of the CPU will be shown as using 100% by Process.cpu_percent().

This is due to the way time elapsed calculation that under Windows use the global cpu_times of user + system (ignore idle, DPC and interrupt).

This PR propose to solve this issue by using the same method as POSIX system: use the monotonic clock (if available) and fallback to time.time().

[giampaolo]

Mmm that part of the code is quite old and honestly I don't remember why the logic on Windows is different. What does not convince me though it this:

On a mostly idle system, a process take few percent of the CPU will be shown as using 100% by Process.cpu_percent().

I've never seen this happening. Please try to elaborate more about how you think the current implementation is broken.

[giampaolo]

Also by doing this:

-        delta_time = st2 - st1		 
+        delta_time = (st2 - st1) * num_cpus

...you're changing also the UNIX implementation. Why is that necessary?

[PierreF]

On Windows the % of CPU of one process is calculated as:

(delta_proc / delta_time) * 100 

Where delta_proc is the number of second the process has taken, and delta_time is intended to be the number of real seconds elapsed.
But delta_time is in fact the number of global cpu user + cpu system that was consumed between the two measurements. So other time (like idle time or interrupt time) or not added.
This was changed in commit ba06504, and was previously a sum(cpu_times).

I'm testing on a Windows 10 if that matter.

I've changed the UNIX implementation to remove the definition of timer() function which would be the same on every system. I could keep the timer() function and just remove the "if POSIX" if you think it's more appropriate.

[giampaolo]

I see. I checked now on Windows by using a busy loop process on a 2 CPUs machine and indeed it is reported as consuming around 200% where instead I would expect 100%.
I think this change should do it:

diff --git a/psutil/__init__.py b/psutil/__init__.py
index f8ce48e..7472ca8 100644
--- a/psutil/__init__.py
+++ b/psutil/__init__.py
@@ -1007,13 +1007,8 @@ class Process(object):
             raise ValueError("interval is not positive (got %r)" % interval)
         num_cpus = cpu_count() or 1
 
-        if POSIX:
-            def timer():
-                return _timer() * num_cpus
-        else:
-            def timer():
-                t = cpu_times()
-                return sum((t.user, t.system))
+        def timer():
+            return _timer() * num_cpus
 
         if blocking:
             st1 = timer()

[PierreF]

Yes. As said I did a little more change to avoid the definition of the timer() function.

Do you want me to update the PR with this change or are you going to directly commit that change ?

[giampaolo]

Yes please update the PR with the change I paste above. While you're at it also change CREDITS and HISTORY please, then it's good to go.

[PierreF]

PR updated. Thanks for your reactivity !

[giampaolo]

It is amazing how something this simple and important has been broken for such a long time. This is an important bugfix. Thanks for bringing it up.

[maxbelanger]

Was just about to report this as a bug; amazed at how quickly it was fixed. Thanks all!

[maxbelanger]

@giampaolo any chance this could make a point-release (i.e. 5.0.2)? We'd love to give this a try :).