Use maximumFramesPerSecond for slow-frame threshold #10220

[JesusRojass]

Use maximumFramesPerSecond for slow-frame threshold, fixes #10220

Discussion

Replaced the hard-coded 16 ms slow-frame threshold with a dynamic threshold derived from UIScreen.mainScreen.maximumFramesPerSecond. The frozen-frame threshold remains fixed at 700 ms.

Existing behavior at 60 Hz is preserved.

The prior implementation assumed a fixed 60 Hz refresh rate and used 16 ms as the slow-frame budget.

On platforms like tvOS and iOS devices with ProMotion (variable refresh), this causes inaccurate “slow rendering” metrics e.g.,
tvOS at 50 Hz uses ~20 ms frame budget, yet is still judged by 16 ms;
ProMotion at 120 Hz would need ~8.3 ms budget.

By deriving the threshold from maximumFramesPerSecond, we align with actual device capabilities and reduce false “slow” counts, especially on 50 Hz and 120 Hz outputs.

Testing

• Existing tests where kept untouched.
• Additional test cases appended in the same test file cover scenarios for 50 fps (~20 ms budget), 60 fps (~16.7 ms budget), and 120 fps (~8.33 ms budget) by stubbing maximumFramesPerSecond.
• The frozen-frame threshold remains 700 ms and is validated in each scenario.

API Changes

• No API Changes

[JesusRojass]

/gemini review

[JesusRojass]

@visumickey @eBlender @tejasd

Ready for review

[gemini-code-assist]

Code Review

This pull request correctly replaces the hardcoded slow-frame threshold with a dynamic value based on the device's maximumFramesPerSecond. This is a great improvement for accurately measuring performance on modern devices with variable refresh rates. The implementation is clean, and the new unit tests are comprehensive. I have a couple of suggestions regarding the new test code to improve its robustness and reduce duplication.

[JesusRojass]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a valuable change by replacing the hard-coded slow-frame threshold with a dynamic one derived from UIScreen.main.maximumFramesPerSecond. This improves the accuracy of slow-frame metrics on devices with different refresh rates. The implementation is solid, and the new tests using method swizzling provide excellent coverage for various scenarios. My review includes a critical point about existing tests that are now broken by this change and need to be updated, as well as a minor suggestion to improve the conciseness of the new test suite.

[JesusRojass]

/gemini review

[gemini-code-assist]

Code Review

This pull request is a great improvement. It replaces the hardcoded slow-frame threshold with a dynamic value based on the device's maximumFramesPerSecond, which will lead to more accurate performance metrics on devices with varying refresh rates like ProMotion displays. The implementation is clean, and the addition of helper functions for calculating the FPS and frame budget is a good approach. I'm particularly impressed with the thoroughness of the new unit tests, which use method swizzling to cover various FPS scenarios effectively. I have a couple of minor suggestions for cleaning up the test code, but overall this is excellent work.

[JesusRojass]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a significant improvement by replacing the hardcoded slow-frame threshold with a dynamic one based on the device's maximumFramesPerSecond. This makes frame performance monitoring more accurate across a wider range of Apple devices, including those with ProMotion displays and different refresh rates like on tvOS. The implementation is clean, with new helper functions to calculate the frame budget. The accompanying tests are thorough, using method swizzling to cover various FPS scenarios, which is a great approach. I have one minor suggestion to optimize the test swizzling implementation for a small performance gain.

[JesusRojass]

/gemini review

[gemini-code-assist]

Code Review

This pull request is a great improvement, replacing the hard-coded slow-frame threshold with a dynamic one based on the screen's maximumFramesPerSecond. This will improve the accuracy of frame performance metrics on devices with variable or non-standard refresh rates. The implementation is clean and the accompanying tests are very thorough, using method swizzling to cover various FPS scenarios.

I have one minor suggestion to clean up some redundant code in the tests.

[JesusRojass]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a valuable improvement by dynamically calculating the slow-frame threshold based on the device's maximumFramesPerSecond. This will lead to more accurate frame rendering metrics on devices with varying refresh rates. The implementation is well-structured, and the new tests are comprehensive. I've provided two comments on the test file: one high-severity comment to address potential test flakiness due to state not being isolated between test iterations, and one low-severity comment for a minor code cleanup. Overall, this is a solid contribution.

[JesusRojass]

/gemini review

[gemini-code-assist]

Code Review

This pull request is a great improvement, replacing the hard-coded slow-frame threshold with a dynamic one based on UIScreen.maximumFramesPerSecond. This will improve accuracy on devices with different refresh rates. The implementation is correct and the new logic is well-covered by new unit tests. I have one suggestion to improve efficiency by caching the calculated frame budget, and a minor style comment on the test code. Overall, excellent work.

[JesusRojass]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a valuable enhancement by dynamically calculating the slow-frame threshold from maximumFramesPerSecond, which improves metric accuracy on devices with varying refresh rates. The implementation is solid, and the new unit tests that cover different FPS scenarios are a great addition. My feedback primarily focuses on opportunities to reduce code duplication by leveraging the new helper functions, which will enhance the code's maintainability.

[JesusRojass]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a valuable improvement by dynamically calculating the slow-frame threshold based on the device's maximumFramesPerSecond. This ensures more accurate frame rendering metrics on devices with varying refresh rates, such as those with ProMotion displays or tvOS devices. The implementation is solid, and the accompanying unit tests are thorough, using method swizzling to cover different FPS scenarios effectively. I have one minor suggestion regarding the notification handling to enhance code clarity and maintainability by consolidating redundant observers.

[JesusRojass]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a dynamic slow-frame threshold based on the device's maximumFramesPerSecond, which is a great improvement for accurate performance monitoring on devices with variable refresh rates. The implementation is clean, caching the computed threshold and updating it when the app becomes active. The accompanying unit tests are thorough, using method swizzling to validate the behavior across different FPS scenarios (50, 60, and 120 FPS). This is a high-quality contribution that enhances the accuracy of frame rendering metrics. I have one minor suggestion to improve test clarity.

[JesusRojass]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the slow-frame detection logic to use a dynamic threshold based on the device's maximumFramesPerSecond, which is a great improvement for supporting various screen refresh rates. The implementation is solid, with caching of the threshold for performance and comprehensive new tests to validate the behavior across different FPS values.

I've added a couple of suggestions in the tests to improve robustness and maintainability regarding test state isolation. Specifically, one is about reverting the method swizzling after tests complete, and the other is about a static variable that can cause state to leak between tests.

Overall, this is a well-executed change.

[JesusRojass]

asked for review but didn't staged files

[JesusRojass]

/gemini review

[gemini-code-assist]

Code Review

This pull request is a great improvement, replacing the hardcoded slow-frame threshold with a dynamic one based on the device's maximumFramesPerSecond. This will lead to more accurate frame performance metrics on devices with varying refresh rates, such as those with ProMotion displays or tvOS devices. The implementation is solid, caching the calculated threshold and refreshing it appropriately. The accompanying unit tests are thorough, using method swizzling to validate the behavior across different refresh rates, which ensures the robustness of this new logic. I have one minor suggestion to improve code maintainability.

[gemini-code-assist]

Code Review

This pull request introduces a significant improvement by dynamically calculating the slow-frame threshold based on the device's maximumFramesPerSecond. This is a great change that makes frame performance monitoring more accurate on devices with varying refresh rates, such as those with ProMotion displays or tvOS devices. The implementation is clean, caching the calculated values for efficiency and refreshing them when the app becomes active. The addition of comprehensive unit tests using method swizzling to cover different FPS scenarios is excellent and ensures the new logic is robust. I have one minor suggestion for a small refactoring to improve code clarity.

[JesusRojass]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a significant improvement by replacing the hard-coded slow-frame threshold with a dynamic value derived from UIScreen.maximumFramesPerSecond. This makes frame performance monitoring more accurate across a range of Apple devices with different screen refresh rates, such as ProMotion displays and tvOS devices. The implementation is robust, including caching the calculated frame budget and refreshing it when the app becomes active. The state management is also improved by moving the previousTimestamp from a static variable to an instance property. The accompanying tests are excellent, using method swizzling to cover various FPS scenarios (50, 60, and 120 FPS) in a clean, table-driven manner. The changes are well-executed and significantly enhance the accuracy of performance metrics.

[JesusRojass]

Some tests Arent passing locally still, I am working on those @visumickey @tejasd @eBlender