[webkit-reviews] review granted: [Bug 195013] [JSC] Revert r226885 to make SlotVisitor creation lazy : [Attachment 362923] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 195013: [JSC] Revert r226885 to make SlotVisitor creation lazy
https://bugs.webkit.org/show_bug.cgi?id=195013

Attachment 362923: Patch

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




--- Comment #6 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 362923
  --> https://bugs.webkit.org/attachment.cgi?id=362923
Patch

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

>>>> Source/JavaScriptCore/ChangeLog:8
>>>> +	      We once changed SlotVisitor creation apriori to drop the lock.
But it turns out that SlotVisitor is memory-consuming.
>>> 
>>> I don't understand what these two sentences have to do with each other.
What does "memory-consuming" here mean?
>> 
>> In previous change, we created # of Heap collectors SlotVisitors apriori.
But each SlotVisitor takes so much memory (each has two MarkedStack, and each
MarkedStack at least has 4KB dirty memory).
>> This patch reverts that apriori creation, and creates SlotVisitors
dynamically.
> 
> And the previous patch removed the lock because we no longer created
SlotVisitors dynamically. But this patch attempts to revert that. Then, the
lock comes back.
> I checked `forEachSlotVisitor()` calls in our JSC code base and the passed
lambda does not cause dead-lock.

"But" => "Also, it turns out..."