-
-
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
Windows setting CPU Percents on cpu.TimeStat object #420
Comments
This PR fixes an issue introduced in Nomad 0.6.0 due to shirou/gopsutil#420. The issue arised from the fact that the Windows stats from gopsutil reports CPUs in percentages where we expected ticks.
Sorry, currently I don't have time to investigate this. What struct should we use? Could you send a PR? |
The CPU package doesn't really have an appropriate struct. I would at the very minimum add documentation stating the different behavior on the Windows platform. |
What makes it even more confusing is that Ticks: https://github.com/shirou/gopsutil/blob/master/cpu/cpu_windows.go#L52-L76 |
This PR fixes an issue introduced in Nomad 0.6.0 due to shirou/gopsutil#420. The issue arised from the fact that the Windows stats from gopsutil reports CPUs in percentages where we expected ticks.
This PR fixes an issue introduced in Nomad 0.6.0 due to shirou/gopsutil#420. The issue arised from the fact that the Windows stats from gopsutil reports CPUs in percentages where we expected ticks.
I was taking a look at this today and was doing some experimentation here I started using the WMI Code generator tool to see what we could get, and saw that the current class we're querying (win32_perfformatteddata_counters_processorinformation) does return percentage values. I used the WMI code generator tool to see if there were any other classes that might get us the CPU time values and I 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 package main
import (
"fmt"
"github.com/marcospedreiro/gopsutil/cpu"
)
func main() {
fmt.Println("[windows] cpu.Times(false)")
t, err := cpu.Times(false)
if err != nil {
fmt.Println(err)
return
}
for _, ct := range t {
fmt.Println(ct)
}
fmt.Println("[windows] cpu.Times(true)")
t, err = cpu.Times(true)
if err != nil {
fmt.Println(err)
return
}
for _, ctf := range t {
fmt.Println(ctf)
}
return
} Currently when I run this on master I get the following on my windows computer:
With my commit that I linked above I get the following:
It appears that the If we sum the user time values for each core:
we get
I'm happy to clean up my commit and submit a PR if we feel this is the correct approach to go with. |
Fixed by #611. |
This should probably be a different struct as the other methods will accept it and behave incorrectly. Also it causes upstream bugs on code that relies on it not being a percent.
https://github.com/shirou/gopsutil/blob/master/cpu/cpu_windows.go#L140
The text was updated successfully, but these errors were encountered: