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

Change PI cpu usage to match widget usage #3171

Merged
merged 1 commit into from
Jun 7, 2024
Merged

Conversation

dkbennett
Copy link
Member

Summary of the pull request

Changes the CPU usage calculation in Project Ironsides to match that of the CPU widget. Both are now very close to task manager's reporting, within reasonable expectations given different polling times and that one monitor's execution can affect the CPU and thereby change the other monitors' results.

References and relevant issues

Closes #3160

Detailed description of the pull request / Additional comments

The values are unlikely to match exactly due to different polling times. Task manager, the widget, and PI poll at 1s intervals, but even slight differences in the time they poll can give different results. Since PerformanceCounter.NextValue() is actually itself rather heavy on CPU, one monitor can affect other monitors. For this reason, two components can use the same perf counter but have slightly different results, so we do not expect them to match precisely. We do expect them to be reasonably close over time.

In analyzing which metric was most correct, I compared Task Manager CPU utilization to the CPU widget utilization and found them to be very close, producing a very similar graph and numbers that were often the same. Since it appears these two are consistent with each other, I changed the PI CPU usage to match the CPU widget's perf counter category, name, and calculation.

Validation steps performed

  • Built and ran, compared Task Manager, the CPU widget, and PI's perf usage. They are now all reasonably close.

PR checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated

@dkbennett dkbennett merged commit 51b856c into main Jun 7, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants