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

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

Closed
chinmaygarde opened this issue Mar 29, 2021 · 86 comments · Fixed by flutter/engine#25644
Closed

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

chinmaygarde opened this issue Mar 29, 2021 · 86 comments · Fixed by flutter/engine#25644
Assignees
Labels
engine flutter/engine repository. See also e: labels. P2 Priority 2 issue likely blocking a tier-1 customer soon. passed first triage tests are present, the PR follows the PR template, no obvious coding errors severe: performance Relates to speed or footprint issues. severe: rendering UI glitches reported at the engine/skia rendering level. waiting for PR to land (fixed) A fix is in flight.

Comments

@chinmaygarde
Copy link
Member

Now that https://bugs.chromium.org/p/skia/issues/detail?id=11392 has landed and been rolled in, preparation of Metal pipelines from harvested SKPs can be implemented. This is a tracking bug to make sure lifecycle and threading implications of the attempt at engine launch are reasoned about before enabling this on all renderer launch attempts.

@chinmaygarde chinmaygarde added engine flutter/engine repository. See also e: labels. severe: performance Relates to speed or footprint issues. severe: rendering UI glitches reported at the engine/skia rendering level. labels Mar 29, 2021
@chinmaygarde chinmaygarde self-assigned this Mar 29, 2021
@TahaTesser TahaTesser added the passed first triage tests are present, the PR follows the PR template, no obvious coding errors label Mar 30, 2021
@zanderso zanderso added this to On deck for Engine in Early-onset jank Apr 2, 2021
@chinmaygarde chinmaygarde added the P2 Priority 2 issue likely blocking a tier-1 customer soon. label Apr 5, 2021
@Hixie Hixie moved this from On deck for Engine to Ongoing projects in Early-onset jank Apr 5, 2021
@chinmaygarde
Copy link
Member Author

The precompilation of harvested SKSL's has been wired up. Working on testing and benchmarking this in the host harness.

chinmaygarde added a commit to chinmaygarde/flutter_engine that referenced this issue Apr 19, 2021
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.

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
chinmaygarde added a commit to chinmaygarde/flutter_engine that referenced this issue Apr 19, 2021
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
@chinmaygarde chinmaygarde added the waiting for PR to land (fixed) A fix is in flight. label Apr 19, 2021
@acoutts
Copy link

acoutts commented Apr 20, 2021

Is this available in master to try now?

@CavalcanteLeo
Copy link

CavalcanteLeo commented Apr 20, 2021

will this fix iOS shader jank issue? (using metal)

@acoutts
Copy link

acoutts commented Apr 20, 2021

will this fix iOS shader jank issue?

See the guide here on how to utilize it: https://flutter.dev/docs/perf/rendering/shader#how-to-use-sksl-warmup
This should let you significantly reduce it.

@zaidkazi
Copy link

zaidkazi commented Apr 20, 2021

I thought that tutorial doeisnt work on metal @acoutts

@alectogeek
Copy link

@zaidkazi and that is why this issue exists.
This PR lets to use the warm-up on metal as well.

@zaidkazi
Copy link

@alectogeek ok cool thanks lol that's what I figured sorry back to the original question is this already available in master ? And are any changes required from the initial tutorial?

@drakmail
Copy link

will this fix iOS shader jank issue?

See the guide here on how to utilize it: https://flutter.dev/docs/perf/rendering/shader#how-to-use-sksl-warmup
This should let you significantly reduce it.

Seems awesome! Do you know, how to use shaders warmup with fastlane (I'm deploying app to appstore according to https://flutter.dev/docs/deployment/cd)?

@acoutts
Copy link

acoutts commented Apr 20, 2021

will this fix iOS shader jank issue?

See the guide here on how to utilize it: https://flutter.dev/docs/perf/rendering/shader#how-to-use-sksl-warmup
This should let you significantly reduce it.

Seems awesome! Do you know, how to use shaders warmup with fastlane (I'm deploying app to appstore according to https://flutter.dev/docs/deployment/cd)?

You just need to add it to your build command, as per that linked guide.

@drakmail
Copy link

will this fix iOS shader jank issue?

See the guide here on how to utilize it: https://flutter.dev/docs/perf/rendering/shader#how-to-use-sksl-warmup
This should let you significantly reduce it.

Seems awesome! Do you know, how to use shaders warmup with fastlane (I'm deploying app to appstore according to https://flutter.dev/docs/deployment/cd)?

You just need to add it to your build command, as per that linked guide.

I don't use build command. My CI script:

cd ios
pod install
bundle install
bundle exec fastlane beta

and my Fastfile:

default_platform(:ios)

platform :ios do
  desc "Push a new beta build to TestFlight"
  lane :beta do
    increment_build_number(build_number: "$CI_PIPELINE_IID")
    build_app(workspace: "Runner.xcworkspace", scheme: "Runner")
    upload_to_testflight
  end
end

hmm, looked up the documentation for gym, used by fastlane, seems that I can use xcargs parameter for build_app

xcargs | Pass additional arguments to xcodebuild for the build phase. Be sure to quote the setting names and values e.g. OTHER_LDFLAGS="-ObjC -lstdc++"

@erf
Copy link

erf commented Apr 20, 2021

Is it possible to run the app with the pre-compiled shaders and not building it?

To test out if the "fix-yank-step" actually works before doing the build command. Not sure if i make sense or this is a silly question.

@acoutts
Copy link

acoutts commented Apr 20, 2021

Is it possible to run the app with the pre-compiled shaders and not building it?

To test out if the "fix-yank-step" actually works before doing the build command. Not sure if i make sense or this is a silly question.

You need to build an adhoc-signed IPA and drag/drop it in the Xcode devices window onto the device to install it.

@erf
Copy link

erf commented Apr 20, 2021

It would not be possible to add a command like: flutter run --with-sksl flutter_01.sksl.json ?

@omchiii
Copy link
Contributor

omchiii commented Apr 20, 2021

It actually works!

Device : Iphone 8

What I did was switch to flutter master branch and flutter upgrade, followed the same steps on https://flutter.dev/docs/perf/rendering/shader & built an adhoc-signed IPA and drag/drop it in the Xcode devices window onto the device to install

VIdeo:

Wihout shader warm up(first run): https://drive.google.com/file/d/1_cHkMleeI-eRhxehq4ikF0Gv3ZE9CTBE/view?usp=sharing
With shader warm up(first run & deleted app & restarted device before installing again) : https://drive.google.com/file/d/1m6a5heZBO_61qpj6Eu8p7QM8RpNOQ6nL/view?usp=sharing

PS. I used the code from #76180

@stx
Copy link

stx commented Apr 20, 2021

A huge improvement for iOS. Nice job, Flutter team.

@alectogeek
Copy link

What about portability to other devices?
Is it also (the same) effective if you warm up on one iPhone model and run the app on another?

@m-j-g
Copy link

m-j-g commented May 3, 2021

is this available in the dev branch or still only on master?

@zanderso
Copy link
Member

zanderso commented May 3, 2021

@m-j-g Yes, this is available on the dev branch.

@Hixie Hixie moved this from Ongoing projects to Closed in Early-onset jank May 3, 2021
@jeremiahlukus
Copy link

Is there anyway to run this locally?
like flutter run --skal-path flutter_01.sksl.json ?

@lukepighetti
Copy link

Is this available in 2.2 on stable? I can't seem to figure out exactly where this is.

@bdlukaa
Copy link

bdlukaa commented May 19, 2021

It's currently on the dev channel @lukepighetti

@bennibau
Copy link

bennibau commented May 20, 2021

Is it possible to run the app with the pre-compiled shaders and not building it?

To test out if the "fix-yank-step" actually works before doing the build command. Not sure if i make sense or this is a silly question.

Yes please use a command like this:

flutter run --profile --bundle-sksl-path flutter_01.sksl.json

please make sure you are on the DEV channel as of right now

I have tried this solution on the example project as mentioned above and also on our production version which is already a huge app. It is MASSIVELY improving the first jank issue. It is basically gone.

@FelixYew
Copy link

I followed the instructions on https://flutter.dev/docs/perf/rendering/shader#how-to-use-sksl-warmup but when I press M on the command line nothing happen, no file or folder created. Can someone hep me? Thank you.

[✓] Flutter (Channel dev, 2.3.0-16.0.pre, on macOS 11.4 20F71 darwin-x64, locale en-GB)
• Flutter version 2.3.0-16.0.pre at /Users/felix/flutter
• Upstream repository https://github.com/flutter/flutter.git
• Framework revision fa5883b (9 days ago), 2021-05-21 13:04:03 -0700
• Engine revision 2f067fc
• Dart version 2.14.0 (build 2.14.0-136.0.dev)

[✓] Xcode - develop for iOS and macOS
• Xcode at /Applications/Xcode.app/Contents/Developer
• Xcode 12.5, Build version 12E262
• CocoaPods version 1.10.1

[✓] Chrome - develop for the web
• Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 4.2)
• Android Studio at /Applications/Android Studio.app/Contents
• Flutter plugin can be installed from:
🔨 https://plugins.jetbrains.com/plugin/9212-flutter
• Dart plugin can be installed from:
🔨 https://plugins.jetbrains.com/plugin/6351-dart
• Java version OpenJDK Runtime Environment (build 11.0.8+10-b944.6916264)

@bennibau
Copy link

@FelixYew you have to press Shift + m which results in M

@MarcoSoares11
Copy link

I have tested the SKSL Shader on my latest app version and the results are amazing, however, I noticed also that the boot up time of the app to start after the splash screen is significantly longer.

Would that be because of the dev channel?

@zanderso
Copy link
Member

@MarcoSoares11 The bundled SkSL shaders are precompiled to machine code on first-run when the application starts. This delay will persist even after the feature makes it to the beta and stable channels. You can reduce the delay by bundling fewer shaders. (We understand this is not a great user experience and we are continuing to pursue improvements in this area.)

@SM2DevLLC
Copy link

SM2DevLLC commented Jun 2, 2021

@zanderso Dumb question, but how do I have multiple devices in one file if it creates a new file every time I hit shift + M? I use theflutter run --profile --cache-sksl command and then press shift + M after I run all the animations on my device. I'd need to stop running the app to run it on another devices, so i'm not quite sure how I handle iPad vs iPhone vs Android phone etc.

@bennibau
Copy link

bennibau commented Jun 2, 2021

The animations are stored as shader code. The code is compiled on startup for your device specific. So in theory you have to do it only on one device because it is not device specific.

@SM2DevLLC
Copy link

@bennibau It says "For best results, capture SkSL shaders on actual Android and iOS devices separately." in the guide and on top of that animations are usually a bit different based on tablet vs phone etc. Is there not a way to combine all the shaders in one file?

@MarcoSoares11
Copy link

@SM2DevLLC I use my personal iPhone to capture the SkSL shaders and it has an incredible performance when I test on Android. For best practices if you have access to both an Android and an iOS device it would be recommended to do the capturing in separate devices as the device is specified in the SkSL file.

@SM2DevLLC
Copy link

@MarcoSoares11 Thanks. Yeah I have been doing both devices, but that's the issue. Each device makes a different file. So iPhone, iPad, Android, etc. Is there a way to combine the files? The other issue is that the landscape iPad view is different, so my assumption would be the shaders are slightly different, so i'd need to have the iPad and iPhone version in one file.

@zanderso
Copy link
Member

zanderso commented Jun 2, 2021

@SM2DevLLC Bundling multiple SkSLs is not yet implemented. Please give a thumbs-up to #73098 to indicate your interest in that feature.

@SM2DevLLC
Copy link

@zanderso Oh ok. So I can't really do iPad and iPhone since they would need to be in the same file, so i'd need to pick one for now to build for release?

@b3nni97
Copy link

b3nni97 commented Jun 4, 2021

I have the problem that I publish my app via Xcode and how can I attach the sksl file there? If I go via Xcode -> Product -> Archive then I can't set anything else there.

@alectogeek
Copy link

I have the problem that I publish my app via Xcode and how can I attach the sksl file there? If I go via Xcode -> Product -> Archive then I can't set anything else there.

@b3nni97 you don't have to attach the sksl file manually. It is enough to use flutter build ios --bundle-sksl-path your-file-name.sksl.json to build an app and after that, you can archive and publish via xCode

@armandojimenez
Copy link

Tried it on my app, it really reduce jank (Not all, there still jank, I know is jank because it dissapear after third open)... but, now my app takes more than 10 secs on the splash screen, and sometime when I restart the phone, it can take more thank 30 sec. Only happens on first app install, and when I restart the phone. On a iPhone 11 Pro Max

@armandojimenez
Copy link

Question, when I do the warm process, Do i only need to trigger the animation with jank only one time? or do I need to run it mutiple times until it runs smooth? does it affect at all the size of the .json and app start time?

@m-j-g
Copy link

m-j-g commented Jun 16, 2021

I'm having a hard time tracking where this is committed. Is there a version number that has this implemented? Trying to stay on top of this so that I'm aware when it makes it to beta/stable

@matehat
Copy link

matehat commented Jun 18, 2021

Same here, is there a way to know when we can get out of the dev channel while keeping Metal shader caching?

@zanderso
Copy link
Member

@m-j-g @matehat This will be on beta channel in version 2.3, which I believe should be out soon.

@brandnewx
Copy link

brandnewx commented Jul 13, 2021

I followed the instructions on https://flutter.dev/docs/perf/rendering/shader#how-to-use-sksl-warmup but when I press M on the command line nothing happen, no file or folder created. Can someone hep me? Thank you.

On a Mac, you need to use Shift + m. Yeah who would have thought M means Shift + m? Probably only me. Kind of silly when the command line literally needs the capital M instead of just m. Flutter team as a whole is really awesome! But sometimes I find the new guy coming in and can't code the command line properly. There's nothing in the command line output that says press Shift + m.

Also one more thing, we have flutter build ios --release --bundle-sksl-path but why didn't they also make flutter run --release --bundle-sksl-path too, just to make our lives less miserable so that we won't need to archive into ipa, sign with adhoc and then manually deploy to the device. To make it more head spinning, flutter run --release --no-build actually ignores the --no-build flag and rebuilds anyway. I mean, whoever writes this command line tool seriously needs to sit down and reflect what's going on since 2019 until now 2021.

@joshbenaron
Copy link

@m-j-g @matehat This will be on beta channel in version 2.3, which I believe should be out soon.

Is it on stable yet?

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
engine flutter/engine repository. See also e: labels. P2 Priority 2 issue likely blocking a tier-1 customer soon. passed first triage tests are present, the PR follows the PR template, no obvious coding errors severe: performance Relates to speed or footprint issues. severe: rendering UI glitches reported at the engine/skia rendering level. waiting for PR to land (fixed) A fix is in flight.
Projects
Development

Successfully merging a pull request may close this issue.