Bug 212105 – [Wasm] Limit the size of Wasm function we optimize in OMG mode

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

[Michael Saboff]

The OMG compilation path for Wasm functions has phases where memory can grow O(N^2) based on the function size.  Long term we should reduce that memory usage.  In the near term we should fallback to the BBQ compilation settings for such large functions.

[Saam Barati]

(In reply to Michael Saboff from comment #0)
> The OMG compilation path for Wasm functions has phases where memory can grow
> O(N^2) based on the function size.  Long term we should reduce that memory
> usage.  In the near term we should fallback to the BBQ compilation settings
> for such large functions.

We already compiled code with BBQ if we are trying to OMG compile. So are we just going to skip the OMG compile? Or should we compile with B3-O1?

[Michael Saboff]

Created attachment 399798 [details]
Patch

[Michael Saboff]

<rdar://problem/63117419>

[Michael Saboff]

(In reply to Saam Barati from comment #1)
> (In reply to Michael Saboff from comment #0)
> > The OMG compilation path for Wasm functions has phases where memory can grow
> > O(N^2) based on the function size.  Long term we should reduce that memory
> > usage.  In the near term we should fallback to the BBQ compilation settings
> > for such large functions.
> 
> We already compiled code with BBQ if we are trying to OMG compile. So are we
> just going to skip the OMG compile? Or should we compile with B3-O1?

We skip the OMG compile.

[Filip Pizlo]

Comment on attachment 399798 [details]
Patch

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

> Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:540
> +    if (m_info.functions[m_functionIndex].data.size() < Options::webAssemblyBBQFallbackSize())

Can we abstract this inequality, so it’s if (functionCall) here and the other places you do this less than check?

[Michael Saboff]

(In reply to Filip Pizlo from comment #5)
> Comment on attachment 399798 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=399798&action=review
> 
> > Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:540
> > +    if (m_info.functions[m_functionIndex].data.size() < Options::webAssemblyBBQFallbackSize())
> 
> Can we abstract this inequality, so it’s if (functionCall) here and the
> other places you do this less than check?

Sure.

[Michael Saboff]

Committed r261930: <https://trac.webkit.org/changeset/261930>

[Saam Barati]

Comment on attachment 399798 [details]
Patch

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

LGTM too.

> Source/JavaScriptCore/ChangeLog:17
> +        Also for Wasm functions at or above  the threashold, we don't tier up to an

threashold => threshold

> Source/JavaScriptCore/runtime/OptionsList.h:456
> +    v(Unsigned, webAssemblyBBQFallbackSize, 50000, Normal, "Limit of Wasm function size above which we fallback to BBQ compilation mode.") \

This feels a bit small? What was the size of the function in your test page?

> Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:1977
> +    procedure.setOptLevel(compilationMode == CompilationMode::BBQMode || function.data.size() >= Options::webAssemblyBBQFallbackSize()

this should only happen for OSR entry, right?