Bug 142589

Summary: Refactored the JSC::Heap extra cost API for clarity and to make some known bugs more obvious
Product: WebKit Reporter: Geoffrey Garen <ggaren>
Component: New BugsAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, kling, mhahnenb
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch kling: review+

Description Geoffrey Garen 2015-03-11 12:45:07 PDT
Refactored the JSC::Heap extra cost API for clarity and to make some known bugs more obvious
Comment 1 Geoffrey Garen 2015-03-11 13:03:07 PDT
Created attachment 248443 [details]
Patch
Comment 2 Mark Hahnenberg 2015-03-11 13:22:55 PDT
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
Comment 3 Mark Hahnenberg 2015-03-11 13:27:39 PDT
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.
Comment 4 Geoffrey Garen 2015-03-11 14:16:30 PDT
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.
Comment 5 Geoffrey Garen 2015-03-11 14:16:47 PDT
> 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.
Comment 6 Geoffrey Garen 2015-03-11 14:17:21 PDT
Created attachment 248446 [details]
Patch
Comment 7 Andreas Kling 2015-03-11 14:28:35 PDT
Comment on attachment 248446 [details]
Patch

r=me
Comment 8 Geoffrey Garen 2015-03-11 14:30:24 PDT
Committed r181407: <http://trac.webkit.org/changeset/181407>