[webkit-reviews] review granted: [Bug 66747] There is no facility for profiling how the write barrier is used : [Attachment 104902] the patch (fix review)

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 66747: There is no facility for profiling how the write barrier is used
https://bugs.webkit.org/show_bug.cgi?id=66747

Attachment 104902: the patch (fix review)
https://bugs.webkit.org/attachment.cgi?id=104902&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=104902&action=review


r=me, but you have a minor bug here in the ENABLE(GGC) case to fix.

> Source/JavaScriptCore/heap/Heap.h:253
>      inline void Heap::writeBarrier(const JSCell* owner, JSValue value)
>      {
> +	   WriteBarrierCounters::countWriteBarrier();
>	   if (!value)
>	       return;
>	   if (!value.isCell())

Since this version of Heap::writeBarrier calls through to the JSCell* version,
you'll double-count cases where you don't early return. You need to call
WriteBarrierCounters::countWriteBarrier() inside each early return, and not in
the body of the function.