Bug 212105 - [Wasm] Limit the size of Wasm function we optimize in OMG mode
Summary: [Wasm] Limit the size of Wasm function we optimize in OMG mode
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-19 13:48 PDT by Michael Saboff
Modified: 2020-05-20 11:22 PDT (History)
7 users (show)

See Also:


Attachments
Patch (5.76 KB, patch)
2020-05-19 18:31 PDT, Michael Saboff
fpizlo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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?