[Impeller] batch up filter graph command buffers. #51912
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
LGTM, I'm excited to see how this affects our blur benchmarks. This should be test exempt since it does not change functionality, just performance. I just have one style note.
impeller/entity/contents/contents.cc
Outdated
| } | ||
| if (!renderer.GetContext() | ||
| ->GetCommandQueue() | ||
| ->Submit({std::move(command_buffer)}) |
There was a problem hiding this comment.
Can you put the explicit type here please. ( https://google.github.io/styleguide/cppguide.html#Type_deduction )
There was a problem hiding this comment.
In the initializer list?
There was a problem hiding this comment.
Oh, it's for a vector. I was thinking it was constructing something like a SubmitInfo and we were using the initializer syntax to avoid explicitly mentioning that type. Man, c++. I feel different about using it for std::vectors. It would be a big pain to have to explicitly list those types. I think /*buffers=*/ would be an improvement since that's typically what we do for rvalues in parameter lists.
| } | ||
| if (!renderer.GetContext() | ||
| ->GetCommandQueue() | ||
| ->Submit({std::move(command_buffer)}) |
|
|
||
| if (!renderer.GetContext() | ||
| ->GetCommandQueue() | ||
| ->Submit({std::move(command_buffer)}) |
|
|
||
| if (!renderer.GetContext() | ||
| ->GetCommandQueue() | ||
| ->Submit({command_buffer}) |
| if (!GetContentContext() | ||
| ->GetContext() | ||
| ->GetCommandQueue() | ||
| ->Submit({command_buffer}) |
| } | ||
| if (!renderer.GetContext() | ||
| ->GetCommandQueue() | ||
| ->Submit({std::move(command_buffer)}) |
jonahwilliams
added
the
autosubmit
…146355) flutter/engine@6974dba...d048b9a 2024-04-05 [email protected] [Impeller] batch up filter graph command buffers. (flutter/engine#51912) 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
|
I did some CPU captures after the fact. Its a bit better, but if you breakdown the top% CPU (filtering to flutter only code) its mostly render pass construction. Part of this might be backpressure, althrough I would expect to see that in waitForNextDrawable..
|
…lutter#146355) flutter/engine@6974dba...d048b9a 2024-04-05 [email protected] [Impeller] batch up filter graph command buffers. (flutter/engine#51912) 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
2 participants
The filter graph frequently creates and submits command buffers in serial. This has some unnecessary overhead, esp on Vulkan where submitting a command buffer has a non-trivial cost. While previously I had tried to batch up all submissions, doing this in a limited manner in the filter graph should be more straightforward.
For gaussians this makes a big difference, as there is a mipmap generation, downsample, then 2 render passes, so we can compress the 4 command buffers into 1.
flutter/flutter#142545