Bug 55204 – malloc in removeChildren shows up on profile of peacekeeper domDynamicCreationCreateElement

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

[Eric Seidel (no email)]

malloc in removeChildren shows up on profile of peacekeeper domDynamicCreationCreateElement

[Eric Seidel (no email)]

Created attachment 83771 [details]
Patch

[Eric Seidel (no email)]

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)]

Created attachment 83774 [details]
Patch

[Maciej Stachowiak]

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 on attachment 83774 [details]
Patch

Will fix.

[Eric Seidel (no email)]

Created attachment 83777 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 83777 [details]
Patch for landing

Clearing flags on attachment: 83777

Committed r79781: <http://trac.webkit.org/changeset/79781>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/79781 might have broken GTK Linux 64-bit Debug