Bug 228053

Summary: DFGByteCodeParser.cpp should avoid resizing the Operands<> of every BasicBlock on every inlining
Product: WebKit Reporter: Robin Morisset <rmorisset>
Component: JavaScriptCoreAssignee: Robin Morisset <rmorisset>
Status: REOPENED ---    
Severity: Normal CC: commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 233387    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
saam: review+, ews-feeder: commit-queue-
Patch none

Description Robin Morisset 2021-07-17 20:01:48 PDT
The dfg bytecode parser only makes use of block->variablesAtTail. But currently it updates the size of variablesAtHead, valuesAtHead, valuesAtTail and intersectionOfPastValuesAtHead every single time it changes the number of Tmps and/or Locals. This happens notably whenever it inlines a function.
It is not nearly as cheap as it looks, as each resizing may reallocate a Vector, requires filling the new slots with zeros, and requires moving the existing values (which are all 0) to the new Vector. This was obvious when looking at profiling of JS2: bzero + memmove are the two hottest C++ functions, and the manipulation of Operands is partly responsible.
It can be easily improved: only resize block->variablesAtTail on inlining, and initialize all of the other operands at the very end of the DFGByteCodeParser.
Comment 1 Robin Morisset 2021-07-17 20:06:08 PDT
Created attachment 433740 [details]
Patch
Comment 2 Robin Morisset 2021-07-17 20:09:43 PDT
Created attachment 433742 [details]
Patch

updated Changelog with perf numbers.
Comment 3 Robin Morisset 2021-07-17 20:11:37 PDT
Created attachment 433743 [details]
Patch

Fix style nitpick.
Comment 4 Radar WebKit Bug Importer 2021-07-24 20:02:16 PDT
<rdar://problem/81064797>
Comment 5 Saam Barati 2021-11-17 15:57:14 PST
Comment on attachment 433743 [details]
Patch

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

r=me

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:8971
> +        block->variablesAtHead = Operands<Node*>(m_numArguments, m_numLocals, m_numTmps);
> +        block->valuesAtHead = Operands<AbstractValue>(m_numArguments, m_numLocals, m_numTmps);
> +        block->valuesAtTail = Operands<AbstractValue>(m_numArguments, m_numLocals, m_numTmps);
> +        block->intersectionOfPastValuesAtHead = Operands<AbstractValue>(m_numArguments, m_numLocals, m_numTmps);

nit: Use the OperandsLike constructor so these lines of code don't have to change in the future if we change the Operands constructor you're invoking?
Comment 6 Robin Morisset 2021-11-17 18:20:26 PST
Created attachment 444624 [details]
Patch

Applied Saam's suggestion and rebased
Comment 7 EWS 2021-11-18 14:57:05 PST
Committed r286030 (244418@main): <https://commits.webkit.org/244418@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 444624 [details].
Comment 8 WebKit Commit Bot 2021-11-19 15:17:55 PST
Re-opened since this is blocked by bug 233387