Bug 142589 – Refactored the JSC::Heap extra cost API for clarity and to make some known bugs more obvious

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Geoffrey Garen]

Refactored the JSC::Heap extra cost API for clarity and to make some known bugs more obvious

[Geoffrey Garen]

Created attachment 248443 [details]
Patch

[Mark Hahnenberg]

Comment on attachment 248443 [details]
Patch

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

Is there a bugzilla bug filed somewhere for fixing all of these broken instances? Might be worth it to paste the link next to the FIXME.

> Source/WebCore/ChangeLog:11
> +        canvas, which is the causde of https://bugs.webkit.org/show_bug.cgi?id=142457.

s/causde/cause

[Mark Hahnenberg]

Comment on attachment 248443 [details]
Patch

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

> Source/JavaScriptCore/heap/SlotVisitorInlines.h:257
> +    heap()->reportExtraMemoryVisited(owner, size);

Not to prematurely optimize, but I wonder if it might make sense to keep private counters in each of the SlotVisitors and then to add them all up at the end of the collection (kind of like we do with other counters)? This could reduce contention during parallel collections that have to report great numbers of DOM elements with extra cost.

[Geoffrey Garen]

Comment on attachment 248443 [details]
Patch

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

>> Source/JavaScriptCore/heap/SlotVisitorInlines.h:257
>> +    heap()->reportExtraMemoryVisited(owner, size);
> 
> Not to prematurely optimize, but I wonder if it might make sense to keep private counters in each of the SlotVisitors and then to add them all up at the end of the collection (kind of like we do with other counters)? This could reduce contention during parallel collections that have to report great numbers of DOM elements with extra cost.

Seems like a good idea. Probably makes the code a little simpler, too. Probably best as a separate patch, since this patch hopes to only rename and document things.

>> Source/WebCore/ChangeLog:11
>> +        canvas, which is the causde of https://bugs.webkit.org/show_bug.cgi?id=142457.
> 
> s/causde/cause

Fixed.

[Geoffrey Garen]

> Is there a bugzilla bug filed somewhere for fixing all of these broken
> instances? Might be worth it to paste the link next to the FIXME.

Bokay.

[Geoffrey Garen]

Created attachment 248446 [details]
Patch

[Andreas Kling]

Comment on attachment 248446 [details]
Patch

r=me

[Geoffrey Garen]

Committed r181407: <http://trac.webkit.org/changeset/181407>