561717 - Much slower JavaScript on gecko compared with webkit when appending to DOMElement.innerHTML

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[gyll]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9) Gecko/20100315 Firefox/3.5.9
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9) Gecko/20100315 Firefox/3.5.9

to observe the problem reported in the title, please unzip the attachment, open the 'WSX\index.html' file, and then toggle the value of the 'innerHTMLSwitch' variable in the 'browser.htm' file (this variable simply toggles between two implementations of a javascript line of code inside the 'browser.htm' file)

Reproducible: Always

Comment 1

[gyll]

Please use the file attached in https://bugzilla.mozilla.org/show_bug.cgi?id=561715

Comment:
1) unzip the archive and open 'WSX\index.html' file; the "home page" of a website will be displayed
2) now click on the second link in the page menu on the left, and measure how long it takes for the page to be displayed (you can repeat this measurement by clicking the home page again and then the second link again, and observe the display delay)
3) in the 'WSX\main.htm' file there is a variable 'innerHTMLSwitch'; toggle the value of this variable between 0 and 1 and then reload the website; then repeat the measurement of how long it takes to change the displayed page when clicking between e.g. the first two links in the page menu on the left, and you'll notice a very significant difference between innerHTMLSwitch=1 and innerHTMLSwitch=0

Comment 2

[Boris Zbarsky [:bzbarsky]]

Doesn't look quite like the bug 404385 issue, since the string is not quite plaintext...  Smaug, want to take a look?

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I hear that with the HTML5 parser we are lickity-split on these sorts of things.  Unless I'm misremembering jst's information, hmm.

Comment 4

[Boris Zbarsky [:bzbarsky]]

What I see on m-c from a few days back measuring just that innerHTML loop highlighted in the code when innerHTMLSwitch is true:

old parser: 288ms
HTML5 parser: 71ms

For comparison:

Safari 4: 50ms
Firefox 3.5 (which the bug is reported against): 351ms

So yeah, with the HTML5 parser things aren't so bad here.  Still somewhat slower than Safari.  I tried testing Chrome 5 as well, but it just totally fails to load the page.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Would be great to get a *minimal* testcase attached using 
"Add an attachment".

There is a patch to cut ~8% (in a simple plain text testcase) of innerHTML+HTML5 time. And there are still other things to optimize. Some bugs are already filed.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Seems like the example page uses += in some cases, and that depends also
on serialization, which will be improved in Bug 561221.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Minimal-ish testcase of the code I measured in comment 4

The image slows both us and Safari down by 20-25ms or so; without it we're at 50 an they're at 25.

Comment 8

[Boris Zbarsky [:bzbarsky]]

General time breakdown for that testcase:

31% parsing
11% MaybeLoadImage converting the string to a URI and kicking off the image load
 3% setting the textContent of a node.
 3% inserting nodes
35% serialization (about 2/3 under 
    nsHTMLContentSerializer::AppendAndTranslateEntities)

Most of the rest of the time is in vm faults....

Comment 9

[gyll]

One more thing i just tried in my firefox (3.5): it seems the difference between firefox and safari 4 gets somewhere near 10:1 if instead of the string "&nbsp &nbsp &nbsp &nbsp " you put "<b>&nbsp &nbsp &nbsp &nbsp </b>" (i.e. if you add some more HTML inside the string, apart from the simple '&nbsp'). Dunno how severe this difference is in your HTML5 parser.

Comment 10

[Boris Zbarsky [:bzbarsky]]

With the html5 parser, the <b> version takes 280ms for me.  Safari takes 170ms.

Comment 11

[Boris Zbarsky [:bzbarsky]]

And the "setting textContent" thing then takes 23% of the time, not 3%.

That looks like the removal of the old nodes, fwiw.  And about half of it seems to be the nsCOMPtr destructor which actualy destroys the old nodes...  nsNodeUtils::LastRelease, nsGenericElement destructor, child array dtor, etc.

Comment 12

[Boris Zbarsky [:bzbarsky]]

We're easily 2x faster than WebKit on the attached testcase now.