Bug 31237

Summary:

REGRESSION (r47022): Performance of DocumentFragment.appendChild is 1000x slower sometimes

Product:

WebKit

Reporter:

Keishi Hattori <keishi>

Component:

DOM

Assignee:

Maciej Stachowiak <mjs>

Status:

RESOLVED FIXED

Severity:

Major

CC:

ap, barraclough, bweinstein, darin, ggaren, joepeck, oliver, pfeldman, sam, spamfaenger, timothy

Priority:

Keywords:

HasReduction, InRadar, Regression

Version:

528+ (Nightly build)

Hardware:

Mac (Intel)

OS:

OS X 10.6

Attachments:

Description	Flags
reduced test case	none
test case results	none
preliminary patch (haven't made sure it compiles and passes tests yet)	none
Patch	oliver: review+

Keishi Hattori

Reported 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

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Keishi Hattori

Comment 1 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

Mark Rowe (bdash)

Comment 2 2009-11-08 00:26:26 PST

<rdar://problem/7375325>

Keishi Hattori

Comment 3 2009-11-11 04:31:12 PST

Created attachment 42952 [details] reduced test case

Keishi Hattori

Comment 4 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%.

Keishi Hattori

Comment 5 2009-11-11 08:40:47 PST

I've tracked the regression somewhere between r47011 and r47060.

Keishi Hattori

Comment 6 2009-11-11 09:06:02 PST

Judging from my profile results, http://trac.webkit.org/changeset/47022 looks suspicious...

Oliver Hunt

Comment 7 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&)

Timothy Hatcher

Comment 8 2009-11-14 15:34:43 PST

*** Bug 31409 has been marked as a duplicate of this bug. ***

Darin Adler

Comment 9 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.

Maciej Stachowiak

Comment 10 2009-11-18 04:51:18 PST

Created attachment 43426 [details] preliminary patch (haven't made sure it compiles and passes tests yet)

Darin Adler

Comment 11 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.

Maciej Stachowiak

Comment 12 2009-11-18 17:33:53 PST

Created attachment 43474 [details] Patch

Maciej Stachowiak

Comment 13 2009-11-18 17:35:02 PST

(the new patch actually passes tests; first version broke Acid3)

Oliver Hunt

Comment 14 2009-11-18 17:38:27 PST

Comment on attachment 43474 [details] Patch r=me

Maciej Stachowiak

Comment 15 2009-11-18 18:14:05 PST

Committed r51162: <http://trac.webkit.org/changeset/51162>

Alexey Proskuryakov

Comment 16 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.

Note You need to log in before you can comment on or make changes to this bug.