Bug 142589 - Refactored the JSC::Heap extra cost API for clarity and to make some known bugs more obvious
Summary: Refactored the JSC::Heap extra cost API for clarity and to make some known bu...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-11 12:45 PDT by Geoffrey Garen
Modified: 2015-03-11 14:30 PDT (History)
3 users (show)

See Also:

Attachments
Patch (33.46 KB, patch)
2015-03-11 13:03 PDT, Geoffrey Garen 		no flags Details | Formatted Diff | Diff
Patch (34.08 KB, patch)
2015-03-11 14:17 PDT, Geoffrey Garen 		kling: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>