This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Skwasm single threaded #56206
Merged
Skwasm single threaded #56206
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
harryterkelsen
approved these changes
Oct 30, 2024
Contributor
harryterkelsen
left a comment
harryterkelsen
left a comment
There was a problem hiding this comment.
LGTM with a couple questions
| ); | ||
|
|
||
| if (await _checkSkiaClient(skiaClient)) { | ||
| print('Successfully checked Skia Gold Client'); |
Contributor
There was a problem hiding this comment.
Are these leftover debug prints or are they intended to be part of the output (e.g. to help debug flaky tests on CI)
Contributor
Author
There was a problem hiding this comment.
I found them actually kind of useful enough to leave in. They only are printed once for each suite when they run, so they aren't particularly chatty. If you don't like them in here I can remove them, but it's actually nice to be able to make sure we created a skia gold client with the right dimensions.
Contributor
There was a problem hiding this comment.
Seems fine to keep in, then.
…from package_lock.yaml for browser versions.
eyebrowsoffire
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Oct 31, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Oct 31, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Oct 31, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Oct 31, 2024
Contributor
|
reason for revert: failing to roll into framework: flutter/flutter#157919 |
jonahwilliams
added
the
revert
auto-submit bot
pushed a commit
that referenced
this pull request
Oct 31, 2024
This reverts commit 621e68c.
auto-submit
bot
removed
the
revert
auto-submit bot
added a commit
that referenced
this pull request
Oct 31, 2024
Reverts: #56206 Initiated by: jonahwilliams Reason for reverting: failing to roll into framework: flutter/flutter#157919 Original PR Author: eyebrowsoffire Reviewed By: {harryterkelsen} This change reverts the following previous change: This PR creates a single-threaded version of the skwasm renderer, appropriate for non-crossOriginIsolated browsing contexts. * The single threaded renderer is essentially the same as the multi-threaded renderer, except instead of spawning a web worker and posting messages to it, it simply schedules microtasks on the main thread in their place. * The new renderer is vended as `skwasm_st.js` and `skwasm_st.wasm` in the same location as multithreaded skwasm. In order to properly build and function, we needed some fixes I put into emscripten that landed in version 3.1.70. That version also changed some behavior that required a few fixes to the CanvasKit build files. * The skwasm loader in flutter.js has been modified to use the skwasm_st variants when encountering a non-crossOriginIsolated context but a browser and configuration that otherwise would allow the use of skwasm. I also added a new `forceSingleThreadedSkwasm` option to the flutter configuration so that we can override this behavior, especially so that we can accurately benchmark the single threaded renderer in a crossOriginIsolated environment. * I also consolidated a bunch of our shards that run tests to just have one per browser/platform combination, so four total. This will address flutter/flutter#124682
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Oct 31, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Oct 31, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Oct 31, 2024
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Oct 31, 2024
…157926) flutter/engine@c40b0b6...f2154ef 2024-10-31 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Skwasm single threaded (#56206)" (flutter/engine#56264) 2024-10-31 [email protected] Roll Skia from 4f8f2ecadfb6 to 3c628426f85f (1 revision) (flutter/engine#56261) 2024-10-31 [email protected] Roll Skia from 7e79a516284b to 4f8f2ecadfb6 (1 revision) (flutter/engine#56255) 2024-10-31 [email protected] Roll Dart SDK from 6a8058eef22c to f3e3dc44b1dc (1 revision) (flutter/engine#56253) 2024-10-31 [email protected] Roll Skia from 3c62d4a94d78 to 7e79a516284b (1 revision) (flutter/engine#56252) 2024-10-31 [email protected] [Impeller] use primitive restart for faster tessellation: write directly into host buffer. (flutter/engine#56173) 2024-10-31 [email protected] Skwasm single threaded (flutter/engine#56206) 2024-10-31 [email protected] [Impeller] expose reference to tessellator instead of shared_ptr. (flutter/engine#56244) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
eyebrowsoffire
added a commit
to eyebrowsoffire/engine
that referenced
this pull request
Oct 31, 2024
…)" This reverts commit f2154ef.
auto-submit bot
pushed a commit
that referenced
this pull request
Nov 1, 2024
This attempts to reland the single-threaded Skwasm PR: #56206 The main changes here are in the second commit: * We need to actually bundle the `skwasm_st` artifacts in `flutter_web_sdk.zip` * The `locateFile` hack no longer works since emscripten doesn't actually create a worker.js file anymore. So instead, that has been modified to use the `mainScriptUrlOrBlob` module API to do a very similar hack. Note: I did presubmit testing with the framework CI and it appears the pertinent tests pass. See flutter/flutter#157967
|
Heya, very cool work! I was trying this out, but ran into the following error and the web app doesn't load for me. What I tried:
The following error is thrown: I've also sent an email with some of this info to @kevmoo since he's been coordinating with us to get our app running with WASM, but I thought I'd put more info here. I'm excited to get WASM going! So far, we've seen some big performance improvements with the multi-threaded renderer, but we have some use cases which don't work with the restrictive COEP & COOP headers. Thanks for all the effort. Could this be related to this part of your comment?
|
|
Ah wait, my bad -- I looked at the project code for a fresh project vs our custom code we have. We had actually enabled a couple of performance optimizations for firefox with JS, but that was a bad idea with the new wasm renderer. Apologies for the noise and thanks again for the contribution. Here are the offending lines I had to remove: |
Contributor
Author
|
@brian-superlist Ah good to hear. Yes the skwasm web renderer requires those APIs so that makes sense. Glad to hear it's working! |
nick9822
pushed a commit
to nick9822/flutter
that referenced
this pull request
Dec 18, 2024
This PR creates a single-threaded version of the skwasm renderer, appropriate for non-crossOriginIsolated browsing contexts. * The single threaded renderer is essentially the same as the multi-threaded renderer, except instead of spawning a web worker and posting messages to it, it simply schedules microtasks on the main thread in their place. * The new renderer is vended as `skwasm_st.js` and `skwasm_st.wasm` in the same location as multithreaded skwasm. In order to properly build and function, we needed some fixes I put into emscripten that landed in version 3.1.70. That version also changed some behavior that required a few fixes to the CanvasKit build files. * The skwasm loader in flutter.js has been modified to use the skwasm_st variants when encountering a non-crossOriginIsolated context but a browser and configuration that otherwise would allow the use of skwasm. I also added a new `forceSingleThreadedSkwasm` option to the flutter configuration so that we can override this behavior, especially so that we can accurately benchmark the single threaded renderer in a crossOriginIsolated environment. * I also consolidated a bunch of our shards that run tests to just have one per browser/platform combination, so four total. This will address flutter#124682
nick9822
pushed a commit
to nick9822/flutter
that referenced
this pull request
Dec 18, 2024
Reverts: flutter/engine#56206 Initiated by: jonahwilliams Reason for reverting: failing to roll into framework: flutter#157919 Original PR Author: eyebrowsoffire Reviewed By: {harryterkelsen} This change reverts the following previous change: This PR creates a single-threaded version of the skwasm renderer, appropriate for non-crossOriginIsolated browsing contexts. * The single threaded renderer is essentially the same as the multi-threaded renderer, except instead of spawning a web worker and posting messages to it, it simply schedules microtasks on the main thread in their place. * The new renderer is vended as `skwasm_st.js` and `skwasm_st.wasm` in the same location as multithreaded skwasm. In order to properly build and function, we needed some fixes I put into emscripten that landed in version 3.1.70. That version also changed some behavior that required a few fixes to the CanvasKit build files. * The skwasm loader in flutter.js has been modified to use the skwasm_st variants when encountering a non-crossOriginIsolated context but a browser and configuration that otherwise would allow the use of skwasm. I also added a new `forceSingleThreadedSkwasm` option to the flutter configuration so that we can override this behavior, especially so that we can accurately benchmark the single threaded renderer in a crossOriginIsolated environment. * I also consolidated a bunch of our shards that run tests to just have one per browser/platform combination, so four total. This will address flutter#124682
nick9822
pushed a commit
to nick9822/flutter
that referenced
this pull request
Dec 18, 2024
This attempts to reland the single-threaded Skwasm PR: flutter/engine#56206 The main changes here are in the second commit: * We need to actually bundle the `skwasm_st` artifacts in `flutter_web_sdk.zip` * The `locateFile` hack no longer works since emscripten doesn't actually create a worker.js file anymore. So instead, that has been modified to use the `mainScriptUrlOrBlob` module API to do a very similar hack. Note: I did presubmit testing with the framework CI and it appears the pertinent tests pass. See flutter#157967
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR creates a single-threaded version of the skwasm renderer, appropriate for non-crossOriginIsolated browsing contexts.
skwasm_st.jsandskwasm_st.wasmin the same location as multithreaded skwasm. In order to properly build and function, we needed some fixes I put into emscripten that landed in version 3.1.70. That version also changed some behavior that required a few fixes to the CanvasKit build files.forceSingleThreadedSkwasmoption to the flutter configuration so that we can override this behavior, especially so that we can accurately benchmark the single threaded renderer in a crossOriginIsolated environment.