Closed Bug 386802 Opened 13 years ago Closed 13 years ago

Mutation events always fired during setting of innerHTML

Categories

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

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: smaug)

References

(Blocks 1 open bug)

Details

(Keywords: perf, regression)

Attachments

(1 file)

The fix for bug 288392 made mutation events always fire if the document is "in the middle of dispatching a mutation event".  The same patch added mozAutoSubtreeModified objects around the code in nsNode3Tearoff::SetTextContent, nsGenericElement::Normalize, and nsGenericHTMLElement::SetInnerHTML.  This means that every single DOM operation performed by those functions fires a mutation event.

So for example, if I set the innerHTML on a <div> to some string a bunch of times in a row (as in bug 386769), we will fire a DOMNodeRemoved when the old textnode is removed and fire a DOMNodeInserted when the new one is inserted.  This slows down SetInnerHTML by 20% or so on that test.

I really don't think we want this perf hit in 1.9.
Flags: blocking1.9?
argh.
Assignee: nobody → Olli.Pettay
Status: NEW → ASSIGNED
Attached patch proposed patchSplinter Review
This is a bit ugly, but should work well enough.
With the patch extra mutation events aren't dispatched just to get
right batching for DOMSubtreeModified. Instead of that, targets for the
possible mutation events are collected.
Speed ups the testcase significantly, though didn't profile.
Attachment #270880 - Flags: superreview?(bzbarsky)
Attachment #270880 - Flags: review?(bzbarsky)
Comment on attachment 270880 [details] [diff] [review]
proposed patch

Makes sense.

We should add a test for this somewhere... perhaps Tdhtml or something?
Attachment #270880 - Flags: superreview?(bzbarsky)
Attachment #270880 - Flags: superreview+
Attachment #270880 - Flags: review?(bzbarsky)
Attachment #270880 - Flags: review+
Flags: in-testsuite?
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: blocking1.9?
Resolution: --- → FIXED
Can confirm a significant improvement in that test (482 down to 251).   Nicely done!
No longer blocks: 386769
Blocks: 442348
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.