Skip to content
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

Get Windows percent swap usage from performance counters #2160

Merged
merged 19 commits into from
Apr 13, 2023

Conversation

dbwiddis
Copy link
Contributor

@dbwiddis dbwiddis commented Oct 21, 2022

Signed-off-by: Daniel Widdis widdis@gmail.com

Summary

Description

Commit Total (physical + swap used) is incremented earlier than physical memory used, which leads to overestimating physical memory available, and results in negative "available" values when subtracting physical available from the available system (commit limit - commit total) value determining swap available.

This PR ignores Commit Total and instead calculates swap usage using the performance counter for Paging File % Usage.

@beqabeqa473
Copy link

Calculations are incorrect in some cases.

For example, in my case, i have 32 GB physical ram and 4.75 swap.

When i run cext.virtual_mem(), I get following result.

(34200948736, 18837475328, 39301222400, 15633743872)
if we subtract 15633743872 and 18837475328 we get a negative result -3203731456 when is alligned by max, and we get that swap is all used, however it is not a trueth.

@dbwiddis
Copy link
Contributor Author

(34200948736, 18837475328, 39301222400, 15633743872)
if we subtract 15633743872 and 18837475328 we get a negative result -3203731456 when is alligned by max, and we get that swap is all used, however it is not a trueth.

That depends on your definition of "used". That swap is "committed".

  • Commit Limit = 39301222400 (36.6 GB = your physical + swapfile)
  • "available" = 15633743872
    • so we can calculate that Commit Total = 23667478528 (22.0 GB).

That "Commit total" is memory reserved for open programs. It is reserved instantly because Windows guarantees it will be available when asked for.

Your used physical memory is 34200948736 - 18837475328 = 15363473408 (14.3 GB)

Physical memory used does not increment until the reserved pages are accessed.

So you have 22.0 GB - 14.3 GB = 7.73 GB of pages reserved (committed) but not yet accessed. This exceeds the size of your swap file, therefore you have "used" all your swap.

@dbwiddis dbwiddis force-pushed the zero-swap branch 2 times, most recently from 379abfd to 78861c2 Compare October 21, 2022 18:40
@dbwiddis dbwiddis force-pushed the zero-swap branch 2 times, most recently from f069638 to 00724f0 Compare October 21, 2022 18:54
@dbwiddis dbwiddis marked this pull request as ready for review October 21, 2022 18:55
@dbwiddis
Copy link
Contributor Author

Sorry for all the pushes. I think I'm done now... but see the linked issue for further discussion on virtual memory API.

@Danstiv
Copy link

Danstiv commented Oct 21, 2022

So you have 22.0 GB - 14.3 GB = 7.73 GB of pages reserved (committed) but not yet accessed. This exceeds the size of your swap file, therefore you have "used" all your swap.

It seems to me that Windows doesn't use swap until committed memory exceeds total physical memory, that would be weird...

@dbwiddis dbwiddis marked this pull request as draft October 21, 2022 19:50
@dbwiddis
Copy link
Contributor Author

dbwiddis commented Oct 21, 2022

It seems to me that Windows doesn't use swap until committed memory exceeds total physical memory, that would be weird...

I would think Windows might put rarely-accessed pages on disk in order to keep faster RAM available for new programs to start up. Right now in my Windows VM I have 12% swap file usage with only 40% "physical" memory usage.

But I do get the point that "reserved" shouldn't mean "used". I'm flipping this back to draft to just ignore the Commit Total and use WMI PDH for the percent swap.

@dbwiddis
Copy link
Contributor Author

@giampaolo I could really use some help here. I'm trying to return a (double) float from the method and use it in a calculation to multiply by an integer and then round back to an integer. My total python experience is self-taught for about 2-3 weeks of hacking around.

I'm confident in the approach here but not the coding details with the casting, rounding, etc.

@giampaolo
Copy link
Owner

giampaolo commented Oct 21, 2022

I'm trying to return a (double) float from the method and use it in a calculation to multiply by an integer and then round back to an integer.

Mmm I'm not sure I understand. To round in python you do:

>>> round(1.423, 1)
1.4

As for the C types: https://docs.python.org/3/c-api/arg.html#numbers

FYI I will be on vacation for the next 7 days (with no laptop) so I won't be able to code for a while. =)

@Danstiv
Copy link

Danstiv commented Oct 21, 2022

Can't build on Windows.

Error
psutil/_psutil_windows.c(42): error C2085: 'TimeoutExpired': not in formal parameter list
psutil/_psutil_windows.c(43): error C2085: 'TimeoutAbandoned': not in formal parameter list
psutil/_psutil_windows.c(51): error C2085: 'psutil_boot_time': not in formal parameter list
psutil/_psutil_windows.c(51): error C2143: syntax error: missing ';' before '{'
psutil/_psutil_windows.c(195): error C2065: 'TimeoutExpired': undeclared identifier
psutil/_psutil_windows.c(195): warning C4047: 'function': 'PyObject *' differs in levels of indirection from 'int'
psutil/_psutil_windows.c(195): warning C4024: 'PyErr_SetString': different types for formal and actual parameter 1
psutil/_psutil_windows.c(202): error C2065: 'TimeoutAbandoned': undeclared identifier
psutil/_psutil_windows.c(202): warning C4047: 'function': 'PyObject *' differs in levels of indirection from 'int'
psutil/_psutil_windows.c(202): warning C4024: 'PyErr_SetString': different types for formal and actual parameter 1
psutil/_psutil_windows.c(1591): error C2065: 'psutil_boot_time': undeclared identifier
psutil/_psutil_windows.c(1591): error C2099: initializer is not a constant
psutil/_psutil_windows.c(1591): warning C4047: 'initializing': 'PyCFunction' differs in levels of indirection from 'int'psutil/_psutil_windows.c(1700): error C2065: 'TimeoutExpired': undeclared identifier
psutil/_psutil_windows.c(1701): warning C4047: '=': 'int' differs in levels of indirection from 'PyObject *'
psutil/_psutil_windows.c(1702): error C2065: 'TimeoutExpired': undeclared identifier
psutil/_psutil_windows.c(1703): error C2065: 'TimeoutExpired': undeclared identifier
psutil/_psutil_windows.c(1703): warning C4047: 'function': 'PyObject *' differs in levels of indirection from 'int'
psutil/_psutil_windows.c(1703): warning C4024: 'PyModule_AddObject': different types for formal and actual parameter 3
psutil/_psutil_windows.c(1705): error C2065: 'TimeoutAbandoned': undeclared identifier
psutil/_psutil_windows.c(1706): warning C4047: '=': 'int' differs in levels of indirection from 'PyObject *'
psutil/_psutil_windows.c(1707): error C2065: 'TimeoutAbandoned': undeclared identifier
psutil/_psutil_windows.c(1708): error C2065: 'TimeoutAbandoned': undeclared identifier
psutil/_psutil_windows.c(1708): warning C4047: 'function': 'PyObject *' differs in levels of indirection from 'int'
psutil/_psutil_windows.c(1708): warning C4024: 'PyModule_AddObject': different types for formal and actual parameter 3
error: command 'C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Community\\VC\\Tools\\MSVC\\14.29.30037\\bin\\HostX86\\x86\\cl.exe' failed with exit code 2

@dbwiddis
Copy link
Contributor Author

Can't build on Windows.

Yeah, I know. I can see the same in appveyor and am fumbling around here....

@Danstiv
Copy link

Danstiv commented Oct 21, 2022

Unsuccessfully 🙂

@dbwiddis
Copy link
Contributor Author

I have no idea why there are compile errors in _psutil_windows.c when I haven't made any changes to that file.

@dbwiddis
Copy link
Contributor Author

Oh, duh, a missing semicolon in wmi.h..... 🙄

@Danstiv
Copy link

Danstiv commented Oct 21, 2022

>>> psutil.swap_memory()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Python310\lib\site-packages\psutil-5.9.4-py3.10-win32.egg\psutil\__init__.py", line 1987, in swap_memory
    return _psplatform.swap_memory()
  File "C:\Python310\lib\site-packages\psutil-5.9.4-py3.10-win32.egg\psutil\_pswindows.py", line 256, in swap_memory
    percentswap = cext.percentswap()
AttributeError: module 'psutil_windows' has no attribute 'percentswap'

Good night, I will continue participation tomorrow 😩

@dbwiddis
Copy link
Contributor Author

AttributeError: module 'psutil_windows' has no attribute 'percentswap'

Yeah I deleted that while troubleshooting the missing semicolon 🙄

@dbwiddis dbwiddis force-pushed the zero-swap branch 4 times, most recently from 05ec114 to 366b4e1 Compare October 21, 2022 23:01
@dbwiddis dbwiddis marked this pull request as ready for review October 21, 2022 23:07
@dbwiddis dbwiddis changed the title Handle mismatch between committed vs. used physical memory Get Windows percent swap usage from performance counters Oct 21, 2022
@dbwiddis
Copy link
Contributor Author

I'm not 100% happy that used and free values are estimates calculated from the percent value because that means they are not gonna be 100% precise, but I guess it's a good compromise anyway.

I have a very strong belief (but no proof) that the performance counter percentage calculation comes from the same data source as #2161. The fact that both usage and peak usage are present and that the denominator of the calculation matches page file size means the other numbers would have to match. Any inaccuracy might be with the intermediate floating points but the numbers are going to be in block sizes anyway so we could be certain of exact rounding if we chose.

dbwiddis and others added 13 commits April 11, 2023 20:34
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
@dbwiddis
Copy link
Contributor Author

I'm not 100% happy that used and free values are estimates calculated from the percent value because that means they are not gonna be 100% precise, but I guess it's a good compromise anyway.

FYI, I was looking at #2161 and you have a test case (with exact match) from the WMI tables backing the performance counters. So this somewhat "proves" the equivalence in terms of data source.

An alternative, more precise method would be #2161, but @Danstiv reported that on his machine that PR fails with ERROR_NOACCESS, so that's a no-go. I guess in the future I will want to modify code so that it tries #2161 first, and fallback on #2160 if it fails, so we have the best of both worlds

This seems reasonable.

As mentioned before, the only lack of precision is taking an integer division. I could avoid the float, however, by fetching integer values. The "Formatted" counter values give the double, but if I fetch the "Raw" counter values they would give exactly the same integers as in that WMI query we are both using as a test case, which I think are in units of page size (4096 I think). That would still give us the benefits of the PDH API lookup and avoid losing any precision with floating points.

I have one request: could you please move this new code into psutil/arch/windows/mem.c file? (I've just created it). Thanks

I think I've addressed all your requests. There's some transient failure on macos unassociated with my changes.

psutil/_psutil_windows.c Outdated Show resolved Hide resolved
psutil/_pswindows.py Outdated Show resolved Hide resolved
Signed-off-by: Daniel Widdis <widdis@gmail.com>
fail if PdhGetFormattedCounterValue fails
@giampaolo
Copy link
Owner

Thanks a lot Daniel. I made a small edit to your changes: if PdhGetFormattedCounterValue fails I raised exception instead of setting percent to 0. I did that because error on PdhGetFormattedCounterValue should indicate some sort of problem with the handle or the value, and not a disabled swap, so it's better to crash and be notified.

@dbwiddis
Copy link
Contributor Author

I made a small edit to your changes

Thanks! Yeah, I agree this is the right approach.

Also thanks for fixing up formatting. I don't normally write code in c or python so don't have the usual set of IDE / linter tools. Glad the substance of the code change is useful!

@giampaolo giampaolo merged commit 0dde184 into giampaolo:master Apr 13, 2023
10 of 11 checks passed
@giampaolo
Copy link
Owner

Finally merged. Thanks and see you around!

@dbwiddis dbwiddis deleted the zero-swap branch April 14, 2023 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Windows] Swap used is bigger than swap total
5 participants