-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[cpu][windows] cpu.Times(true) should not return percent values #611
Conversation
cpu/cpu_test.go
Outdated
perCPUIdleTimeSum += pc.Idle | ||
} | ||
margin := 2.0 | ||
if !isWithinMargin(perCPUUserTimeSum, cpuTotal[0].User, margin) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use assert.InEpsilon
(or assert.InDelta) instead.
Thanks for this great PR and your explanations, I will test it in the coming days. 👍 |
Updated the test suite based on your comment (good call out I forgot about that!) Thanks! Let me know what you find. I'm not 100% sure yet as to why dividing by 10,000,000 gives the desired values because I haven't been able to identify what the units of Looking at win32_perfrawdata_counters_processorinformation percentusertime_properties I see that the counter type is
From the Microsoft documentation stating that there are
|
Which Windows versions were these modifications tested on? |
|
@Lomanic Just checking in, were you able to test these changes? Let me know if there are any modifications you want me to make. |
Didn't have time to do that during the weekend sorry. Probably this one to come. |
Thanks a lot for this great PR @marcospedreiro, appreciated! |
Latest gosutil includes two backward incompatible changes: First, it removed unused Stolen field in shirou/gopsutil@cae8efc#diff-d9747e2da342bdb995f6389533ad1a3d . Second, it updated the Windows cpu stats calculation to be inline with other platforms, where it returns absolate stats rather than percentages. See shirou/gopsutil#611.
Latest gosutil includes two backward incompatible changes: First, it removed unused Stolen field in shirou/gopsutil@cae8efc#diff-d9747e2da342bdb995f6389533ad1a3d . Second, it updated the Windows cpu stats calculation to be inline with other platforms, where it returns absolate stats rather than percentages. See shirou/gopsutil#611.
Latest gosutil includes two backward incompatible changes: First, it removed unused Stolen field in shirou/gopsutil@cae8efc#diff-d9747e2da342bdb995f6389533ad1a3d . Second, it updated the Windows cpu stats calculation to be inline with other platforms, where it returns absolate stats rather than percentages. See shirou/gopsutil#611.
Details
perCPUTimesWithContext(ctx context.Context)
incpu/cpu_windows.go
to return CPU time metrics as opposed to percentage values such thatcpu.Times(true)
is consistent withcpu.Times(false)
on windows and other platforms.TestCpu_times()
incpu/cpu_test.go
to validate that summing the per CPU user, system, and idle times is within a certain margin of the cpu time reported by the total.The current class we're querying (win32_perfformatteddata_counters_processorinformation) returns percentage values. I used the WMI Code generator tool to experiment and see what types of classes and queries were available and saw win32_perfrawdata_counters_processorinformation. From the linked documentation, they are stated to return identical values, except the latter has a few extra fields.
However querying my windows computer for
win32_perfrawdata_counters_processorinformation
returned different values that were not percentages. I experimented locally with the new class and the following code snippet:and got the output:
It appears that the
win32_perfrawdata_counters_processorinformation
may be giving us cpu time data as opposed to percentages, but what I've found in terms of documentation so far has not been helpful, not to mention that we are off by a couple orders of magnitude from thecpu-total
values.If we sum the user time values for each core:
111033750000.0 + 112372968750.0 + 111817812500.0 + 121851093750.0 + 125105781250.0 + 108120156250.0
we get
690301562500
which is the same value as the cpu total69030.2
except larger by a factor of 10,000,000, which is the number of clock ticks per second on windowsTesting
make build_test
Tested on linux (Centos7), macOS 10.14.1, and Windows 10 with the following code:
Example Windows 10 Output:
Example MacOS Output:
example Centos7 Output
Relevant Issues