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

Fix ps command CPU usage on Apple Silicon M1 macs. #4142 #6457

Merged
merged 3 commits into from
Sep 1, 2022
Merged

Fix ps command CPU usage on Apple Silicon M1 macs. #4142 #6457

merged 3 commits into from
Sep 1, 2022

Conversation

obaudys
Copy link
Contributor

@obaudys obaudys commented Sep 1, 2022

Description

The cpu user and system times returned by libproc are not in
nanoseconds; they are in mach ticks units. This is not documented very
well. The convert from mach ticks to ns, the kernel provides a timebase
info function and datatype:

(https://developer.apple.com/documentation/driverkit/3433733-mach_timebase_info)

The commit makes the PS command work for me.

image

Tests

Existing tests should cover this just fine. However it would be good to get confirmation from someone with an Intel mac to see if ps output makes sense.

Ondrej Baudys added 3 commits September 1, 2022 14:19
The cpu user and system times returned my libproc are not in
nanoseconds; they are in mach ticks units.  This is not documented very
well.  The convert from mach ticks to ns, the kernel provides a timebase
info function and datatype:

https://developer.apple.com/documentation/driverkit/3433733-mach_timebase_info

The commit makes the PS command work for me.
@fdncred
Copy link
Collaborator

fdncred commented Sep 1, 2022

oh wow. it's so nice to have cpu utilization again. thanks so much for this. now, if we can only fix the virtual memory, why oh why is everything ~400-~500GB? it's not like that in activity monitor. I wonder if it's the same sort of thing?

@obaudys
Copy link
Contributor Author

obaudys commented Sep 1, 2022

if we can only fix the virtual memory, why oh why is everything ~400-~500GB?

As far as I can see, the data for virtual memory size is populated by the C library function proc_pidinfo, which is a wrapper around a syscall __proc_info. So, I'll say it is accurate, and the native /bin/ps utility's "rss" field matches nushell's "mem":

image

Funnily enough my Google Chrome worker processes have 1.5TB mapped into their address space, apparently. Both nushell and /bin/ps agree on this.

Activity Monitor does not seem to report the the rss field unless you enable the "Real Mem" column, and I have no idea how it actually calculates the "Memory" column:

image

@fdncred
Copy link
Collaborator

fdncred commented Sep 1, 2022

@obaudys When I compared activity monitor to nushell's ps command earlier this morning. The virtual memory was way off, but maybe I was looking at the wrong column. All I saw was 95% (or greater) of my processes were using over 400GB of virtual memory. I'll take a closer look later this afternoon like you suggest. Thanks for the info.

@rgwood
Copy link
Contributor

rgwood commented Sep 1, 2022

Thanks @obaudys! Really appreciate these kinds of platform-specific fixes :)

@obaudys obaudys deleted the M1-ps-cputime-fix branch September 1, 2022 23:29
dheater pushed a commit to dheater/nushell that referenced this pull request Sep 2, 2022
…ell#6457)

* Fix ps command CPU usage on Apple Silicon M1 macs. nushell#4142

The cpu user and system times returned my libproc are not in
nanoseconds; they are in mach ticks units.  This is not documented very
well.  The convert from mach ticks to ns, the kernel provides a timebase
info function and datatype:

https://developer.apple.com/documentation/driverkit/3433733-mach_timebase_info

The commit makes the PS command work for me.

* Cargo fmt of previous commit.

* Clippy format suggestion

Co-authored-by: Ondrej Baudys <ondrej.baudys@nextgen.net>
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.

4 participants