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

Updating default wasm features #15376

Open
dschuff opened this issue Oct 27, 2021 · 16 comments
Open

Updating default wasm features #15376

dschuff opened this issue Oct 27, 2021 · 16 comments

Comments

@dschuff
Copy link
Member

dschuff commented Oct 27, 2021

Do we have a policy on when we update the default set of wasm features enabled? I feel like we've discussed this a while ago but can't find any issues about it.

Here's the current state of things as best I can find.
Safari shipped the following features in version 14.1, which is included with MacOS Big Sur 11.3 and iOS 14.5 (both in late April 2021): Bigint, signext, and atomics (but not SharedArrayBuffer AFAIK, and not bulk memory)

Safari 15, included in MacOS Monterey 12, iOS 15 (Sept 20, 2021): Streaming compilation, bulk memory, reference types, nontrapping float-to-int.

Safari 15 shipped to MacOS versions older than the new MacOS 12, but I don't think that's true of iOS. (But I think iOS users update their OS faster than macOS users).

Firefox and Chrome have had all of those features for quite a while (other than reference types for some reason, which isn't too relevant for Emscripten).

Of the features Safari shipped in May, IMO really only BigInt is very valuable on its own.
Of the features shipped in September, nontrapping float-to-int and bulk memory will save some code size (and maybe performance, in the case of bulk memory).

How long do we want to wait before enabling these by default (and/or how many users do we want to catch?)

According to my best guess from here desktop Safari 14.1 is currently about 72% of MacOS users, and my best guess from here is that iOS 14.5+ is currently about 67% of iOS users.

@kripken
Copy link
Member

kripken commented Oct 28, 2021

In general we do this through the MIN_FIREFOX_VERSION, MIN_CHROME_VERSION etc. settings' default values. If a feature works in all relevant versions of all major browsers, we can enable it by default.

We don't have a precise plan for when to bump those versions, and it looks from the history like we never have since they were added. At that time @juj set the versions to something like a year or so earlier, and it uses an extended support release for Firefox (is that the only one with extended support?). Those seem like reasonable guidelines to me - the only one I'm not sure of is Safari (how often is that released?).

Those initial numbers were set in late 2019 (#9937), so we could update all browsers almost 2 years forward.

@dschuff
Copy link
Member Author

dschuff commented Oct 28, 2021

I think Safari is released twice a year, once with the major OS updates in the fall, and once in the spring.

@dschuff
Copy link
Member Author

dschuff commented Oct 28, 2021

Also, it looks like we currently use MIN_{browser}_VERSION to gate use of JS APIs, but not to set wasm features at all right now. so we'd want to add that.

@dschuff
Copy link
Member Author

dschuff commented Oct 28, 2021

Oh, and while we're at it, we should maybe also drop default support for EdgeHTML (i.e. old edge), and let MIN_CHROME_VERSION gate Chromium-edge versions.

@juj
Copy link
Collaborator

juj commented Oct 28, 2021

Do we have a policy on when we update the default set of wasm features enabled?

How long do we want to wait before enabling these by default (and/or how many users do we want to catch?)

Before we make any decisions to update min wasm feature set reqs by default, or make a judgement call on how many % of users would need to have a feature, it would be good to first empower Emscripten users to be able to do so. Currently there is no good source of information for that:

I'd bet most of our users are not comfortable with adopting newer minimum feature levels, because the data to help them understand what a bump in minimum feature level will mean for them is not readily available. If the data is there that they can quickly caniuse.com verify the support levels, they'll be easier to give no objections to bumping the minimum levels.

Though before we choose to bump these min versions, we can certainly already now start adding checks that test if all the minimum environment features are such that e.g. non-trapping fptoint will be supported. E.g. if MIN_XXX_VERSION all passed on the cmdline are suitably new, then we can automatically internally enable -mnontrapping-fptoint, unless user explicitly passed -mno-nontrapping-fptoint on the cmdline. That sounds like good to have, though note that we don't currently have a MIN_NODE_VERSION option.. that might be good to add for this, to not disturb existing node.js users when the min levels are eventually bumped?

@dschuff
Copy link
Member Author

dschuff commented Oct 28, 2021

Though before we choose to bump these min versions, we can certainly already now start adding checks that test if all the minimum environment features are such that e.g. non-trapping fptoint will be supported. E.g. if MIN_XXX_VERSION all passed on the cmdline are suitably new, then we can automatically internally enable -mnontrapping-fptoint, unless user explicitly passed -mno-nontrapping-fptoint on the cmdline.

Yes, this is what I meant by my comment above. And to your first point, I agree that we should publicize more fine-grained implementation history. We'll need to collect that information anyway in order to encode it into emcc's behavior, so we might as well make it accessible.

I went ahead and created a spreadsheet to just start collecting the information for now, and we can figure out how best to publish it separately.

@dschuff
Copy link
Member Author

dschuff commented Oct 28, 2021

/cc @RReverser @tlively (based on activity from the linked issues above)

@juj
Copy link
Collaborator

juj commented Oct 28, 2021

That looks great - yeah, with that kind of info accumulated, users will be able to make a decision on which features they'll care about.

@sbc100
Copy link
Collaborator

sbc100 commented Oct 28, 2021

Do we want to keep emscripten-defaults in sync with llvm-defaults? IIRC that answer was yes last time we discussed this?

Does that mean that if the policy we decide on (what ever the specifics are) mandates that, for example, "mutable-globals" is enabled by default in llvm then we may need to increase our MIN_XXX_VERSION settings such that "mutable-globals" would also be enabled for emscripten?

@juj
Copy link
Collaborator

juj commented Oct 29, 2021

Do we want to keep emscripten-defaults in sync with llvm-defaults?

My initial gut feeling is no - I expect they would be doing the decision based on even less analysis to what developers are using in the wild, compared to what we'd at Emscripten side are doing. Though I don't know what kind of discussions take place there.

"mutable-globals" is enabled by default in llvm then we may need to increase our MIN_XXX_VERSION settings such that "mutable-globals" would also be enabled for emscripten?

I hope this would not be the case, but that we'd make a decision based on our own analysis on which versions have become obsolete. That might mean reverting LLVM's default decisions.

In the past, Emscripten has defaulted to the "very compatible" end of the spectrum, e.g. supporting IE11 for quite a long while before eventually dropping it. I hope that general "very compatible" strategy will still be kept, because I think it is a better default for developers who don't want to invest time/don't have enough knowledge to figuring out the compatibility landscape.

@tlively
Copy link
Member

tlively commented Oct 29, 2021

To be clear, we have complete control over both the LLVM and Emscripten defaults, and they can be entirely independent.

We previously decided to be somewhat aggressive about updating LLVM defaults (default after phase 4 + 6 months, IIRC) but actually executing on that decision fell through the cracks. Being more conservative in Emscripten makes sense to me, but we do want to have some concrete updating policy so that users do eventually get the benefits of new features.

@sbc100
Copy link
Collaborator

sbc100 commented Oct 29, 2021

I think the in past @kripken has argued that we should keep llvm and emscripten in sync with defaults.

@dschuff
Copy link
Member Author

dschuff commented Jan 26, 2024

I looked at the data from https://gs.statcounter.com/ios-version-market-share/mobile-tablet/worldwide/#monthly-202212-202312 (the graph isn't very useful because it doesn't aggregate by major version, but I downloaded the CSV) and found that the current market share for iOS v17 at 54.2%, v16 at 30.6%, v15 at 9.2, and everything lower at 6.0%.
However the data from caniuse.com (see e.g. https://caniuse.com/mdn-javascript_operators_await_top_level for another feature which shipped in Safari 15) seems to suggest that iOS Safari versions 14.8 or less have only 0.85% share; that's a pretty big discrepancy, not sure what to make of that.

edit: Actually I think the caniuse data is a percentage of all browsers, not just iOS browsers. That probably accounts for the difference.

But either way it's maybe getting close to the point where we can enable bulk memory, nontrapping-fptoint, and BigInt by default.

@sbc100
Copy link
Collaborator

sbc100 commented Jan 26, 2024

Lets do it!

@dschuff
Copy link
Member Author

dschuff commented Jan 31, 2024

I think in order to flip the switch on this the way we did with signext, we'd want Binaryen bulk-memory and nontrapping-fptoint-lowering passes, right?

@sbc100
Copy link
Collaborator

sbc100 commented Jan 31, 2024

Ah yes, that is correct. Although I think only the bulkmemory ops that llvm itself generates would need to be in the lowering pass (i.e probably just memory.copy)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants