Skip to content

Implement getMaxRss for Windows #15035

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

Merged
merged 4 commits into from
Mar 24, 2023
Merged

Implement getMaxRss for Windows #15035

merged 4 commits into from
Mar 24, 2023

Conversation

xEgoist
Copy link
Contributor

@xEgoist xEgoist commented Mar 21, 2023

closes #14956

As stated in the commit message, this is done using K32GetProcessMemoryInfo to get PeakWorkingSetSize.

Since GetProcessMemoryInfo is used in Psapi.dll (Or Kernel32.dll), I will try to issue a pull request later for a ntdll refactor to this function.

Edit: Switched to kernel32 for compatibility + psapi calls kernel32 anyway.

@xEgoist xEgoist force-pushed the windowsMaxRss branch 3 times, most recently from 6401747 to 4600d2f Compare March 21, 2023 17:11
@kubkon
Copy link
Member

kubkon commented Mar 21, 2023

Thanks for the PR! As I explained in #15039 do you think you could try implementing the kernel32 function yourself using only calls to ntdll?

@xEgoist
Copy link
Contributor Author

xEgoist commented Mar 21, 2023

Thanks for the PR! As I explained in #15039 do you think you could try implementing the kernel32 function yourself using only calls to ntdll?

Certainly. I wasn't initially sure if I should include it in this PR, but I was going to implement GetProcessMemoryInfo in ntdll later this week. But it is reasonable to include both as there is not really much refactor to be done.

Will update this PR as soon as I implement it.

xEgoist added 2 commits March 22, 2023 06:13
In Windows, the equivalent to maxrss is PeakWorkingSetSize which is
found in PROCESS_MEMORY_COUNTERS in bytes.

Currently, this is done by calling `GetProcessMemoryInfo` in kernel32.
`GetProcessMemoryInfo` is implemented using `NtQueryInformationProcess`
with `ProcessVmCounters` to obtain `VM_COUNTERS`. The structs, enum
definitions are found in `winternl.h` or `ntddk.h` in the latest WDK.
This should give the same results as using `K32GetProcessMemoryInfo`
@xEgoist
Copy link
Contributor Author

xEgoist commented Mar 22, 2023

I tested the GetProcessMemoryInfo implementation using this code:

const std = @import("std");
const w = std.os.windows;

pub fn main() !void {

    const stdout_file = std.io.getStdOut().writer();
    var bw = std.io.bufferedWriter(stdout_file);
    const stdout = bw.writer();
    
    const pHandle = w.kernel32.GetCurrentProcess();
    var pmc: w.PROCESS_MEMORY_COUNTERS = undefined;
    var pmc2: w.PROCESS_MEMORY_COUNTERS = undefined;

    if (w.kernel32.K32GetProcessMemoryInfo(pHandle, &pmc, @sizeOf(w.PROCESS_MEMORY_COUNTERS)) != 0) {
        try stdout.print("K32: MaxRss is {} Bytes!\n", .{pmc.PeakWorkingSetSize});
    }
    else  {
        try stdout.print("Something Went Wrong!\n", .{});
    }

    try w.GetProcessMemoryInfo(pHandle, &pmc2);
    try stdout.print("Nt: MaxRss is {} Bytes!\n", .{pmc2.PeakWorkingSetSize}); 


    if (w.kernel32.K32GetProcessMemoryInfo(pHandle, &pmc, @sizeOf(w.PROCESS_MEMORY_COUNTERS)) != 0) {
        try stdout.print("K32 Again: MaxRss is {} Bytes!\n", .{pmc.PeakWorkingSetSize});
    } 
    else  {
        try stdout.print("Something Went Wrong!\n", .{});
    }
    try bw.flush();
}

Both functions reported the same MaxRss.

This change allows the function to return the process memory info
directly instead of copying the result of the underlying Nt function.
@kubkon
Copy link
Member

kubkon commented Mar 23, 2023

Could you take a final look at this @squeek502 and if you are happy with this I'll merge it?

@squeek502
Copy link
Collaborator

Looks good to me 👍

@kubkon kubkon merged commit 3aa0a7e into ziglang:master Mar 24, 2023
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.

add Windows support for detecting maxrss of child process
3 participants