[webkit-reviews] review granted: [Bug 132137] Make collectOnEveryAllocation a runtime option : [Attachment 230097] the patch

[bugzilla-daemon at webkit.org]

Mark Hahnenberg <mhahnenberg at apple.com> has granted Mark Lam
<mark.lam at apple.com>'s request for review:
Bug 132137: Make collectOnEveryAllocation a runtime option
https://bugs.webkit.org/show_bug.cgi?id=132137

Attachment 230097: the patch
https://bugs.webkit.org/attachment.cgi?id=230097&action=review

------- Additional Comments from Mark Hahnenberg <mhahnenberg at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=230097&action=review


r=me with good perf numbers

>>> Source/JavaScriptCore/heap/MarkedAllocator.cpp:159
>>> +		 allocationCount = 0;
>> 
>> This is sort of an odd way to write this. Why not trigger a GC when you
exceed the limit rather than when you hit 0?
> 
> Just a heuristic based on my experience of testing with collections on every
100 allocations.  I found that collecting on the first allocation rather than
the last makes it a lot more likely that I’ll see issues (based on our
regression tests as the workload).  For some short running tests, they may not
get to the 100th slow path allocation before the test ends.

Fair enough.

You should factor this out into an ALWAYS_INLINE method so that we keep the
main method relatively clean. I know you're just replacing old crufty code, but
let's improve things while we're here :-)