564591 - Speed up BindToTree/UnbindFromTree

Status: RESOLVED FIXED

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

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

We're doing too much work in BindToTree/UnbindFromTree. We should optimize it more for performance. I'll start with a couple of simple patches to use nsINode flags to avoid doing work unless necessary.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Speed up accesskey

Avoid searching for accesskey attribute on each bind/unbind unless such an attribute exists. Use a flag to indicate when the attribute is set.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Speed up XBL

Only call into XBL code when NODE_MAY_BE_IN_BINDING_MNGR is set. Also clean up the signature for nsBindingManager::ChangeDocumentFor.

There are other ChangeDocumentFor functions that should likely be cleaned up, but I wanted to focus on just the perf critical BindToTree/UnbindFromTree code in this bug.

Comment 3

[Timothy Nikkel (:tnikkel)]

The last chunk of the accesskey patch looks like it belongs in the XBL patch.

Comment 4

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

Comment on attachment 444250 [details] [diff] [review]
Speed up accesskey

The XBL part shouldn't be in this patch.

Please make sure we have tests for accesskey changes, and if we don't, add
some.

Comment 5

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

Comment on attachment 444251 [details] [diff] [review]
Speed up XBL

Yeah, I was going to do this myself :)

Comment 7

[Justin Wood (:Callek)]

Comment on attachment 444251 [details] [diff] [review]
Speed up XBL

Had to back this out due to burnings.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Landed a new patch that's merged to trunk. Another ChangeDocumentFor caller had been added which needed to use RemovedFromDocument.

So these two checkins are the final ones:
http://hg.mozilla.org/mozilla-central/rev/f792d441a233
http://hg.mozilla.org/mozilla-central/rev/058e0244ecf6