Bug 31237 - REGRESSION (r47022): Performance of DocumentFragment.appendChild is 1000x slower sometimes
Summary: REGRESSION (r47022): Performance of DocumentFragment.appendChild is 1000x slo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Intel OS X 10.6
: P1 Major
Assignee: Maciej Stachowiak
URL:
Keywords: HasReduction, InRadar, Regression
: 31409 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-11-07 21:39 PST by Keishi Hattori
Modified: 2010-04-09 09:42 PDT (History)
11 users (show)

See Also:


Attachments
reduced test case (3.29 KB, text/html)
2009-11-11 04:31 PST, Keishi Hattori
no flags Details
test case results (253.42 KB, image/png)
2009-11-11 04:40 PST, Keishi Hattori
no flags Details
preliminary patch (haven't made sure it compiles and passes tests yet) (4.01 KB, patch)
2009-11-18 04:51 PST, Maciej Stachowiak
no flags Details | Formatted Diff | Diff
Patch (11.98 KB, patch)
2009-11-18 17:33 PST, Maciej Stachowiak
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keishi Hattori 2009-11-07 21:39:20 PST
I noticed the Web Inspector syntax highlighter was way too slow. It seems that DocumentFragment.appendChild is very very slow just in the latest WebKit nightly.

Test: Open http://keishi.net/SourceSyntaxHighlighter3/ and look at console

WebKit nightly(r50616) - 4241ms
Chromium nightly(4.0.240.0 (31395)) - 1494ms
Safari 4(4.0.3 (6531.9)) - 1487ms
Comment 1 Keishi Hattori 2009-11-07 21:51:26 PST
Also Node.appendChild(DocumentFragment) much slower in Safari than Chromium.

http://keishi.net/SourceSyntaxHighlighter4/

WebKit nightly(r50616) - 4478ms
Chromium nightly(4.0.240.0 (31395)) - 2581ms
Safari 4(4.0.3 (6531.9)) - 4354ms
Comment 2 Mark Rowe (bdash) 2009-11-08 00:26:26 PST
<rdar://problem/7375325>
Comment 3 Keishi Hattori 2009-11-11 04:31:12 PST
Created attachment 42952 [details]
reduced test case
Comment 4 Keishi Hattori 2009-11-11 04:40:36 PST
Created attachment 42953 [details]
test case results

I pressed each button once and then the last one one more time.

WebKit nightly is always extremely slow for "appendChild( DocumentFragment with 10000 childNodes )" test.  "DocumentFragment.appendChild(span) x10000" and "div.appendChild(span) x10000" is OK 50% of the time but 1000x slower for the other 50%.
Comment 5 Keishi Hattori 2009-11-11 08:40:47 PST
I've tracked the regression somewhere between r47011 and r47060.
Comment 6 Keishi Hattori 2009-11-11 09:06:02 PST
Judging from my profile results, http://trac.webkit.org/changeset/47022 looks suspicious...
Comment 7 Oliver Hunt 2009-11-11 12:20:47 PST
I agree with keishi - shark blames marking (98% in jsnode marking):
	1.1%	98.2%	WebCore	338 B	WebCore::JSNode::markChildren(JSC::MarkStack&)
Comment 8 Timothy Hatcher 2009-11-14 15:34:43 PST
*** Bug 31409 has been marked as a duplicate of this bug. ***
Comment 9 Darin Adler 2009-11-14 15:55:26 PST
We figured out the cause. It's the DOM node mark function. Maciej and Oliver are talking about solutions.
Comment 10 Maciej Stachowiak 2009-11-18 04:51:18 PST
Created attachment 43426 [details]
preliminary patch (haven't made sure it compiles and passes tests yet)
Comment 11 Darin Adler 2009-11-18 08:26:20 PST
Comment on attachment 43426 [details]
preliminary patch (haven't made sure it compiles and passes tests yet)

Looks good.
Comment 12 Maciej Stachowiak 2009-11-18 17:33:53 PST
Created attachment 43474 [details]
Patch
Comment 13 Maciej Stachowiak 2009-11-18 17:35:02 PST
(the new patch actually passes tests; first version broke Acid3)
Comment 14 Oliver Hunt 2009-11-18 17:38:27 PST
Comment on attachment 43474 [details]
Patch

r=me
Comment 15 Maciej Stachowiak 2009-11-18 18:14:05 PST
Committed r51162: <http://trac.webkit.org/changeset/51162>
Comment 16 Alexey Proskuryakov 2010-04-09 09:42:05 PDT
The assumption in this patch was not quite correct, fragment roots don't always have wrappers. Fixed one hang resulting from that in bug 37181, but there might be more.