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: | P1 | Keywords: | HasReduction, InRadar, Regression | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Mac (Intel) | ||||||||||||
OS: | OS X 10.6 | ||||||||||||
Attachments: |
|
Description
Keishi Hattori
2009-11-07 21:39:20 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 Created attachment 42952 [details]
reduced test case
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%.
Judging from my profile results, http://trac.webkit.org/changeset/47022 looks suspicious... I agree with keishi - shark blames marking (98% in jsnode marking): 1.1% 98.2% WebCore 338 B WebCore::JSNode::markChildren(JSC::MarkStack&) *** Bug 31409 has been marked as a duplicate of this bug. *** We figured out the cause. It's the DOM node mark function. Maciej and Oliver are talking about solutions. Created attachment 43426 [details]
preliminary patch (haven't made sure it compiles and passes tests yet)
Comment on attachment 43426 [details]
preliminary patch (haven't made sure it compiles and passes tests yet)
Looks good.
Created attachment 43474 [details]
Patch
(the new patch actually passes tests; first version broke Acid3) Comment on attachment 43474 [details]
Patch
r=me
Committed r51162: <http://trac.webkit.org/changeset/51162> 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. |