Bug 200807 – [JSC] WebAssembly BBQ should switch compile mode for size of modules

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yusuke Suzuki]

On ARM devices, our fixed executable memory size is only 128MB, and it cannot be expanded further since 128MB is the maximum value of relative jump in ARM.
As a result, some of Wasm apps exhaust executable memory. Wasm interpreter should be a long term solution.
As a short term solution, we should have heuristics that switches compiler mode in BBQ: BBQ Air bloats simple code which takes much executable memory. Use BBQ B3 instead for significantly large wasm code on executable-memory-limited environments.

[Yusuke Suzuki]

<rdar://problem/54275409>

[Yusuke Suzuki]

Created attachment 376488 [details]
Patch

[Mark Lam]

Comment on attachment 376488 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=376488&action=review

r=me

> Source/JavaScriptCore/ChangeLog:10
> +        the sacrifice of start-up time, since BBQ Air bloats so lengthy code consuming large amount of executable memory.

I suggest /bloats so lengthy code consuming large/bloats such lengthy code, and thereby consumes a large/.

> Source/JavaScriptCore/runtime/Options.h:495
> +    v(size, webAssemblyBBQAirModeThreshold, isIOS() ? (10 << 20) : 0, Normal, "If 0, we always use BBQ Air. If Wasm module code size hits this threshold, we compile Wasm module with B3 BBQ mode.") \

I suggest using 10 * MB instead.

> Source/JavaScriptCore/wasm/WasmBBQPlan.cpp:144
> +    // FIXME: Some webpages use very large Wasm module, and it exhausts all executable memory in ARM64 devices since the size of executable memory region is 128MB.

/is 128MB/is only limited to 128MB/.

> Source/JavaScriptCore/wasm/WasmBBQPlan.cpp:145
> +    // The long term solution should be introducing Wasm interpreter. But as a short term solution, we introduce heuristics switching back to BBQ B3 at the sacrifice of start-up time,

/should be introducing Wasm interpreter/should be to introduce a Wasm interpreter/.
/heuristics switching back/heuristics to switch back/.

> Source/JavaScriptCore/wasm/WasmBBQPlan.cpp:146
> +    // since BBQ Air bloats so lengthy code consuming large amount of executable memory.

I suggest rephrasing as “as BBQ Air bloats such lengthy Wasm code and will consume a large ...”

[Yusuke Suzuki]

Ah, oops. I need to change the fallback code to module information collecting part.

[Yusuke Suzuki]

Created attachment 376574 [details]
Patch

[Sam Weinig]

Comment on attachment 376574 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=376574&action=review

> Source/JavaScriptCore/ChangeLog:12
> +        Currently, I picked 10MB since the reported website is using 11MB wasm module.

Since the amount of memory on iOS devices is likely to change over time, can this be made in terms of some runtime signal?

[Yusuke Suzuki]

Comment on attachment 376574 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=376574&action=review

>> Source/JavaScriptCore/ChangeLog:12
>> +        Currently, I picked 10MB since the reported website is using 11MB wasm module.
> 
> Since the amount of memory on iOS devices is likely to change over time, can this be made in terms of some runtime signal?

The problem is that we are exhausting executable-memory, not usual memory. In our JIT executable memory allocator, we allocate fixed-sized executable memory, and use it for JIT functions.
The size of this fixed-sized executable-memory region is the same in all the iOS devices, 128MB. And this size is not related to device's memory size. This is related to ARM64 instruction sets.
We cannot extend it further for now, since this 128MB size is derived from the maximum-relative-jump-offset in ARM64 instruction set.
In JSC, right now, we are assuming that any JIT code can be reachable from the other JIT code with relative-offset-jump. Since ARM64's relative-offset-jump is +- 128MB, this 128MB executable memory region is the maximum size in the current design.

[Mark Lam]

Comment on attachment 376574 [details]
Patch

r=me

[Yusuke Suzuki]

(In reply to Mark Lam from comment #8)
> Comment on attachment 376574 [details]
> Patch
> 
> r=me

Thanks, landing.

[Yusuke Suzuki]

Committed r248824: <https://trac.webkit.org/changeset/248824>

[Saam Barati]

Comment on attachment 376574 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=376574&action=review

> Source/JavaScriptCore/wasm/WasmBBQPlan.cpp:273
> +        // FIXME: Some webpages use very large Wasm module, and it exhausts all executable memory in ARM64 devices since the size of executable memory region is only limited to 128MB.

what web pages?

Also, this falls down if you compile many modules, instead of one big module. This doesn't feel like a very robust solution to this problem.

[Yusuke Suzuki]

Comment on attachment 376574 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=376574&action=review

>> Source/JavaScriptCore/wasm/WasmBBQPlan.cpp:273
>> +        // FIXME: Some webpages use very large Wasm module, and it exhausts all executable memory in ARM64 devices since the size of executable memory region is only limited to 128MB.
> 
> what web pages?
> 
> Also, this falls down if you compile many modules, instead of one big module. This doesn't feel like a very robust solution to this problem.

The radar includes the information about this page.
This fix is not aiming at being the robust solution as described in this FIXME. This is short term solution until Wasm interpreter is introduced.
The motivation behind this fix is, it is better than having some pages working rather than completely not-working.

[Alexey Proskuryakov]

*** Bug 200686 has been marked as a duplicate of this bug. ***

[Walter Stumpf]

Hello,

Thanks for resolving this issue so quickly!  I was wondering if you guys knew ballpark when this patch will land in iOS.  I’m assuming it’s too late for 13.0 since the regression still occurs for me in 13.1 beta 4.  I don’t mean that as a complaint!  But I’m trying to get an appreciation for which versions of iOS we should be blocking for end users.  Thanks again for the help!

[Keith Miller]

(In reply to Walter Stumpf from comment #14)
> Hello,
> 
> Thanks for resolving this issue so quickly!  I was wondering if you guys
> knew ballpark when this patch will land in iOS.  I’m assuming it’s too late
> for 13.0 since the regression still occurs for me in 13.1 beta 4.  I don’t
> mean that as a complaint!  But I’m trying to get an appreciation for which
> versions of iOS we should be blocking for end users.  Thanks again for the
> help!

Hi Walter,

First, sorry about the regression. Unfortunately, Apple has a policy of not commenting on future product releases, however, so we can't give a definitive timeline.

[Walter Stumpf]

No problem, I completely understand.  We greatly appreciate the webkit team even working around the problem for us in the first place!  I will continue to cross my fingers when installing new iOS betas in the meantime! :)

[Walter Stumpf]

Hey just wanted to confirm iOS 13.2 beta resolves the issue for us!  Thank you!!!