[Impeller] Adjustments to SubmitKHR and queries. #47249
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
I can't provide it, but I think this fixes flutter/flutter#135642 |
chinmaygarde
changed the title
chinmaygarde
left a comment
chinmaygarde
left a comment
There was a problem hiding this comment.
Suggestion about using fml::Thread instead of a "concurrent" runner with count 1. But just nits. LGTM.
| submit_message_loop_ = fml::ConcurrentMessageLoop::Create(1u); | ||
| submit_message_loop_->PostTaskToAllWorkers([]() { | ||
| // submitKHR is extremely cheap and mostly blocks on an internal fence. | ||
| fml::RequestAffinity(fml::CpuAffinity::kEfficiency); |
There was a problem hiding this comment.
How about just fml::Thread? That has a handy way to name the thread too. You can still post task to it to set it as an efficiency affinity.
There was a problem hiding this comment.
Oh neat, didn't realize that. makes more sense
| std::shared_ptr<CommandPoolRecyclerVK> command_pool_recycler_; | ||
| std::string device_name_; | ||
| std::shared_ptr<fml::ConcurrentMessageLoop> raster_message_loop_; | ||
| std::shared_ptr<fml::ConcurrentMessageLoop> submit_message_loop_; |
There was a problem hiding this comment.
Here and elsewhere, perhaps call it queue_submit_task_runner? I was initially wondering, "submit what"?
| // found in the LICENSE file. | ||
|
|
||
| #include "impeller/renderer/backend/vulkan/context_vk.h" | ||
| #include "fml/concurrent_message_loop.h" |
There was a problem hiding this comment.
Nit: Newline between primary header and the rest.
jonahwilliams
added
the
autosubmit
…137135) flutter/engine@d588950...8f7488e 2023-10-24 [email protected] Roll Skia from b3923b715c50 to 70978e42479c (1 revision) (flutter/engine#47257) 2023-10-24 [email protected] Update fml::ThreadPriority enum to match style guide. (flutter/engine#47255) 2023-10-24 [email protected] [Impeller] Adjustments to SubmitKHR and queries. (flutter/engine#47249) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
It turns out the fixing #47249 revealed further bugs: we were actually overwriting previous presents and dropping frames, which was what the forward progress error was hinting. Once this problem is fixed, we started hitting present timeouts or overdraw of swapchain images. The fix for this is that we shouldn't even ask for a swapchain image until after we've called present, which actually wasn't tracked with anything. The error: ``` A Dart VM Service on Pixel 6 Pro is available at: http://127.0.0.1:62491/_N4PSEPUtA0=/ The Flutter DevTools debugger and profiler on Pixel 6 Pro is available at: http://127.0.0.1:9102?uri=http://127.0.0.1:62491/_N4PSEPUtA0=/ W/OnBackInvokedCallback(19160): OnBackInvokedCallback is not enabled for the application. W/OnBackInvokedCallback(19160): Set 'android:enableOnBackInvokedCallback="true"' in the application manifest. W/vulkan (19160): dequeueBuffer timed out: Function not implemented (-38) E/flutter (19160): [ERROR:flutter/impeller/base/validation.cc(49)] Break on 'ImpellerValidationBreak' to inspect point of failure: Could not acquire next swapchain image: Timeout E/flutter (19160): [ERROR:flutter/shell/gpu/gpu_surface_vulkan_impeller.cc(64)] No surface available. ```
2 participants
Removes the rest of the validation errors by increasing the size of the query ring buffer. Because we rely on callbacks fired from the fence waiter, when the phone is under load these can be substantially delayed from the frame finishing.
Adjusts presentation to use a dedicated default priority/slow core affinity/single threaded task runner. From the ARM video guide, submitKHR will block until all previously submitted work has been scheduled. This needs to happen in order, and doesn't need to run at high priority or even on medium speed cores.