incorrect virtual memory?

[fdncred]

sysinfo/src/windows/process.rs

Line 782 in 639f6af

p.virtual_memory = (pmc.PrivateUsage as u64) / 1_000;

I'm guessing that this code may not be right because in nushell it's showing that i have terabytes of virtual memory, which i don't have. nushell needs the number in bytes so we're doing something like this process.virtual_memory() * 1000 to get bytes.

Do you think nushell is reporting it the wrong way or is it being calculated the wrong way in sysinfo?

[GuillaumeGomez]

The doc says that virtual_memory returns the size in KB as you can see here. So it depends what size nushell wants to display.

[fdncred]

Thanks.

[GuillaumeGomez]

Well, the initial question remains: if they intend to display bytes and not kilobytes and you still have terabytes, then there is a problem in sysinfo. If you could maybe confirm? :)

[fdncred]

This is where we're tracking this issue. Your example program seems to be reporting differently than we're reporting in nushell. I'm not sure why yet. I'll try to get back to you on this.

nushell/nushell#3003

I really don't get why it's not working because this is all we're doing.

dict.insert_untagged(
            "virtual",
            UntaggedValue::filesize(process.virtual_memory() * 1000),
        );

[fdncred]

@GuillaumeGomez I've gone through this a bit in our code. If you see below, it looks like we're following the model as you used in your example application. You can see where I inserted some eprintln! statements to see if our formatting is doing anything odd somewhere later. Unfortunately, it appears that process.virutal_memory() is outputting wrong information in nushell. Do you see anything odd in the code snippet below?

pub async fn ps(tag: Tag, long: bool) -> Result<Vec<Value>, ShellError> {
    let mut sys = System::new_all();
    sys.refresh_all();
    let duration = std::time::Duration::from_millis(500);
    std::thread::sleep(duration);
    sys.refresh_all();

    let mut output = vec![];

    let result = sys.get_processes();

    for (pid, process) in result.iter() {
        let mut dict = TaggedDictBuilder::new(&tag);
        dict.insert_untagged("pid", UntaggedValue::int(*pid));
        dict.insert_untagged("name", UntaggedValue::string(process.name()));
        eprintln!("myname {}", process.name());
        dict.insert_untagged(
            "status",
            UntaggedValue::string(format!("{:?}", process.status())),
        );
        dict.insert_untagged(
            "cpu",
            UntaggedValue::decimal_from_float(process.cpu_usage() as f64, tag.span),
        );
        dict.insert_untagged("mem", UntaggedValue::filesize(process.memory() * 1000));
        eprintln!("mymem {}", process.memory());
        dict.insert_untagged("virtual", UntaggedValue::filesize(process.virtual_memory()));
        eprintln!("myvm {}", process.virtual_memory());
...

I also tried sys.get_process(pid) and it had the same results. I was trying to do sys.refresh_process(pid) but I was getting compiler errors.

[GuillaumeGomez]

No, your code looks correct as far as I can tell. The issue must be coming from sysinfo. Can you maybe add some debug in sysinfo in order to track where the issue is coming from? I can't seem to replicate it on my laptop so I'll have to rely on you to find it (sorry...).

[fdncred]

Probably. What do you want me to add? When I run your example app, it works correctly.

[GuillaumeGomez]

Simply some println! around to track the virtual memory value in order to check where it goes wrong.

[fdncred]

@GuillaumeGomez Could this be the problem?

sysinfo/src/windows/system.rs

Line 258 in 6db49a8

proc_.virtual_memory = (pi.VirtualSize as u64) / 1_000;

I'm wondering if that code above should be PrivateUsage instead of VirtualSize

sysinfo/src/windows/process.rs

Line 782 in 6db49a8

p.virtual_memory = (pmc.PrivateUsage as u64) / 1_000;

[fdncred]

PrivateUsage prints the correct memory. VirtualSize is not correct. I'm not sure why. According to Microsoft The VirtualSize member contains the current size, in bytes, of virtual memory used by the process. so it seems right but the value is wrong.

[GuillaumeGomez]

I never paid attention but it seems you're right. How weird... When you change to PrivateUsage everywhere, does it look better?

[fdncred]

This seems to be working much better.

    let mut sys = System::new_all();
    let mut output = vec![];
    let result: Vec<usize> = sys.get_processes().keys().cloned().collect::<Vec<usize>>();

    for pid in result {
        sys.refresh_process(pid);
        if let Some(process) = sys.get_process(pid) {
            let mut dict = TaggedDictBuilder::new(&tag);
...
            dict.insert_untagged("mem", UntaggedValue::filesize(process.memory() * 1000));
            dict.insert_untagged(
                "virtual",
                UntaggedValue::filesize(process.virtual_memory() * 1000),
            );

sys.refresh_process(pid) eventually calls the function with PrivateUsage.

I just wish sysinfo wouldn't get the information in KB. Maybe Microsoft is doing that. I can't remember now.

[GuillaumeGomez]

I think the information is returned in KB in linux, so all other systems kinda need to return the same "level".

[GuillaumeGomez]

Do you mind sending a PR for this fix? Since you found where the problem was coming from... ;)

[fdncred]

I don't think there is anything to PR because the route I took was essentially not to do this:

let result = sys.get_processes();

    for (pid, process) in result.iter() {

because once you do that, you can't call sys.refresh_process(pid). <-- This function works to update the memory correctly on Windows. Any other way is not working.

And a worse note - I just saw that now the memory numbers on Linux may be artificially large. Ugh.

[GuillaumeGomez]

What the hell... There are a lot of things that need to be fixed apparently.

[fdncred]

I was going to try and look at it in WSL2 and see if anything jumps out.

[GuillaumeGomez]

Thanks, that'd be really appreciated. But I still think that if any path/route leads to invalid data, then there is something to fix. Can you provide a small test code so I can run it locally on my windows and try to fix it please?

[fdncred]

Something is going on in linux as well I think. mem looks ok but virutal not so much.

ps -l | where pid == 51
╭───┬─────┬──────┬────────┬────────┬────────────┬───────────────────┬────────┬───────────────┬─────────╮
│ # │ pid │ name │ status │ cpu    │ mem        │ virtual           │ parent │ exe           │ command │
├───┼─────┼──────┼────────┼────────┼────────────┼───────────────────┼────────┼───────────────┼─────────┤
│ 0 │  51 │ fish │ Sleep  │ 0.0000 │ 12,040,000 │ 5,838,651,392,000 │      9 │ /usr/bin/fish │ fish    │
╰───┴─────┴──────┴────────┴────────┴────────────┴───────────────────┴────────┴───────────────┴─────────╯

This is what virtual should be as per the man proc.

cat /proc/51/stat | cut -d" " -f23
1,459,662,848

5.8TB vs 1.4GB

[fdncred]

This is the small test code from above, tweaked a bit, that shows how it works in Windows.

    let mut sys = System::new_all();
    let mut output = vec![];
    let result: Vec<usize> = sys.get_processes().keys().cloned().collect::<Vec<usize>>();

    for pid in result {
        sys.refresh_process(pid);
        if let Some(process) = sys.get_process(pid) {
           println!("name {}", process.name());
           println!("mem {}", process.memory() * 1000);
           println!("virtual {}", process.virtual_memory() * 1000);
        }
    }

[fdncred]

I think I may see the problem in Linux.

sysinfo/src/linux/system.rs

Line 655 in a3099e0

entry.virtual_memory = u64::from_str(parts[22]).unwrap_or(0) * page_size_kb;

According to man proc

              (23) vsize  %lu
                     Virtual memory size in bytes.

It's reporting in bytes and then sysinfo is multiplying page_size_kb.

[GuillaumeGomez]

Great catch! Interested into sending a PR for this one at least?

[fdncred]

#430

[GuillaumeGomez]

I'll reopen since the issue still exists on windows (based on your comments).