[webkit-reviews] review granted: [Bug 238777] [JSC] Reduce sizeof(BaselineCallLinkInfo) to make bug 238535 good : [Attachment 456648] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 238777: [JSC] Reduce sizeof(BaselineCallLinkInfo) to make bug 238535 good
https://bugs.webkit.org/show_bug.cgi?id=238777

Attachment 456648: Patch

https://bugs.webkit.org/attachment.cgi?id=456648&action=review




--- Comment #5 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 456648
  --> https://bugs.webkit.org/attachment.cgi?id=456648
Patch

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

r=me

>>> Source/JavaScriptCore/bytecode/Repatch.cpp:1744
>>> +		 frameShuffler.emplace(stubJit,
*optimizingCallLinkInfo.frameShuffleData());
>> 
>> Here, we're copying the CallFrameShuffleData, right?  Can we change the
idiom such that we use:
>> 
>>     CallFrameShuffler localFrameShuffler;
>>     std::optional<CallFrameShuffler*> frameShuffler;
>> 
>> ... and in the baseline case, pass the &localFrameShuffler to an init
function (factored out of createForBaselineOrLLIntTailCall()) instead of
calling createForBaselineOrLLIntTailCall() directly?
> 
> Ah, no. CallFrameShuffler just constructs itself via `const
CallFrameShuffleData&`, and does not copy it. So it is just referencing the
value.

Sorry.	I conflated CallFrameShuffleData with CallFrameShuffler.  You are
correct that there's no unnecessary copying here.