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

feat: Improve metrics collected during benchmarks #2031

Merged
merged 1 commit into from
Sep 2, 2024

Conversation

maxep
Copy link
Member

@maxep maxep commented Aug 30, 2024

What and why?

Memory and CPU metrics were only reporting the last values collected after 10 s interval. It cannot provide accurate data, instead we are now collecting metrics more frequently and report the aggregated values during push.

How?

Collect CPU usage and Memory footprint using timers configured to read values every 100 ms, then report the aggregation depending on the metric type:

  • The max memory footprint
  • The average CPU usage
  • The min frame rate.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes

@maxep maxep requested review from a team as code owners August 30, 2024 15:54
Copy link
Member

@mariedm mariedm left a comment

Choose a reason for hiding this comment

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

Looks great! 🙌

General question: do you know the impact of the more frequent metric collection on the app's performance?

}
}

extension MetricAggregator where T: BinaryFloatingPoint {
Copy link
Member

Choose a reason for hiding this comment

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

I was wondering if we could merge these two extensions since the logic inside is the same, but it doesn't seem straightforward.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried, but failed :/ Problem is that there is 2 distinct Double.init that take either a BinaryInteger or a BinaryFloatingPoint. There is no common method that can help computing the avg value.

@maxep maxep merged commit 8cbca94 into develop Sep 2, 2024
2 checks passed
@maxep maxep deleted the maxep/improve-metrics branch September 2, 2024 09:01
@mariedm mariedm mentioned this pull request Sep 11, 2024
3 tasks
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.

2 participants