Add static_path_tessellation macrobenchmark #131837

knopp · 2023-08-03T09:56:03Z

This adds a macrobenchmark representative of a real world application that uses SVG icons. The scenario of rasterizing complex paths that don't change over time does not seem to be covered by any other macrobenchmark and shows a significantly slower impeller performance compared to skia.

It's actually bit problematic to measure this because on A15 the CPU load with impeller is high enough to trigger CPU frequency change. So in order to get consistent reading I had to add a spinning background thread that would keep the CPU at highest frequency.

  [NSThread detachNewThreadWithBlock:^{
    while (true) {
      pthread_yield_np();
    }
  }];

flutter drive --profile --local-engine=ios_profile -t test_driver/run_app.dart --driver test_driver/path_tessellation_static_perf_test.dart

average_frame_build_time_millis	Time
Impeller	0.46686524822695047
Skia	0.4625749999999999
Skia - No RasterCache	0.47173750000000086

average_frame_rasterizer_time_millis	Time
Impeller	6.654328519855595
Skia - Raster Cache	0.2534123711340209 *
Skia - No RasterCache	0.53424375

Adding the GeometryPainter seems to have triggered the complexity threshold for raster cache.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

jonahwilliams · 2023-08-05T01:16:49Z

Can we add two varieties of this? One static path, and one with the same paths but under a scale transform that prevents the patches from being cached? For example something like transform.translate(random.nextDouble() * 2) each frame.

My biggest concern with caching implementations is that if we only measure cases where the caching is successful (cough cough raster cache), then we'll miss cases where we make non-optimal tradeoffs otherwise. For example, at some point we'll move to a compute based approach for polyline/stroke paths. That may slightly regress a benchmark with tons of completely static paths, but dramatically improve a benchmark with uncachable paths.

If we have both benchmarks, then we'll be more aware of the tradeoffs we make.

jonahwilliams

Also, you can turn off the raster cache in the skia backend by setting willChange to true on the CustomPaint widget

knopp · 2023-08-05T07:39:48Z

I like the idea. For example computing bounds does lot of small allocations right now. Fixing that is easy, but will not show any difference for cached paths (bounds are cached with path), but might for uncached paths.

knopp · 2023-08-05T14:18:32Z

I split it into path_tessellation_static_perf_test.dart and path_tessellation_dynamic_perf_test.dart. There is also a button to toggle the animation. Animating size of custom painters, which is enough to invalidate and recreate paths.

jonahwilliams · 2023-08-07T19:20:39Z

dev/benchmarks/macrobenchmarks/lib/src/path_tessellation.dart

+                      key: const Key(
+                          'animate_button'), // this key is used by the driver test
+                      child: const Text('Animate'),


Rather than have a button toggle between them, can we configure this via an argument passed to the PathTessellationPage widget constructor - and then have two separate routes for static and dynamic?

Or leave the toggle, but let the route configure the default? I guess its fine - I just have driver test PTSD

I don't mind making this two separate routes. I actually wanted to do it that way initially but I think other tests seem to have toggles so I wasn't sure if that's the preferred way.

Yeah, that's fine!

Given that the home page button is actually used to push the route (runDriverTestForRoute), it doesn't really seem to make sense to add a separate route without adding a separate home button.

So I not quite sure what you prefer. I can either make this two routes + two buttons at the home page, or one route + animate button (leave as it is now).

jonahwilliams

LGTM with nit.

I will handle turning this on for CI, I have some other pending benchmarks to add so I'll just make the configuration change after this lands.

…131837)

flutter/flutter@ad0aa8d...436df69 2023-08-08 [email protected] Roll Flutter Engine from 9c83d90b01bd to 146c4c9487fc (6 revisions) (flutter/flutter#132112) 2023-08-08 [email protected] Roll Flutter Engine from c27109291e22 to 9c83d90b01bd (5 revisions) (flutter/flutter#132108) 2023-08-08 [email protected] Roll Flutter Engine from be085f6699b6 to c27109291e22 (3 revisions) (flutter/flutter#132086) 2023-08-08 [email protected] Revert "Replace TextField.canRequestFocus with TextField.focusNode.canRequestFocus" (flutter/flutter#132104) 2023-08-08 [email protected] [Impeller] add drawVertices and drawAtlas benchmarks. (flutter/flutter#132080) 2023-08-07 [email protected] Adds more documentations around ignoreSemantics deprecations. (flutter/flutter#131287) 2023-08-07 [email protected] [web] New HtmlElementView.fromTagName constructor (flutter/flutter#130513) 2023-08-07 [email protected] Move mock canvas to flutter_test (flutter/flutter#131631) 2023-08-07 [email protected] Add static_path_tessellation macrobenchmark (flutter/flutter#131837) 2023-08-07 [email protected] [web] Remove usage of `ui.webOnlyInitializePlatform()` (flutter/flutter#131344) 2023-08-07 [email protected] Roll Flutter Engine from 39a575f65d50 to be085f6699b6 (1 revision) (flutter/flutter#132069) 2023-08-07 [email protected] Android context menu theming and visual update (flutter/flutter#131816) 2023-08-07 [email protected] Roll Flutter Engine from 39ce1c097bce to 39a575f65d50 (2 revisions) (flutter/flutter#132064) 2023-08-07 [email protected] CupertinoContextMenu improvement (flutter/flutter#131030) 2023-08-07 [email protected] Roll Flutter Engine from 5b47c0577060 to 39ce1c097bce (3 revisions) (flutter/flutter#132057) 2023-08-07 [email protected] Slider should check `mounted` before start interaction (flutter/flutter#132010) 2023-08-07 [email protected] Roll Packages from ce53da1 to d7ee75a (7 revisions) (flutter/flutter#132058) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages 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 Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…131837)

knopp force-pushed the add_static_path_tesselection_macrobenchmark branch 2 times, most recently from 1ef1fab to fd481ce Compare August 3, 2023 10:05

knopp changed the title ~~Add static_path_tesselation macrobenchmark~~ Add static_path_tessellation macrobenchmark Aug 3, 2023

knopp force-pushed the add_static_path_tesselection_macrobenchmark branch from fd481ce to ca218de Compare August 3, 2023 14:42

knopp mentioned this pull request Aug 3, 2023

WIP: [impeller] Add tessellation cache flutter/engine#44324

Closed

8 tasks

Add static_path_tesselation macrobenchmark

201c27f

knopp force-pushed the add_static_path_tesselection_macrobenchmark branch from 3305a57 to 749429b Compare August 4, 2023 15:47

knopp mentioned this pull request Aug 4, 2023

[Impeller] Cache Polyline and tessellation for Path flutter/engine#44390

Closed

8 tasks

Add geometry painter

e60d840

knopp force-pushed the add_static_path_tesselection_macrobenchmark branch from 749429b to e60d840 Compare August 4, 2023 20:29

jonahwilliams reviewed Aug 5, 2023

View reviewed changes

Add static and dynamic versions

974ee54

github-actions bot added the a: desktop Running on desktop label Aug 5, 2023

Only parse paths once

cd35c50

jonahwilliams reviewed Aug 7, 2023

View reviewed changes

jonahwilliams approved these changes Aug 7, 2023

View reviewed changes

jonahwilliams mentioned this pull request Aug 7, 2023

[Impeller] add drawVertices and drawAtlas benchmarks. #132080

Merged

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 7, 2023

auto-submit bot merged commit 1c7e2af into flutter:master Aug 7, 2023

knopp deleted the add_static_path_tesselection_macrobenchmark branch August 8, 2023 09:33

engine-flutter-autoroll mentioned this pull request Aug 8, 2023

Roll Flutter from ad0aa8de7512 to 436df69a4684 (17 revisions) flutter/packages#4663

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 8, 2023

1c7e2af Add static_path_tessellation macrobenchmark (flutter/flutter#…

3f0c601

…131837)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023

1c7e2af Add static_path_tessellation macrobenchmark (flutter/flutter#…

acd0832

…131837)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add static_path_tessellation macrobenchmark #131837

Add static_path_tessellation macrobenchmark #131837

Uh oh!

knopp commented Aug 3, 2023 •

edited

Loading

Uh oh!

jonahwilliams commented Aug 5, 2023

Uh oh!

jonahwilliams left a comment

Uh oh!

knopp commented Aug 5, 2023

Uh oh!

knopp commented Aug 5, 2023

Uh oh!

jonahwilliams Aug 7, 2023

Uh oh!

jonahwilliams Aug 7, 2023

Uh oh!

knopp Aug 7, 2023

Uh oh!

jonahwilliams Aug 7, 2023

Uh oh!

knopp Aug 7, 2023

Uh oh!

jonahwilliams left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Add static_path_tessellation macrobenchmark #131837

Add static_path_tessellation macrobenchmark #131837

Uh oh!

Conversation

knopp commented Aug 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

jonahwilliams commented Aug 5, 2023

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

knopp commented Aug 5, 2023

Uh oh!

knopp commented Aug 5, 2023

Uh oh!

jonahwilliams Aug 7, 2023

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Aug 7, 2023

Choose a reason for hiding this comment

Uh oh!

knopp Aug 7, 2023

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Aug 7, 2023

Choose a reason for hiding this comment

Uh oh!

knopp Aug 7, 2023

Choose a reason for hiding this comment

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

knopp commented Aug 3, 2023 •

edited

Loading