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+

Geoffrey Garen
Reported 2015-03-11 12:45:07 PDT
Refactored the JSC::Heap extra cost API for clarity and to make some known bugs more obvious
Attachments
Patch (33.46 KB, patch)
2015-03-11 13:03 PDT, Geoffrey Garen 		no flags
DetailsFormatted DiffDiff
Patch (34.08 KB, patch)
2015-03-11 14:17 PDT, Geoffrey Garen 		kling: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Geoffrey Garen
Comment 1 2015-03-11 13:03:07 PDT
Created attachment 248443 [details] Patch
Mark Hahnenberg
Comment 2 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
Mark Hahnenberg
Comment 3 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.
Geoffrey Garen
Comment 4 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.
Geoffrey Garen
Comment 5 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.
Geoffrey Garen
Comment 6 2015-03-11 14:17:21 PDT
Created attachment 248446 [details] Patch
Andreas Kling
Comment 7 2015-03-11 14:28:35 PDT
Comment on attachment 248446 [details] Patch r=me
Geoffrey Garen
Comment 8 2015-03-11 14:30:24 PDT
Committed r181407: <http://trac.webkit.org/changeset/181407>
Note You need to log in before you can comment on or make changes to this bug.