Bug 212105

Summary: [Wasm] Limit the size of Wasm function we optimize in OMG mode
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, fpizlo, keith_miller, mark.lam, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch fpizlo: review+

Description Michael Saboff 2020-05-19 13:48:05 PDT
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.
Comment 1 Saam Barati 2020-05-19 15:37:36 PDT
(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?
Comment 2 Michael Saboff 2020-05-19 18:31:11 PDT
Created attachment 399798 [details]
Patch
Comment 3 Michael Saboff 2020-05-19 18:32:09 PDT
<rdar://problem/63117419>
Comment 4 Michael Saboff 2020-05-19 18:43:18 PDT
(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.
Comment 5 Filip Pizlo 2020-05-19 19:07:04 PDT
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?
Comment 6 Michael Saboff 2020-05-19 19:08:12 PDT
(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.
Comment 7 Michael Saboff 2020-05-20 10:48:24 PDT
Committed r261930: <https://trac.webkit.org/changeset/261930>
Comment 8 Saam Barati 2020-05-20 11:22:31 PDT
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?