-
Notifications
You must be signed in to change notification settings - Fork 159
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 for M3 Max - Using average of cores instead of relying on cluster idle ratio #68
base: main
Are you sure you want to change the base?
Conversation
please @tlkh merge this one 🙏 |
@gavi Thank you so much! Your fix worked for me. |
I am not sure what to trust though. If you look at the plist file that is generated by |
Using a M3 Max here. The readings with this PR seem OK when the P-cores are actives, but they are overestimated when they are not. I think it is because the calculations do not take into account the P-Clusters and the P-Cores can also be down a part of the time, and that must be added to the "idle" time. I got something more plausible when I replace line 97
with this:
|
@phgrosjean I think your change is much better. I am updating my PR |
Oh yes awesome. I noticed the down CPUs counter last night when I ran
powermetrics manually to understand the output. Didn’t know how to account
for them yet.
I did notice that the graph looks more accurate when I stress the computer
to actually use the P-cores and that could explain what you just wrote.
Will try soon. Thank you!
…On Wed, Jan 10, 2024 at 12:03 PM Philippe Grosjean ***@***.***> wrote:
Using a M3 Max here. The readings with this PR seem OK when the P-cores
are actives, but they are overestimated when they are not. I think it is
because the calculations do not take into account the P-Clusters and the
P-Cores *can also be down* a part of the time, and that must be added to
the "idle" time. I got something more plausible when I replace line 97
total_idle_ratio += cpu["idle_ratio"]
with this:
total_idle_ratio += cluster["down_ratio"] + (1 - cluster["down_ratio"]) * (cpu["idle_ratio"] + cpu["down_ratio"])
—
Reply to this email directly, view it on GitHub
<#68 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADLJ5PU5N7PDC26IYKVG7H3YN3CUPAVCNFSM6AAAAABBGYINLCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBVGI2DMNJQGI>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
I updated the PR |
@gavi thanks for the fix! would be great to see this merged. |
For anyone who are not sure how to port this changes to your own machine, just replace the |
|
Looking forward to this being merged and released, thanks @gavi ! |
Not likely, seems the repo author is off the planet to Mars. |
Seems like the idle ratio of Cluster is reporting as zero in
powermetrics
which might be causing the CPU gauges to show 100%. This code fix simply averages the individual core idle ratios.