bpo-34060: Report system load when running test suite for Windows#8287
bpo-34060: Report system load when running test suite for Windows#8287ammaraskar wants to merge 2 commits intopython:masterfrom
Conversation
|
A quick note on the LOADAVG_FACTOR_1F and SAMPLING_INTERVAL; that given dampening factor is for a sample rate of 5 seconds. SAMPLING_INTERVAL should really be changed to 5 or re-run the equation with the different interval. |
|
Thanks again Jeremy, I changed the interval to 5 to be more in line with the Linux scheduler. |
| HQUERY hQuery; | ||
| HCOUNTER hCounter; | ||
|
|
||
| if ((s = PdhOpenQueryW(NULL, 0, &hQuery)) != ERROR_SUCCESS) |
There was a problem hiding this comment.
Please move the assignment on a separated line. Same comments for other assignments in if().
| goto WMIerror; | ||
| } | ||
|
|
||
| HANDLE event = CreateEventW(NULL, FALSE, FALSE, L"LoadUpdateEvent"); |
There was a problem hiding this comment.
Hum. It seems like you allocate a resource, but never release it.
I would suggest to create an object which keeps track of these resources and release them later. getloadavg() would be a method of that object.
There was a problem hiding this comment.
Would it be acceptable to do it in the module's m_free function? I was initially thinking of doing something similar but thought that would create a lot of extra code and complexity for an internal API with one use case.
There was a problem hiding this comment.
I dislike using m_free() for that.
|
Also, I don't think this actually needs a NEWS entry right? It relates to cpython build process and doesn't actually affect the language. Could you mark it with the |
|
What extra runtime dependencies does this add to
(Haven't looked too closely at the implementation yet, but when I do, it will be with the view that it is a public API change. Unless you rework it into its own module first.) |
|
(Edited above post). Let me take that back - was thinking of |
|
Aah yeah I think you have a good point, I think making it its own module would be a bit overkilll so I'll clean up the API anticipating some public usage. |
See the bpo ticket for more technical details on the implementation and otherwise.
https://bugs.python.org/issue34060