design
design copied to clipboard
WebAssembly
Function body maximum size
Among the browser vendors we agreed upon a limit of function body size to allow in webassembly module of 128KB.
However, the benchmark Godot has a function of 151975Bytes or 148KB (function 2187).
For some reason Chrome and Firefox allow it. Now we end up in Edge where we invalidate this module.
It makes me think that we didn't understand the limit correctly. According to v8/module-decoder.cc, it looks like it is checking the size in bytes.
Can someone from the other browser vendors help me understand this ?
D'oh, this particular limit check wasn't factored into the shared validation code and thus was only being checked by WebAssembly.validate in FF. That's an awfully big function; I'll see if perhaps Godot isn't enabling optimizations in their build. But if it is just some giant interpreter loop or something, perhaps this is evidence we've set the limit too low.
Also it appears that function index 21555 is even bigger, with 219504 bytes.
It makes me think that we didn't understand the limit correctly. According to v8/module-decoder.cc, it looks like it is checking the size in bytes.
Unfortunately, it looks like DecodeWasmFunction is only used by SyncDecodeWasmFunction which is only used by a unittest. The limit is also used in the streaming decoder, but it's not clear why it's not erroring out there (likely because the streaming decoder is not used here). I filed a v8 bug: https://crbug.com/v8/6959.
FWIW we don't apply all the limits because some didn't make sense to our implementation. The ones we apply are here: https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/wasm/WasmLimits.h
In particular, I dropped function size because it's a ridiculously low limit which basic wasm programs exceed (e.g. the benchmarks from the PLDI paper trips it).
Do we want to revisit the limits that we use and make them part of the spec (either js or web spec)? I agree that 128kb is too low.
In general, if Wasm programs should work consistently across browsers, having any sort of restrictions be written in the specification and with conformance tests seems like a good idea to me. Otherwise, there's a natural "race to the bottom" (or top, here?) where browsers try to be compatible with each other as much as possible and permit what all the other ones permit. This sort of dynamic is responsible for a lot of the mess that we have in JS and the rest of the web platform.
I don't understand the details of this particular limit; is there any past discussion that was written down anywhere about it?
@littledan, a race to the top is usually a good thing when it comes to arbitrary limits. Every language implementation has tons of implicit limitations, almost no language even documents their possibility (C is the only exception that comes to mind, and only for some hand-selected cases). Picking concrete numbers on the spec level makes no sense in most cases, because the limits are not static but depend on many complex platform-dependent and implementation-specific variables.
If it doesn't make sense in the spec, documenting the limits somewhere for toolchains to refer to makes sense. Maybe a new .md in webassembly/tool-conventions?
Anyhow yeah, sounds like 128kb is way too low. I think having a chosen limit is better than each engine having its own implicit limit unless we think all engines will be able to handle functions up to the maxModuleSize (1gb). I doubt that, especially given the subtle limits that show up like the 32mb local jump range on ARM. How about 1mb?
It might be nice to have some more information about the largest functions we've seen before making this choice.
@kripken What are the biggest you've ever seen? Alon was telling me he was thinking of bringing the "outlining" feature (which splits up crazy-big functions) to wasm. With this, we wouldn't necessarily need to accommodate the biggest possible functions.
I've been thinking about this, but I don't yet have a good idea for something like outlining in wasm (the asm.js approach was quite hackish and we had a bunch of bugs there). I may end up just emitting a warning in the meantime.
Size-wise, I don't have good data on this, none of the biggest ones were recent, and I measured them as lines in the text format anyhow.
Could we do 64MB? I've never seen a wasm module that big, let alone a single function, so it seems like a safe bound. Or is there a benefit to a lower value? (I'm not sure why we need a bound at all so I'm missing some background I guess.)
Assuming there is a small factor expansion between wasm bytecode and machine code (I've seen 2-4x), and given ARM(32,64)'s 32mb local-jump limit, I'd be surprised if engines didn't fail in practice well before 32mb. It's possible of course to handle >32mb functions if you design for it, but I expect not all engines do. Happy to hear from other engines though.
One engine's data point: my .NET-based implementation inherits all of its limitations from the .NET framework itself, which doesn't seem to have any hard limit on function size (at least none that I've heard of or have been able to find after some quick searches).
I still think it is a good idea that engines have a "gentleman's agreement" to enforce a static size limit for functions. I'm OK bumping up the 128k limit, but I think we should probably pick a reasonable one before the race to the top picks up :)
@titzer Perhaps it's best not to apply the 128K limit right now? It will break Godot and likely other things. This will cause confusion and frustration for users.
We've seen as high as 1.5MB, for instance this: https://bugs.chromium.org/p/chromium/issues/detail?id=769607
If we're already seeing pathological cases of 1.5mb, then maybe we should bump all the way up to 10mb?
@kripken For now though, do you know how Godot is being built/is there any way to compile it smaller? Edge just shipped, and this isn't a reliability issue for existing production websites so we're going to be stuck with 128kb limit until next Windows release.
@MikeHolman: When is the next Windows release?
We can try to figure out a solution for Godot, but we may need to do something more radical if we can't expect a browser update soon. Because we've already seen other examples from other codebases like the one @flagxor mentioned, and also there may not be a practical solution in some cases, if the code is from a code generator, or something else that can't be worked around by preventing inlining.
In asm.js we experimented with outlining/function splitting. Overall it was messy and buggy, so I'd rather not repeat that, but maybe we have no choice.
I expect it will be in about 6 months, so I think a short term targeted workaround will be sufficient.
@flagxor Oh hah, from your comments, looks like that 1.5mb was a data section expressed as one big function.
@MikeHolman I've emailed the Godot author (https://github.com/eska014/godot/tree/wasm-benchmark) asking if all optimizations are enabled and, if so, if the function can be artificially split somehow.
@MikeHolman That being said, I wouldn't be surprised if this broke for various uses in the wild over the next 6 months so, since it's such a trivial fix, it may be worth pushing to uplift the fix.
With a worst-case 4x bytecode-to-machine-code expansion, a 10mb function could hit ARM 32mb limits; so to be a bit conservative, could we do something smaller than 10mb?
Windows is conservative about servicing. Even though it is a small fix, it needs to represent a significant reliability issue or a security issue. I'm not going to be able to convince them that this super new feature has enough web presence and hits the pathological case often enough to meet that bar.
I'm hesitant to go very much lower than 10mb, since we're still in early days and already have an example less than an order of magnitude away. Is 7mb a reasonable compromise?
If there's a new limit, should it be included in some specification and shared tests?
@MikeHolman 6 months sounds like a long time, so I guess we should look into function splitting on the toolchain side even if godot has a manual workaround - thoughts?
Regardless it's interesting to know if they have a workaround. @lukewagner btw, fwiw that wasm looks optimized to me.
@littledan I don't know about whether it belongs in the specifications... maybe so as an appendix. Above I suggested the tool-conventions repo.
@lukewagner Appendix seems fine to me. The important thing would be that implementations actually converge on something shared (if we want to have a shared limit like this; maybe @rossberg or @jfbastien disagree). This could even be something as rough as shared tests, where a comment in the tests points to this bug thread.
It just seems unfortunate that we're in this position of having had a miscommunication that might lead to incompatibility between browsers; I'm wondering if we can prevent more of these in the future.