Test Framework for SKP based shader warmup #19260

freiling · 2020-06-24T03:40:31Z

Description

This pull request mainly sets up a test framework for warming up the shader cache by drawing skia pictures serialized to disk at runtime, and some custom serialization hooks to decrease the disk footprint from Image data, which is not needed to warm up shaders. Includes other ancillary changes to make skp serialization work properly on fuchsia.

Related Issues

fxb/47057

Tests

I added the following tests:

SkpWarmupTest.Basic
SkpWarmupTest.Image

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the contributor guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the C++, Objective-C, Java style guides for the engine.
I read the tree hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation.
All existing and new tests are passing.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change. If not, delete the remainder of this section.
- I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
- I got input from the developer relations team, specifically from: Replace with the names of who gave advice
- I wrote a migration guide: https://flutter.dev/go/breaking-changes-template Replace this with a link to a pull request that adds the migration guide to https://flutter.dev/docs/release/breaking-changes

liyuqian · 2020-06-24T19:45:57Z

shell/common/rasterizer.cc


  frame->Raster(*tree, true);

+#if defined(OS_FUCHSIA)
+ SkSerialProcs procs = {0};


Format issue: space doesn't align.

liyuqian · 2020-06-24T19:49:48Z

shell/common/skp_shader_warmup_unittests.cc

+    WaitForIO(shell.get());
+
+    // Count the number of shaders this builder generated. We use this as a proxy
+    // for whether new shaders were generated, since skia will dump an skp any


I think Flutter only dumps one skp per frame no matter how many shaders are compiled in that frame. So this will count the number of frames, not the number of shaders.

Noted. I think thats ok for the purpose of this test, since the goal is to tell if new shaders were generated and we don't care how many, and in all cases we only pump one frame. I can definitely change the comment to reflect this though

liyuqian · 2020-06-24T20:02:03Z

shell/common/skp_shader_warmup_unittests.cc

+
+        SkDeserialProcs procs = {0};
+        procs.fImageProc = DeserializeImageWithoutData;
+        sk_sp<SkPicture> picture = SkPicture::MakeFromStream(stream.get(), &procs);


I'm a little surprised that SkPicture::MakeFromStream can be used here without changing Skia's build configuration. I think Flutter has disabled that Skia feature. Is there a Fuchsia config file that re-enables it?

BTW, I can't find SkpWarmupTest in https://api.cirrus-ci.com/v1/task/6670703166488576/logs/test_host.log

First question i dunno, seems to work for me.

Second question: the whole test is ifdef'd with OS_FUCHSIA right now because it was failing on windows and i figured it was best to isolate any problems with it to Fuchsia since thats the only platform where this is relevant right now. I assume that means its not included in host_debug_unopt

arbreng

LGTM besides nits and a bug for tracking purposes

arbreng · 2020-07-02T17:33:12Z

shell/common/rasterizer.cc

@@ -457,16 +454,30 @@ static sk_sp<SkData> ScreenshotLayerTreeAsPicture(
  SkMatrix root_surface_transformation;
  root_surface_transformation.reset();

+#if defined(OS_FUCHSIA)
+  // Our ScopedFrame implementation doesnt do the right thing here so


Please make a bug for this and block fxb/46971 on it

This change adds custom image serialization functions for skp serialization which write out onlythe image metadata and not the contents of the images themselves. This allows SKP's to be used for shader warmup with a significantly reduced disk space footprint.

)

googlebot added the cla: yes label Jun 24, 2020

auto-assign bot requested a review from gw280 June 24, 2020 03:41

freiling marked this pull request as draft June 24, 2020 03:45

freiling changed the title ~~Freiling dump skp~~ Test Framework for SKP based shader warmup Jun 24, 2020

freiling force-pushed the freiling-dump-skp branch from 374cdfa to d93faa9 Compare June 24, 2020 04:09

freiling requested review from liyuqian, arbreng and chinmaygarde June 24, 2020 04:25

freiling force-pushed the freiling-dump-skp branch from d93faa9 to 0aad8b6 Compare June 24, 2020 04:29

liyuqian reviewed Jun 24, 2020

View reviewed changes

arbreng approved these changes Jul 2, 2020

View reviewed changes

freiling marked this pull request as ready for review July 9, 2020 04:55

auto-assign bot requested a review from flar July 9, 2020 04:55

freiling force-pushed the freiling-dump-skp branch 6 times, most recently from 9c40e5a to 82f9de7 Compare July 16, 2020 20:31

freiling force-pushed the freiling-dump-skp branch 4 times, most recently from 31f4254 to 24a742d Compare July 20, 2020 21:03

freiling added 3 commits July 20, 2020 22:10

[fuchsia] fix Rasterizer::ScreenshotLayerTreeAsPicture on Fuchsia

6816a72

[fuchsia] Add shell test for skp based shader warmup

f77120d

freiling force-pushed the freiling-dump-skp branch from 24a742d to 160313e Compare July 20, 2020 22:10

freiling merged commit 15d19a4 into flutter:master Jul 21, 2020

engine-flutter-autoroll mentioned this pull request Jul 21, 2020

Roll Engine from dcc9a4048da8 to 15d19a494914 (31 revisions) flutter/flutter#61915

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 21, 2020

15d19a4 Test Framework for SKP based shader warmup (flutter/engine#19260

f1fa103

)

engine-flutter-autoroll mentioned this pull request Jul 21, 2020

Roll Engine from dcc9a4048da8 to 464e4b9bdace (32 revisions) flutter/flutter#61917

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 21, 2020

15d19a4 Test Framework for SKP based shader warmup (flutter/engine#19260

30b6d9c

)

engine-flutter-autoroll mentioned this pull request Jul 21, 2020

Roll Engine from dcc9a4048da8 to e14c87eb0818 (33 revisions) flutter/flutter#61919

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 21, 2020

15d19a4 Test Framework for SKP based shader warmup (flutter/engine#19260

d626a01

)

engine-flutter-autoroll mentioned this pull request Jul 21, 2020

Roll Engine from dcc9a4048da8 to ff8d3e98da3d (34 revisions) flutter/flutter#61932

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 21, 2020

15d19a4 Test Framework for SKP based shader warmup (flutter/engine#19260

dc925e5

)

engine-flutter-autoroll mentioned this pull request Jul 21, 2020

Roll Engine from dcc9a4048da8 to d4684bc79ac2 (35 revisions) flutter/flutter#61939

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 21, 2020

15d19a4 Test Framework for SKP based shader warmup (flutter/engine#19260

354da73

)

engine-flutter-autoroll mentioned this pull request Jul 21, 2020

Roll Engine from dcc9a4048da8 to d4684bc79ac2 (35 revisions) flutter/flutter#61942

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 21, 2020

15d19a4 Test Framework for SKP based shader warmup (flutter/engine#19260

3fe096d

)

engine-flutter-autoroll mentioned this pull request Jul 21, 2020

Roll Engine from dcc9a4048da8 to 5a73d4dc2503 (36 revisions) flutter/flutter#61944

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 21, 2020

15d19a4 Test Framework for SKP based shader warmup (flutter/engine#19260

0e339d9

)

engine-flutter-autoroll mentioned this pull request Jul 23, 2020

Roll Engine from dcc9a4048da8 to 626244a72c5d (67 revisions) flutter/flutter#62080

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 23, 2020

15d19a4 Test Framework for SKP based shader warmup (flutter/engine#19260

11ca413

)

dnfield mentioned this pull request Jun 22, 2022

Delete the skp_shader_warmup_test harness if possible flutter/flutter#106435

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test Framework for SKP based shader warmup #19260

Test Framework for SKP based shader warmup #19260

freiling commented Jun 24, 2020 •

edited

liyuqian Jun 24, 2020

liyuqian Jun 24, 2020

freiling Jun 24, 2020

liyuqian Jun 24, 2020

freiling Jun 24, 2020

arbreng left a comment

arbreng Jul 2, 2020

Test Framework for SKP based shader warmup #19260

Test Framework for SKP based shader warmup #19260

Conversation

freiling commented Jun 24, 2020 • edited

Description

Related Issues

Tests

Checklist

Breaking Change

liyuqian Jun 24, 2020

Choose a reason for hiding this comment

liyuqian Jun 24, 2020

Choose a reason for hiding this comment

freiling Jun 24, 2020

Choose a reason for hiding this comment

liyuqian Jun 24, 2020

Choose a reason for hiding this comment

freiling Jun 24, 2020

Choose a reason for hiding this comment

arbreng left a comment

Choose a reason for hiding this comment

arbreng Jul 2, 2020

Choose a reason for hiding this comment

freiling commented Jun 24, 2020 •

edited