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

tweak(conhost-v2): switch CPU utilization tracking to dynamic scaling #2953

Merged

Conversation

tens0rfl0w
Copy link
Contributor

Goal of this PR

Starting with Windows 8, Microsoft updated CPU performance counters to use "Processor Utility" instead of "Processor Time" to reflect dynamic frequency scaling (e.g., Intel Turbo Boost, AMD Precision Boost). This causes the in-game metrics to display wrong CPU utilization values.

How is this PR achieving the goal

This change ensures in-game metrics display those more accurate values.

This PR applies to the following area(s)

FiveM, RedM

Successfully tested on

Game builds: Not applicable

Platforms: Windows (Client)

Checklist

  • Code compiles and has been tested successfully.
  • Code explains itself well and/or is documented.
  • My commit message explains what the changes do and what they are for.
  • No extra compilation warnings are added by these changes.

Fixes issues

Reported here: https://discord.com/channels/779705925577080842/1260757053635297381 (Cfx.re Engineering Group)

Starting with Windows 8, Microsoft updated CPU performance counters to use "Processor Utility" instead of "Processor Time" to reflect dynamic frequency scaling (e.g., Intel Turbo Boost, AMD Precision Boost). This change ensures in-game metrics display those more accurate values.
@github-actions github-actions bot added RedM Issues/PRs related to RedM invalid Requires changes before it's considered valid and can be (re)triaged labels Nov 21, 2024
@iridium-cfx
Copy link
Contributor

I suppose this depends on what you class as the right/wrong values.

Does this now match what you see in task manager?

@tens0rfl0w
Copy link
Contributor Author

Yes, when this change was introduced in Win8, MSFT also changed the metrics source for CPU utilization in the task manager to this new metric. (https://learn.microsoft.com/en-us/troubleshoot/windows-client/performance/cpu-usage-exceeds-100)

Copy link
Contributor

@iridium-cfx iridium-cfx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I'd agree it makes sense for the average user to try and match what task manager says.

@iridium-cfx iridium-cfx added ready-to-merge This PR is enqueued for merging and removed invalid Requires changes before it's considered valid and can be (re)triaged RedM Issues/PRs related to RedM labels Nov 21, 2024
@IllInuz
Copy link

IllInuz commented Nov 21, 2024

Yeah this makes more sense to the average user i would say, this has also been done by triple a's to avoid the confusion.

@prikolium-cfx prikolium-cfx merged commit 2c5ca84 into citizenfx:master Nov 25, 2024
3 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR is enqueued for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants