Bug 55204

Summary: malloc in removeChildren shows up on profile of peacekeeper domDynamicCreationCreateElement
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aestes, commit-queue, eric, mjs, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 48719    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Eric Seidel (no email)
Reported 2011-02-24 22:04:01 PST
malloc in removeChildren shows up on profile of peacekeeper domDynamicCreationCreateElement
Attachments
Patch (2.20 KB, patch)
2011-02-24 22:06 PST, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Patch (3.64 KB, patch)
2011-02-24 22:59 PST, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Patch for landing (2.91 KB, patch)
2011-02-25 00:10 PST, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2011-02-24 22:06:27 PST
Created attachment 83771 [details] Patch
Eric Seidel (no email)
Comment 2 2011-02-24 22:07:38 PST
Here are my benchmark numbers: Before: avg 396.96666666666664 median 399 stdev 6.554811633872905 min 375 max 402 After: avg 386.26666666666665 median 388 stdev 6.303085664092541 min 368 max 395 I'm not sure that I trust them though. They're quite noisy. That's 30 runs of (locally modified) peacekeeper domDynamicCreationCreateElement.
Eric Seidel (no email)
Comment 3 2011-02-24 22:59:10 PST
Created attachment 83774 [details] Patch
Maciej Stachowiak
Comment 4 2011-02-24 23:34:14 PST
Comment on attachment 83774 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=83774&action=review r=me > Source/WebCore/dom/ContainerNode.cpp:359 > + // FIXME: We should tune the inlineCapacity. Having this function call malloc > + // as a result of needing to expand capacity is expensive an shows up on benchmarks. This FIXME is probably not super helpful. Just do your best to tune it now. Someone else can always tune more later. > Source/WebCore/dom/ContainerNode.cpp:521 > + // FIXME: We should tune the inlineCapacity. Having this function call malloc > + // as a result of needing to expand capacity is expensive an shows up on benchmarks. Ditto. > Source/WebCore/dom/ContainerNode.cpp:524 > + // childNodeCount() results in a full walk of the nodes, but should be faster > + // than having to malloc twice if we have lots of kids. Dup of a previous comment - maybe this version can be shorter?
Eric Seidel (no email)
Comment 5 2011-02-24 23:35:09 PST
Comment on attachment 83774 [details] Patch Will fix.
Eric Seidel (no email)
Comment 6 2011-02-25 00:10:27 PST
Created attachment 83777 [details] Patch for landing
WebKit Commit Bot
Comment 7 2011-02-26 05:59:52 PST
Comment on attachment 83777 [details] Patch for landing Clearing flags on attachment: 83777 Committed r79781: <http://trac.webkit.org/changeset/79781>
WebKit Commit Bot
Comment 8 2011-02-26 05:59:57 PST
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 9 2011-02-27 09:01:27 PST
http://trac.webkit.org/changeset/79781 might have broken GTK Linux 64-bit Debug
Note You need to log in before you can comment on or make changes to this bug.