Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always cache SkSL when using the Metal backend. #17468

Merged
merged 1 commit into from
Apr 2, 2020

Conversation

chinmaygarde
Copy link
Member

There is no ability to compile shaders online and cache those binaries when
using the Metal backend. SkSL caching must always be used.

@chinmaygarde
Copy link
Member Author

@liyuqian: I was under the impression that SkSL caching was enabled on iOS when using the OpenGL backend. That does not seem to be case from what I could observe. Can you confirm the caching strategy on iOS? AFAIK, iOS does not support OpenGL shader binaries. So, without SkSL caching, there should no shader caching going on at all when using OpenGL. Is that accurate?

@liyuqian
Copy link
Contributor

liyuqian commented Apr 2, 2020

@chinmaygarde :

  1. SkSL cache reading is turned on by default, but SkSL cache writing is turned off by default. The writing is only enabled with --cache-sksl. So if you have SkSL artifacts already written (through --cache-sksl), then all the future runs (without --cache-sksl) should try to read and warm up those SkSL shaders.

  2. The non-binary persistent cache reading and writing is enabled by default on iOS. Such cache is not "SkSL cache" because we're not setting GrContextOptions::ShaderCacheStrategy::kSkSL (but under the hood they might also be using SkSLs). Such non-binary persistent cache is also not warmed up through GrContext::precompileShader. They're only exposed to Skia through PersistentCache::load. It still needs quite some time to compile even with cache hit.

That's what I know. What I don't know is how persistent cache and SkSL shader warmup work with Metal. Somehow, I had an impression that the binary shader persistent cache should work with Metal because it also works with Vulkan, and it seems that Vulkan and Metal have similar pipelines. @brianosman also explicitly doubts whether SkSL warmup would work with Metal. But it seems that your experiment shows that SkSL warm-up does work?

@chinmaygarde
Copy link
Member Author

Thanks for the clarifications, some followup questions:

SkSL cache reading is turned on by default, but SkSL cache writing is turned off by default. The writing is only enabled with --cache-sksl. So if you have SkSL artifacts already written (through --cache-sksl), then all the future runs (without --cache-sksl) should try to read and warm up those SkSL shaders.

Who then writes the SkSLs on iOS? Are SkSLs cached at all when using the OpenGL backend?

The non-binary persistent cache reading and writing is enabled by default on iOS.

My understanding was that it was enabled but never used on iOS because GL_NUM_SHADER_BINARY_FORMATS was 0 there.

So if SkSLs are never written to the cache and the number of binary shader formats supported is 0 (which I may be mistaken on), is there any shader caching at all on iOS?

also explicitly doubts whether SkSL warmup would work with Metal. But it seems that your experiment shows that SkSL warm-up does work?

I am mistaken, both SkSL and non-SkSL shaders should be available for caching. I was looking at the Skia source in GrMtlPipelineStateBuilder.mm and though the if clause extended past this->storeShadersInCache. I'm going to check a few more things and double check non-SkSL shaders are being written.

There is no ability to compile shaders online and cache those binaries when
using the Metal backend. SkSL caching must always be used.
@chinmaygarde
Copy link
Member Author

chinmaygarde commented Apr 2, 2020

Based on my experiments, the Metal backend supports caching both SkSL and MSL. I verified this by specifying that SkSL should NOT be cached and pulling the cached files from iOS. Turns out, Skia was caching MSL directly which is just perfect.
Screen Shot 2020-04-02 at 12 53 57 AM

@liyuqian I have updated the patch now to match the following behavior, let me know if this lines up with your understanding of how things should work. By default, MSL will be cached and loaded by Skia. If the --cache-sksl flag is specified, SKSL will be dumped to a known location and can be packaged with the application. If SkSL is shipped with the application and loaded by the persistent cache, the cost of generating SkSL is ameliorated but the SkSL to MSL step remains. That MSL can in turn be cached again.

This patch is good to go. PTAL.

Copy link
Contributor

@liyuqian liyuqian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. See below for answers to your questions. I also have one question for you: after this patch, how does the 99th percentile and worst frame time change, and does it alleviate the regression in flutter/flutter#53768 ?

Who then writes the SkSLs on iOS? Are SkSLs cached at all when using the OpenGL backend?

Currently no one (by default) on iOS/Android. Home devices are the only one using SkSLs right now. iOS/Android usage is waiting for tool features: flutter/flutter#53116

So if SkSLs are never written to the cache and the number of binary shader formats supported is 0 (which I may be mistaken on), is there any shader caching at all on iOS?

As you observed, there are other cache options than SkSLs and binary. MSL is an example. I believe on OpenGL iOS, that would be plain text GLSL cache.

If SkSL is shipped with the application and loaded by the persistent cache, the cost of generating SkSL is ameliorated but the SkSL to MSL step remains. That MSL can in turn be cached again.

I'm not exactly sure what happens with MSL, so I'll give you the answer for GLSL. If GLSL is cached, it will only be loaded during animations, and there's a still big compilation time from GLSL to binary, so there's only a tiny speedup and the jank mostly remains. If SkSL is cached, all SkSLs will be loaded and compiled to binary during startup, so there will be no jank during animations.

@brianosman
Copy link
Contributor

Caching MSL will be similar to caching GLSL - most of the work still happens after that point. (Both compiling the MSL to Metal's binary format, and compiling "pipelines" that reference the shaders and other render state).

@chinmaygarde
Copy link
Member Author

after this patch, how does the 99th percentile and worst frame time change, and does it alleviate the regression in flutter/flutter#53768 ?

This patch is unrelated to that issue as the time taken the first time around to to generate the shaders is the same with or without the cache. I ran into this issue as I was adding additional traces to verify that issue though.

If SkSL is cached, all SkSLs will be loaded and compiled to binary during startup, so there will be no jank during animations.

I don't believe this will be possible in Metal as entire pipelines will have to be created. So my sense is that SkSL caching will be less of a win for Metal (unless I suppose we took that SkSK and generated MSL and shader binaries from it offline and packaged those binaries into the application, if possible). @brianosman is that understanding correct?

@chinmaygarde chinmaygarde merged commit 0b3f2d3 into flutter:master Apr 2, 2020
@chinmaygarde chinmaygarde deleted the always_cache_sksl branch April 2, 2020 20:00
@liyuqian
Copy link
Contributor

liyuqian commented Apr 2, 2020

So my sense is that SkSL caching will be less of a win for Metal (unless I suppose we took that SkSK and generated MSL and shader binaries from it offline and packaged those binaries into the application, if possible).

Yes, that's my understanding.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 2, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 3, 2020
dnfield pushed a commit to flutter/flutter that referenced this pull request Apr 3, 2020
* 0b3f2d3 Always cache SkSL when using the Metal backend. (flutter/engine#17468)

* ff62dec Roll to clang11, mark 4 (flutter/engine#17483)

* 08ae3bb Remove JSON codec from C++ client wrapper (flutter/engine#17312)

* 2e90965 Fix bad texture view config (flutter/engine#17486)

* abc7293 [pipeline] Add trace event for lag between target and display times (flutter/engine#17384)

* Updated bin/internal/fuchsia-linux.version
goderbauer pushed a commit to goderbauer/engine that referenced this pull request Apr 16, 2020
There is no ability to compile shaders online and cache those binaries when
using the Metal backend. SkSL caching must always be used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants