-
-
Notifications
You must be signed in to change notification settings - Fork 841
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
web: Enable the bulk-memory WebAssembly extension #5225
Conversation
83f4426
to
62b31b5
Compare
Can you mark it as either draft or smth like "[DO NOT MERGE YET]"? Just in case :) Also, when the time comes, let's also test AVM-heavy code. I did find cases where extremely AVM-heavy code was slightly slower as |
32c7891
to
ebaa995
Compare
I switched from setting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! LGTM
ebaa995
to
ff24c2e
Compare
I'd appreciate it if someone on Windows could check if the new condition for the |
Unfortunately the windows setting in config.toml doesn't work because of rust-lang/cargo#6858. I think we'll have to stick with setting RUSTFLAGS on CI for that. |
Oh... :/ Thanks for checking! How about the original, less ideal version?
|
I'm just worried about the potential conflict where the CI setup (including the action building the nightly releases) sets the |
Considering this is bound to wait for a couple more months, maybe for now we could just wait and see if toolchains figure out something nicer? I think long-term, rustc itself will be switching some of wasm features on by default. |
This should be fine -- I also tried using Should we think about shipping multiple WASM modules with features enabled/disabled, and loading the appropriate module based on browser capabilities? Maybe bulk extensions will be ubiquitous enough that it's not a big deal, but there will be other features that we may want to selectively enable as well (webgpu, SIMD, interface types, ...). This might allow us to keep running on slightly older browsers while these features roll out. |
Partly I'm also hoping for what @adrian17 wrote: That over (hopefully not too long) time, rustc/cargo itself will enable the "most important" extensions by default, so we won't have to deal with them, and can delegate the responsibility for the effects of this decision to the Rust team. But I can also imagine building even just two WASM modules: one for the "happy path" (when support is detected for all the extensions we want), and one for compatibility (which enables no extensions at all, like now). This would complicate the build process and the loader code, result in slightly larger releases and hosted package, and might even bring difficulties when debugging user issues... - for perhaps too slight a benefit. (Let it be some extra performance now, or legacy compatibility later...) Whether it's worth it or not, is of course not an easy question. (Well, I, for one, am silently rooting that something like this can be made to work, but I understand that it's not trivial and not without downsides, so...) |
Safari 15 is now out!!!! Everyone, update right away!! |
With Safari 14.1 already starting to slowly fade out in favor of 15.0, I'll leave the judgement to merge this at any time to someone else. Here is the download link for the CSV data for these charts. |
I'm going to point out that, unlike prior versions, Apple's decided to keep iOS 14 installable for an additional year. This may hamper adoption. |
Oh no... :/ |
Safari and iOS are one single thing - you can't update them independently. Also, Apple doesn't allow shipping third-party browser engines, so all alternate browsers on iOS are using the same version of Safari that ships with the OS. |
Yeah, I knew about the web engine restriction, but not about last-gen iOS being stuck with the same version it came with... That's a bummer. Anyway, I can wait! |
ff24c2e
to
2b226dc
Compare
Getting a panic on |
I think the panic in |
2b226dc
to
2658651
Compare
I guess we should decide when to pull the trigger on this -- possibly when 15.0 becomes majority, or wait even longer? Can iOS 15.0 run on every device that 14.0 can? We should also have a "you need bulk memory/update your browser" error message in place.
Sorry, this comment was supposed to be on the VP6 PR! |
Here's yet another fancy graph, this time with linear extrapolations based on the last 1, 2, and 3 weeks.
I, for one, have no idea. However, at least according to this and this list, looks like it should be the case. The latter includes the iPhone 13 variants, but no other difference.
Would we want something like https://github.com/GoogleChromeLabs/wasm-feature-detect for this, or just another rung in the if-ladder in |
If we can deduce this specific error from the panic, it'd be nice if we can avoid the dependency. But if this is difficult, we can rely on |
2658651
to
b439451
Compare
f98f0eb
to
e9f7363
Compare
@torokati44 assuming the same website is correct, here is the iOS market share by version which also tells us the Safari version as they are updated together. And here is the last 3 months Link: |
Superseded by #5834! Take that, Safari! 😃 |
This should improve all around performance on web, to a varying degree.
I'll gladly provide benchmark numbers upon request; but so far I've seen that at least while decoding videos, this should save anywhere between 10% and 20% of processing time, "for free".
There are still quite a few calls to the naive
memcpy
andmemset
functions (explicit loop and load/store in WASM), mostly fromcore
andstd
.To get rid of these as well, the
build-std
cargo feature would be needed (which does something similar to what Xargo was for), but that is a nightly-only flag as of now, because there are still some issues with it.This raises the minimum browser version requirements to:
Chrome 75 (released on 2019-06-05): https://v8.dev/blog/v8-release-75#bulk-memory-operations, https://www.chromestatus.com/feature/4590306448113664Chrome 87 is already required
Firefox 79 (released on 2020-06-28): https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/79#webassemblyFirefox 84 is already required
I know basically nothing about the habits of 🍎 ecosystem users, but based on past data, Safari 15.0 is supposed to be the most used version by November, and is supposed to reach near its top market share by about January or February:
Source: https://gs.statcounter.com/browser-version-market-share
So, I don't think this should be merged for at least a couple weeks, but we can still talk about it here until then. :)