Timebase

[odormond]

Summary

• OS: macOS 12.0.1, 11.6.1
• Bug fix: yes
• Type: core
• Fixes: [macOS / M1] cpu_percent() gives incorrect information #1956

Description

Mach sys calls report cpu time in "tick units" -- on intel cpus, these happen to be nsecs but on M1 machines they're about 40 nsecs. This PR corrects for this, but I think it still needs two improvements: the value of mach_timebase_info should be cached somewhere, and there are probably other places where the code depends on 1 tick=1 nsec that also need to be updated. Sounds reasonable? I can work on both of those, but I wanted to get some feedback first before I spend any more time on this.

Based on @rmalouf PR #1958.

[giampaolo]

the value of mach_timebase_info should be cached somewhere

Makes sense. You can do that in psutil_setup() function, which is called on module init:

psutil/psutil/_psutil_common.c

Line 395 in 8fa1746

psutil_setup(void) {

You should define the new global var as a C extern defined in _psutil_common.h. grep for PSUTIL_SYSTEM_INFO to see how it's done.

[giampaolo]

Since you did the new conversion above, is the / 1000000000.0 division still necessary?

[odormond]

Yes. The mach timebase conversion ratio produces nanoseconds.

[odormond]

the value of mach_timebase_info should be cached somewhere

Makes sense. You can do that in psutil_setup() function, which is called on module init:

psutil/psutil/_psutil_common.c

Line 395 in 8fa1746

psutil_setup(void) {

You should define the new global var as a C extern defined in _psutil_common.h. grep for PSUTIL_SYSTEM_INFO to see how it's done.

Sure. I just pushed another commit doing just that. If it's good for you I can squash it to replace the previous two.

About the division by a billion, the mach_timebase_info give the conversion ratio from ticks to nanoseconds so the result still needs to be converted to seconds.

[giampaolo]

In order to be consistent with PSUTIL_SYSTEM_INFO, PSUTIL_CRITICAL_SECTION and PSUTIL_DEBUG I suggest to:

• call this PSUTIL_MACH_TIMEBASE_INFO
• define it in _psutil_common.h file

[odormond]

In order to be consistent with PSUTIL_SYSTEM_INFO, PSUTIL_CRITICAL_SECTION and PSUTIL_DEBUG I suggest to:

* call this `PSUTIL_MACH_TIMEBASE_INFO`

That makes more sense indeed. Sorry.

* define it in `_psutil_common.h` file

I don't understand this. The header file does have the extern definition and storage can only be in the .c file. What am I missing?

[giampaolo]

Merged. Thanks!