-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Add an Engine method to get the 1% percentile low FPS #67136
base: master
Are you sure you want to change the base?
Conversation
for (int i = 0; i < 100; i++) { | ||
frametime_1_percent_low = MAX(frametime_1_percent_low, recent_frametimes[i]); | ||
} | ||
Engine::get_singleton()->_fps_1_percent_low = 1'000'000.0 / frametime_1_percent_low; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- There's an interesting comment on line 3173 about FPS reporting. Based on that, shouldn't this code be inside the IF from line 3173 (the if below)?
- Suggestion: Since it is already printing the FPS below (L3178 and 3181), WDYT aboud add the 1% low FPT there too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an interesting comment on line 3173 about FPS reporting. Based on that, shouldn't this code be inside the IF from line 3173 (the if below)?
1% low FPS reporting is designed to be updated as often as possible, rather than just once per second. This makes the value less stable when you print out its value in _process
, but it makes changes more reactive, which I think is better overall.
The editor performance monitor and --print-fps
will still only update the displayed value once per second due to how they work (see the graph screenshot in OP).
In the long term, we could look into adding a smoothing algorithm like #63356, but I think this is better left for a future PR.
Suggestion: Since it is already printing the FPS below (L3178 and 3181), WDYT aboud add the 1% low FPT there too?
That's a good idea. I implemented this 🙂
957c0fb
to
c3edd33
Compare
Generally, when discussing performance (at least for web apps and APIs), the common thing to call this would be "99th percentile" (for the longest/slowest 1% of requests). e.g. Sentry's performance tools, Google Cloud's latency metrics, Datadog's "percentile aggregations", etc. To mirror that, should this be called "1st percentile FPS"? (or even just call it 99th percentile FPS, since that's still true here, it's just that in this case we want higher numbers rather than wanting lower numbers). |
See https://www.capframex.com/blog/post/Explanation%20of%20different%20performance%20metrics and https://www.capframex.com/blog/post/The%20challenge%20of%20displaying%20performance%20metrics%20as%20FPS. The "1% low" term generally seems more popular than "99th percentile" nowadays in gaming, though both are open to ambiguity as shown in the linked articles. |
We discussed this in a PR review meeting. The feature looks nice and useful, but we have some concerns on the implementation:
|
As I wrote in OP, there are other "% low" metrics out there, but 1% low is by far the most commonly used one.
The computation could be disabled in release builds, but it only takes a few microseconds per frame so I'd prefer not. It should be possible for developers to profile fully optimized release exports after all. |
c3edd33
to
4abe42e
Compare
4abe42e
to
e851733
Compare
Rebased and tested again, I'm noticing strange behavior from the 1% low FPS counter (look at the Core line): fps_low.mp4Most noticeably, when capping at 30 FPS, it never goes past 10 FPS (which is a FPS cap that is momentarily applied when I go to the 30 FPS cap, but it stays here even if I stay at a perfect 30 FPS for 10+ seconds). Edit: Fixed thanks to @CaelusV's review comment below. |
e851733
to
b2ce236
Compare
Measuring performance by only looking at the average FPS can be misleading, as microstutters will not be represented in the final value. This can lead to confusion as a game might have an average FPS of over 60, yet it can feel very stuttery if its 1% low FPS are in the single digits. This adds a Engine method and profiler readout for the 1% low FPS over the last 100 rendered frames. This is a moving metric by definition, but it still helps make these FPS drops more noticeable compared to an average readout.
b2ce236
to
a3601f3
Compare
Measuring performance by only looking at the average FPS can be misleading, as microstutters will not be represented in the final value. This can lead to confusion as a game might have an average FPS of over 60, yet it can feel very stuttery if its 1% low FPS are in the single digits.
This adds a Engine method and profiler readout for the 1% low FPS over the last 100 rendered frames. This is a moving metric by definition, but it still helps make these FPS drops more noticeable compared to an average readout.
This closes godotengine/godot-proposals#5459.
Testing project: test_fps_1_percent_low.zip
Preview
Performance monitor
Video
FPS cap is engaged at the middle of the video. The recording has some stuttering, but it feels smooth in practice. The
[Core]
line is the metric introduced in this PR, while[Script]
is a GDScript prototype I left in the testing project for comparison.out.mp4