Wire up Metal shader precompilation from offline training runs. #25644

chinmaygarde · 2021-04-19T08:20:53Z

This commit depends on Skia support for Metal SKSL precompilation.

When a new Metal onscreen context is acquired, the engine will attempt to load
shaders stored in the io.flutter.shaders.json file packaged in the Flutter
asset bundle. This file can be obtained via offline training runs.

This works very similarly to the OpenGL backend. However, in the OpenGL backend,
shell initialization waits for precompilation to be completed. On the other
hand, in the Metal backend, precompilation happens immediately after shell
initialization. This is in an attempt to parallelize SKSL precompilation with
Dart isolate and framework setup. While it may be possible to parallelize
precompilation of SKSLs in the future, they are processed serially today.

Support for testing shells (that hold the Skia persistent cache) using the Metal
backend did not exist and has been added in this patch. However, I don't believe
this testing is sufficient because it only verifies that SKSLs can be dumped and
subsequently reused with a Metal backed Shell. Importantly, it doesn't verify
where is shell setup this precompilation happens. For that, the persistent cache
interface will need to be reworked to better fit with the shell unittests. I
considered it out of scope and will followup with those test updates in a later
patch.

I have also made tracing of precompilation consistent with OpenGL and Metal (and
Vulkan when support for that is added). The
PersistentCache::PrecompileKnownSKSLs trace will summarize the number of SKSLs
being precompiled (it is a counter trace) and the time taken to precompile each.

Fixes flutter/flutter#79298.

These traces show that precompilation is functional and occurs during application launch.

This commit depends on [Skia support for Metal SKSL precompilation][1]. When a new Metal onscreen context is acquired, the engine will attempt to load shaders stored in the `io.flutter.shaders.json` file packaged in the Flutter asset bundle. This file can be [obtained via offline training runs][2]. This works very similarly to the OpenGL backend. However, in the OpenGL backend, shell initialization waits for precompilation to be completed. On the other hand, in the Metal backend, precompilation happens immediately after shell initialization. This is in an attempt to parallelize SKSL precompilation with Dart isolate and framework setup. While it may be possible to parallelize precompilation of SKSLs in the future, they are processed serially today. Support for testing shells (that hold the Skia persistent cache) using the Metal backend did not exist and has been added in this patch. However, I don't believe this testing is sufficient because it only verifies that SKSLs can be dumped and subsequently reused with a Metal backed Shell. Importantly, it doesn't verify where is shell setup this precompilation happens. For that, the persistent cache interface will need to be reworked to better fit with the shell unittests. I considered it out of scope and will followup with those test updates in a later patch. I have also made tracing of precompilation consistent with OpenGL and Metal (and Vulkan when support for that is added). The `PersistentCache::PrecompileKnownSKSLs` trace will summarize the number of SKSLs being precompiled (it is a counter trace) and the time taken to precompile each. Fixes flutter/flutter#79298. [1]: https://bugs.chromium.org/p/skia/issues/detail?id=11392 [2]: https://flutter.dev/docs/perf/rendering/shader#how-to-use-sksl-warmup

jason-simmons · 2021-04-19T19:10:42Z

common/graphics/persistent_cache.cc

+    return 0;
+  }
+
+  if (known_sksls.empty()) {


This is harmless but redundant. The loop will do nothing if there are no shaders.

jason-simmons · 2021-04-19T19:11:21Z

common/graphics/persistent_cache.h

+
+  //----------------------------------------------------------------------------
+  /// @brief      Precompile SKSLs packaged with the application and gathered
+  ///             during during previous runs in the given context.


typo: "during during"

jason-simmons · 2021-04-19T19:12:50Z

shell/common/persistent_cache_unittests.cc

-TEST_F(ShellTest, CacheSkSLWorks) {
+static void WaitForRaster(Shell* shell) {
+  std::promise<bool> raster_task_finished;
+  shell->GetTaskRunners().GetIOTaskRunner()->PostTask(


This should be GetRasterTaskRunner

Oof. Good catch. I added this to prevent a potential race.

jason-simmons · 2021-04-19T19:18:30Z

shell/common/persistent_cache_unittests.cc

+    }
+    return true;
+  };
+  fml::VisitFiles(dir.fd(), remove_visitor);


Can fml::RemoveFilesInDirectory be used here?

Yeah, you're right. I just copied the test right above this one which was probably written before the utility was available. Updated both.

jason-simmons · 2021-04-19T19:22:33Z

shell/gpu/gpu_surface_metal.mm

@@ -155,6 +168,10 @@

 // |Surface|
 std::unique_ptr<GLContextResult> GPUSurfaceMetal::MakeRenderContextCurrent() {
+  // A context may be either be necessary to render to the surface or to snapshot an offscreen


typo: "be either be"

jason-simmons · 2021-04-19T19:25:37Z

common/graphics/persistent_cache.cc

@@ -191,7 +192,35 @@ sk_sp<SkData> ParseBase64(const std::string& input) {
  return data;
 }

-std::vector<PersistentCache::SkSLCache> PersistentCache::LoadSkSLs() {
+size_t PersistentCache::PrecompileKnownSKSLs(GrDirectContext* context) const {


In Skia and elsewhere in the engine the spelling is "SkSL". I would prefer using that spelling in this PR for consistency.

Roger. I gsubbed all instances in //flutter for consistency.

chinmaygarde

Addressed all comments. PTAL. The presubmit failures were because I had assumed that GL emulation was available everywhere. This is not the case of Fuchsia. Fixed licenses which seem to be including test files as well 🤷🏽 .

chinmaygarde · 2021-04-19T20:04:34Z

common/graphics/persistent_cache.cc

@@ -191,7 +192,35 @@ sk_sp<SkData> ParseBase64(const std::string& input) {
  return data;
 }

-std::vector<PersistentCache::SkSLCache> PersistentCache::LoadSkSLs() {
+size_t PersistentCache::PrecompileKnownSKSLs(GrDirectContext* context) const {


Roger. I gsubbed all instances in //flutter for consistency.

chinmaygarde · 2021-04-19T20:04:51Z

common/graphics/persistent_cache.cc

+    return 0;
+  }
+
+  if (known_sksls.empty()) {


chinmaygarde · 2021-04-19T20:05:57Z

common/graphics/persistent_cache.h

+
+  //----------------------------------------------------------------------------
+  /// @brief      Precompile SKSLs packaged with the application and gathered
+  ///             during during previous runs in the given context.


chinmaygarde · 2021-04-19T20:06:22Z

shell/common/persistent_cache_unittests.cc

-TEST_F(ShellTest, CacheSkSLWorks) {
+static void WaitForRaster(Shell* shell) {
+  std::promise<bool> raster_task_finished;
+  shell->GetTaskRunners().GetIOTaskRunner()->PostTask(


Oof. Good catch. I added this to prevent a potential race.

chinmaygarde · 2021-04-19T21:49:10Z

shell/common/persistent_cache_unittests.cc

+    }
+    return true;
+  };
+  fml::VisitFiles(dir.fd(), remove_visitor);


Yeah, you're right. I just copied the test right above this one which was probably written before the utility was available. Updated both.

chinmaygarde · 2021-04-19T21:49:40Z

shell/gpu/gpu_surface_metal.mm

@@ -155,6 +168,10 @@

 // |Surface|
 std::unique_ptr<GLContextResult> GPUSurfaceMetal::MakeRenderContextCurrent() {
+  // A context may be either be necessary to render to the surface or to snapshot an offscreen


acoutts · 2021-04-19T23:41:01Z

@chinmaygarde to use this we just need to use the same as under OpenGL with --cache-sksl and --bundle-sksl-path ?

chinmaygarde · 2021-04-19T23:43:30Z

Right. The workflow is identical.

acoutts · 2021-04-20T02:11:11Z

Thanks @chinmaygarde - just to confirm, the latest master has it?

Flutter 2.2.0-11.0.pre.168 • channel master • https://github.com/flutter/flutter.git
Framework • revision ecbea9a9d5 (4 hours ago) • 2021-04-20 03:44:04 +0530
Engine • revision 60f12daf85
Tools • Dart 2.14.0 (build 2.14.0-8.0.dev)

…s. (flutter/engine#25644)

zanderso · 2021-04-20T15:56:03Z

@acoutts, @chinmaygarde's flutter/engine commit is still making its way into flutter/flutter master. It's not there yet. flutter/flutter#80769 looks like it will succeed when the flutter/flutter tree turns green.

…ter#25644)

chinmaygarde requested a review from jason-simmons April 19, 2021 08:20

google-cla bot added the cla: yes label Apr 19, 2021

jason-simmons reviewed Apr 19, 2021

View reviewed changes

Address review comments.

a9ba734

chinmaygarde commented Apr 19, 2021

View reviewed changes

jason-simmons approved these changes Apr 19, 2021

View reviewed changes

chinmaygarde added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 19, 2021

fluttergithubbot merged commit 7802d58 into flutter:master Apr 19, 2021

chinmaygarde deleted the mtl_sksl_offline_training branch April 19, 2021 23:25

engine-flutter-autoroll mentioned this pull request Apr 20, 2021

Roll Engine from 7f4d7c67a5a1 to 79c5ebe83504 (11 revisions) flutter/flutter#80769

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 20, 2021

7802d58 Wire up Metal shader precompilation from offline training run…

c50dc5a

…s. (flutter/engine#25644)

GuoXi pushed a commit to GuoXi/engine that referenced this pull request Apr 25, 2021

Wire up Metal shader precompilation from offline training runs. (flut…

0bfc0ed

…ter#25644)

gw280 mentioned this pull request Jan 22, 2022

Post-submit Mac Host Engine is 15.95% flaky for past 30 days flutter/flutter#96807

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wire up Metal shader precompilation from offline training runs. #25644

Wire up Metal shader precompilation from offline training runs. #25644

chinmaygarde commented Apr 19, 2021

jason-simmons Apr 19, 2021

chinmaygarde Apr 19, 2021

jason-simmons Apr 19, 2021

chinmaygarde Apr 19, 2021

jason-simmons Apr 19, 2021

chinmaygarde Apr 19, 2021

jason-simmons Apr 19, 2021

chinmaygarde Apr 19, 2021

jason-simmons Apr 19, 2021

chinmaygarde Apr 19, 2021

jason-simmons Apr 19, 2021

chinmaygarde Apr 19, 2021

chinmaygarde left a comment

chinmaygarde Apr 19, 2021

chinmaygarde Apr 19, 2021

chinmaygarde Apr 19, 2021

chinmaygarde Apr 19, 2021

chinmaygarde Apr 19, 2021

chinmaygarde Apr 19, 2021

acoutts commented Apr 19, 2021

chinmaygarde commented Apr 19, 2021

acoutts commented Apr 20, 2021

zanderso commented Apr 20, 2021

Wire up Metal shader precompilation from offline training runs. #25644

Wire up Metal shader precompilation from offline training runs. #25644

Conversation

chinmaygarde commented Apr 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chinmaygarde left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acoutts commented Apr 19, 2021

chinmaygarde commented Apr 19, 2021

acoutts commented Apr 20, 2021

zanderso commented Apr 20, 2021