Skip to content

Conversation

@flar
Copy link
Contributor

@flar flar commented Aug 28, 2023

This benchmark will track the performance of the RTree implementation to cull very large pictures down to just the portion visible on the screen.

@github-actions github-actions bot added the f: scrolling Viewports, list views, slivers, etc. label Aug 28, 2023
@flar
Copy link
Contributor Author

flar commented Aug 28, 2023

We should have probably added this benchmark back when we fixed #92366 by adding RTrees to DisplayList in flutter/engine#38429 so that we could track a number of changes that were made to the code - some potentially positive, some potentially regressing.

In particular, I recently ran a local copy of the benchmark from #92366 and discovered that not only had we lost almost all of the gains, but the app now takes up to 20 seconds to draw its first frame. The root cause is an oversight in an old PR that added fine-grained bounds when drawing one DL into another in flutter/engine#37451 and any case that would cause such a DL -> DL recording. At the time it was added, there weren't any cases where we might do that, but in the intervening time we've added a couple of cases.

  • Impeller has been rendering layer trees by recording the whole tree into a tree-wide DL. If there is a DLLayer that has a very large DL in it, we will iterate over a large number of operations that might not even pass the culling into the frame-sized tree DL.
  • Embedders have been recording parts of a tree into DisplayLists using the DisplayListEmbedderViewSlice

When these things happen for the test case mentioned above, 300,000+ drawline calls, that all overlap are iterated and consolidated into a single rectangle somewhat quadratically causing a huge regression.

Some work is being done that will reduce some of these cases, including:

  • Impeller is switching over to a direct implementation of DlCanvas for rendering the layer tree which will avoid recording a huge DL into another.
  • @knopp recently modified the consolidation of rectangles to use a DlRegion object which greatly improved its performance, but still doesn't help as much as simply reducing the culling rect to use to iterate the bounds when we are transferring them from one DL to another.

After this benchmark lands, I will be landing a 1-line fix for the biggest cause of the regression here - the fact that iterating the rtree bounds inside DlCanvasToReceiver does not provide the cull rect. (see #133437)

We should also consider punting on the fine grained bounds for some DisplayLists, particularly if they contain quite a lot of rendering calls. (see #133436)

@flar flar force-pushed the very-long-picture-scrolling-benchmark branch from 82d6484 to c8516cd Compare August 28, 2023 08:35
@flar flar force-pushed the very-long-picture-scrolling-benchmark branch from ab40b36 to ccc6703 Compare August 28, 2023 21:06
@flar flar requested review from chinmaygarde and zanderso August 31, 2023 18:11
@chinmaygarde chinmaygarde changed the title add benchmark for scrolling very long pictures Add benchmark for scrolling very long pictures Aug 31, 2023
@flar flar added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 31, 2023
@auto-submit auto-submit bot merged commit b51859e into flutter:master Aug 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 1, 2023
auto-submit bot pushed a commit to flutter/engine that referenced this pull request Sep 2, 2023
)

Fixes flutter/flutter#133437

With this fix, the [benchmarks](flutter/flutter#133434) for pictures with huge bounds become runnable under more conditions. Currently those benchmarks only run on platforms where we don't run into this issue. The performance on those platforms/conditions will not change much, but with this fix we can run them on Impeller and add a variant that has a platform view on the display.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App f: scrolling Viewports, list views, slivers, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants