Use eglPresentationTimeANDROID to avoid bogging down the GPU #29727
Conversation
flutter-dashboard
bot
added
embedder
| } | ||
| if (!compositor_frame) { | ||
| return RasterStatus::kFailed; | ||
| } |
There was a problem hiding this comment.
This file is much easier to review if you choose the hide whitespace option in GitHub.
There was a problem hiding this comment.
After googling it, an easy way to do this is to just add ?w=1 to the URL of this page and it will disable it for just this one visit.
|
This pull request was opened against a branch other than master. Since Flutter pull requests should not normally be opened against branches other than master, I have changed the base to master. If this was intended, you may modify the base back to main. See the Release Process for information about how other branches get updated. Reviewers: Use caution before merging pull requests to branches other than master, unless this is an intentional hotfix/cherrypick. |
| SurfaceFrame::SubmitInfo submit_info; | ||
| submit_info.frame_damage = damage.GetFrameDamage(); | ||
| submit_info.buffer_damage = damage.GetBufferDamage(); | ||
| submit_info.target_time = frame_timings_recorder.GetVsyncTargetTime(); |
There was a problem hiding this comment.
Since we're now depending on GetVsyncTargetTime for something other than metrics, I think we'll need to update VsyncWaiter to always get the current Display refresh rate instead of caching it. https://github.com/flutter/engine/blob/main/shell/platform/android/io/flutter/view/VsyncWaiter.java#L39
On Android S and later, Display.getRefreshRate() returns the current display refresh rate, instead of the app maximum. https://developer.android.com/about/versions/12/reference/compat-framework-changes#display_info_nr_advanced_supported
There was a problem hiding this comment.
@jason-simmons FYI - since I think you recently changed this to not do that.
There was a problem hiding this comment.
The call to getRefreshRate() was taking a significant amount of time on the platform thread in some cases (see flutter/flutter#90883)
Reintroducing that call may be risky if it needs to be done on every frame.
There was a problem hiding this comment.
Do we know if those devices were susceptible when running on Android S?
I guess the problem here is that some devices will scale the refresh rate automatically/dynamically.
It might also be interesting to see if the same delays exist when using the NDK API instead.
There was a problem hiding this comment.
The Huawei device referenced in flutter/flutter#90883 (comment) was running Android P
I don't know if Android S ensures better performance for getRefreshRate
There was a problem hiding this comment.
#29800 Should resolve this.
|
I wonder if this will help with the condition seen here: flutter/flutter#75540 |
|
The vsync timing will be fixed by #29800, which will land before this one. This patch is ready for review. |
Attempt to reland: flutter#29727 TODO: 1. This isn't right in the cases where the frame has been resubmitted. 2. This hasn't been wired for impeller yet.
Attempt to reland: flutter#29727 TODO: 1. This isn't right in the cases where the frame has been resubmitted. 2. This hasn't been wired for impeller yet.
Attempt to reland: #29727 with some fixes.
…er#33881) Attempt to reland: flutter#29727 with some fixes.
Fixes flutter/flutter#93352
Improves Android benchmarks on both Pixel 4 and a lower end Android Go device for 99th percentile and average raster times.
This works by telling the system compositor what timestamp we intended to show this frame for. This way, if we end up with a frame that gets submitted right at the beginning of a vsync and then a second frame submitted on the same vsync, the compositor will only try to show the second frame on the screen and save the GPU some work.
Without this, a situation like that results in an "avalanche" of calls where the GPU is behind the CPU and keeps delaying CPU work until we finally stop submitting frames. This can be observed as a lengthy
dequeuBufferin a systrace enabled trace, as shown in the linked issue. This avalanche is often triggered by a frame that does a shader compile through a couple vsyncs and then is followed by a bunch of very fast frames that take less than a vsync to render - the first of those fast frames gets delivered before the end of the vsync that the slow frame ended in.We cannot implement this ourselves because we don't know how long the swap buffers call will take on the system side, and if we try to guess we can very well get it wrong.
I've filed issues to look into adding this for Vulkan and Metal, although we should also first take traces there to make sure it's warranted.
See also: https://android-developers.googleblog.com/2020/04/high-refresh-rate-rendering-on-android.html