-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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] Swap used is bigger than swap total #2074
[Windows] Swap used is bigger than swap total #2074
Comments
The negative value for free is probably a result of the calculation in #1927. Root cause is probably that these numbers are fetched from I previously mentioned this in #1921 (comment) |
Thanks @dbwiddis. Feel free to make a PR if you want. I'm not sure when I'll have time to get to this. |
Submitting this as a draft PR as I believe it will address giampaolo#2074. I do not have either a C or Python development environment to test these changes, so they are provided in the hopes someone else can test and confirm they fix the issue. There's probably some room to simplify the code with temporary variables to reduce redundancy. For background, I have implemented precisely this logic in my own (Java-based) project [here](https://github.com/oshi/oshi/blob/3bb9eafbe2062edad4108b7b12501b1f9be27361/oshi-core/src/main/java/oshi/hardware/platform/windows/WindowsVirtualMemory.java#L110-L118) and [here](https://github.com/oshi/oshi/blob/3bb9eafbe2062edad4108b7b12501b1f9be27361/oshi-core/src/main/java/oshi/hardware/platform/windows/WindowsGlobalMemory.java#L253-L263) following a detailed investigation in oshi/oshi#1182
It seems that I have a similar issue using "Python version: 3.10.4 64Bit on Windows 11 pro 64Bit Version 21H2" |
Hello! |
Submitting this as a draft PR as I believe it will address giampaolo#2074. I do not have either a C or Python development environment to test these changes, so they are provided in the hopes someone else can test and confirm they fix the issue. There's probably some room to simplify the code with temporary variables to reduce redundancy. For background, I have implemented precisely this logic in my own (Java-based) project [here](https://github.com/oshi/oshi/blob/3bb9eafbe2062edad4108b7b12501b1f9be27361/oshi-core/src/main/java/oshi/hardware/platform/windows/WindowsVirtualMemory.java#L110-L118) and [here](https://github.com/oshi/oshi/blob/3bb9eafbe2062edad4108b7b12501b1f9be27361/oshi-core/src/main/java/oshi/hardware/platform/windows/WindowsGlobalMemory.java#L253-L263) following a detailed investigation in oshi/oshi#1182 Signed-off-by: Daniel Widdis <[email protected]>
Submitting this as a draft PR as I believe it will address giampaolo#2074. I do not have either a C or Python development environment to test these changes, so they are provided in the hopes someone else can test and confirm they fix the issue. There's probably some room to simplify the code with temporary variables to reduce redundancy. For background, I have implemented precisely this logic in my own (Java-based) project [here](https://github.com/oshi/oshi/blob/3bb9eafbe2062edad4108b7b12501b1f9be27361/oshi-core/src/main/java/oshi/hardware/platform/windows/WindowsVirtualMemory.java#L110-L118) and [here](https://github.com/oshi/oshi/blob/3bb9eafbe2062edad4108b7b12501b1f9be27361/oshi-core/src/main/java/oshi/hardware/platform/windows/WindowsGlobalMemory.java#L253-L263) following a detailed investigation in oshi/oshi#1182 Signed-off-by: Daniel Widdis <[email protected]>
Submitting this as a draft PR as I believe it will address giampaolo#2074. I do not have either a C or Python development environment to test these changes, so they are provided in the hopes someone else can test and confirm they fix the issue. There's probably some room to simplify the code with temporary variables to reduce redundancy. For background, I have implemented precisely this logic in my own (Java-based) project [here](https://github.com/oshi/oshi/blob/3bb9eafbe2062edad4108b7b12501b1f9be27361/oshi-core/src/main/java/oshi/hardware/platform/windows/WindowsVirtualMemory.java#L110-L118) and [here](https://github.com/oshi/oshi/blob/3bb9eafbe2062edad4108b7b12501b1f9be27361/oshi-core/src/main/java/oshi/hardware/platform/windows/WindowsGlobalMemory.java#L253-L263) following a detailed investigation in oshi/oshi#1182 Signed-off-by: Daniel Widdis <[email protected]>
* Use system-level values for Windows virtual memory Submitting this as a draft PR as I believe it will address #2074. I do not have either a C or Python development environment to test these changes, so they are provided in the hopes someone else can test and confirm they fix the issue. There's probably some room to simplify the code with temporary variables to reduce redundancy. For background, I have implemented precisely this logic in my own (Java-based) project [here](https://github.com/oshi/oshi/blob/3bb9eafbe2062edad4108b7b12501b1f9be27361/oshi-core/src/main/java/oshi/hardware/platform/windows/WindowsVirtualMemory.java#L110-L118) and [here](https://github.com/oshi/oshi/blob/3bb9eafbe2062edad4108b7b12501b1f9be27361/oshi-core/src/main/java/oshi/hardware/platform/windows/WindowsGlobalMemory.java#L253-L263) following a detailed investigation in oshi/oshi#1182 Signed-off-by: Daniel Widdis <[email protected]> * Formatting, credits, change log Signed-off-by: Daniel Widdis <[email protected]> Signed-off-by: Daniel Widdis <[email protected]>
This didn't solve my problem. |
Hello. With swap be turned on, i am getting this result.
I don't think this is information we are expecting to see. |
More detailed and interesting example.
|
Can you also show the physical memory values? |
That's what I managed to find out, I hope I understand everything correctly.
And these are the results from another machine with 32 gb RAM and enabled swap.
Later, in 5.9.0 or earlier, this behavior was changed, and psutil.swap memory began to return incorrect results both with and without swap enabled (probably after mergeing pr #1927).
This solution is not good enough, since you need to add a separate function, test it, document it, etc., but it will finally solve all the problems with incorrect data that arise. |
|
Hey @Danstiv thanks for the detailed explanation. Let me try to add a bit more detail and suggestions here.
This was broken, as the values being called "swap" were, in reality, "swap + physical". The behavior prior to my PR merged yesterday was based on MEMORYSTATUSEX and the numbers reported for "swap" were:
The problem with these numbers is that they are "for the system or the current process, whichever is smaller". The change in #2077 did precisely what these docs suggested for the "system-wide" value, calling totalSys = perfInfo.CommitLimit * pageSize;
availSys = totalSys - perfInfo.CommitTotal * pageSize;
The values are there and can easily be recovered, if desired, by adding the physical and swap values. The
And the calculation of the values psutil returns is: mem = cext.virtual_mem()
totphys, availphys, totsys, availsys = mem The physical memory part seems to be working. Here's the calculation for swap, which went in in #1927. mem = cext.virtual_mem()
total_phys = mem[0]
free_phys = mem[1]
total_system = mem[2]
free_system = mem[3]
# system memory (commit total/limit) is the sum of physical and swap
# thus physical memory values need to be substracted to get swap values
total = total_system - total_phys
free = min(total, free_system - free_phys)
used = total - free Where you can simply back out the subtraction by adding total_phys back to total (thus commit limit exactly equals physical total, as expected). And for commit total, we can take the "free" value and add back in free_phys. So for the example above:
This indicates the All this is a long way of saying "reverding to the old behavior gains us nothing".
This is unneeded for the same reason. You can back into the calculation with the given numbers. However, the second part of option 2 is good:
The "Otherwise" calculation is exactly what's being done, with the exception of the I think, ultimately, we just need to change this one line: free = min(total, free_system - free_phys) to free = max(0, min(total, free_system - free_phys)) |
Also, the root cause of this is the edge case where Commit Total is incremented before Physical Available is decremented:
That instantaneous change to commit total can cause it to exceed physical "used" and in the edge case of "zero swap" results in the negative values we are seeing. |
This bug aside, an important thing that we need and which is currently missing is a unit test which compares psutil/psutil/tests/test_windows.py Lines 121 to 124 in 55b588a
Maybe we can do the same for swap mem by using WMI / Win32_PageFile? Is "pagefile" and "swap memory" the same thing on Windows? (I'm not sure) |
I'm working on another PR with this change to the # system memory (commit total/limit) is the sum of physical and swap
# thus physical memory values need to be substracted to get swap values
total = total_system - total_phys
# commit total is incremented immediately (decrementing free_system)
# while the corresponding free physical value is not decremented until
# pages are accessed, so free_phys is an overestimate of the portion
# free_system contributed to by physical memory, and in some edge cases
# can exceed free system memory.
free = max(0, free_system - free_phys)
used = total - free I'll submit as draft for now and look into the unit test options. There are also performance counters which may be relevant. |
Ok, that's cool. |
Win32_PageFile appears deprecated and the Win32_PageFileUsage class is in units of pages (and I can't see an easy place to get page size). But the perfmon counters and associated WMI class look like a good alternative here. I think the commit limit minus the physical total should equal swap file size for a unit test. The committed bytes should be "close to" the commit total value.
Yes, it is, but the way limits are handled are different between Windows and Linux. Linux "guesses" whether there is swap available and never actually uses it all, using a "swappiness" property to control the level of "risk". Windows enforces it strictly by reserving the worst case scenario and expanding the paging file (swap file) if there is any chance it will be needed. Ultimately, though, the negative calculation is more of a human-readable interpretation separating the (accurate) commit total/limit vs. (accurate within a different context) physical total/used. Ultimately, "swap used" is an overestimate and "swap available" is an underestimate compared to a similar situation on Linux. |
I added several more tests in the PR... may be out of order at this point after the merge... |
Actually, @Danstiv, with the conditionals ( In my java-based project I separately report "swap" values (using the perf counter percent calculation) and also report "virtual memory" values which are the sum of physical + swap and use the commit limit. Total virtual memory is actually what the operating system(s) use; it's transparent to userspace whether the memory being written to is on disk or in RAM, it's just "virtual memory". So the "old values" were closer to this interpretation, but not consistent with the "swap file" calculations. The swap file size is a somewhat accurate interpretation here, but the whole calculation breaks down when trying to use the "Commit Total" to calculate swap usage.... see this comment on my PR. The performance counters do expose a "percent swap used" which I've added in the tests, and perhaps we should just multiply that fraction by the total swap size (which is an accurate calculation). But if we do that we lose the information in the "Commit Total" which includes reserved physical memory as well. @giampaolo what are your thoughts here? |
I must admit I am confused, also because it's a lot of information to process + I'm not good at math hehe. psutil/psutil/arch/windows/disk.c Line 367 in 2974941
This is a completely different approach but it seems to me that theoretically it could work. |
Are you suggesting to infer Note: we cannot use the wmi python module, as that's a third-party dep used by unit-tests only. |
Yes. I don't think it's available elsewhere.
Well, drat.
We don't need to use WMI, it's just a convenient front-end to the performance counter api (PDH) which I've seen used elsewhere, e.g.: psutil/psutil/arch/windows/wmi.c Line 40 in 3398175
|
@dbwiddis take a look at #2161. With this I'm able to get the same values I get by running |
@dbwiddis interesting comment in zabbix source code:
|
Looks like we have competing PRs for the same information using completely different methods. I wonder how they compare to each other! That comment you quoted looks like what we are already doing to calculate (total) swap size by taking the Commit Limit and subtracting physical memory. This part is working just fine. The part that's not working is assuming the reserved portion of the "commit total" (used + reserved) is all on the swap file. I've updated my PR with a counter-based approach using % Paging File. I'm confident in this approach as I use it on my own project and it seems to match other displays. (I'm less confident in my C and Python usage!) It looks like your PR provides a different method but I see it may run afoul of permissions issues... |
Signed-off-by: Daniel Widdis <[email protected]>
* 'master' of https://github.com/giampaolo/psutil: add windows test for free physical mem giampaolo#2074 fix OSX tests broken by accident update HISTORY + give CREDITS for @arossert, @smoofra, @mayeut for giampaolo#2102, giampaolo#2156, giampaolo#2010 build fix for Mac OS, Apple Silicon (giampaolo#2010) Linux: fix missing SPEED_UNKNOWN definition (giampaolo#2156) Use system-level values for Windows virtual memory (giampaolo#2077) feature: use ABI3 for cp36+ (giampaolo#2102) fix py2 compatibility improve API speed benchmark script giampaolo#2102 fix: linter issues from giampaolo#2146 (giampaolo#2155) chore: skip test_cpu_freq on macOS arm64 (giampaolo#2146) pre-release + give CREDITS to @mayeut (PR giampaolo#2040) and @eallrich (new supporter) Fix a typo (giampaolo#2047) move isort and coverage config into pyproject.toml fix giampaolo#2021, fix giampaolo#1954, provide OSX arm64 bins + add pyproject.toml (giampaolo#2040) refactor git_log.py
Signed-off-by: Daniel Widdis <[email protected]>
Summary
Description
The text was updated successfully, but these errors were encountered: