Performance degradation with delegate events and pseudo-classes

[menno.vanslooten@…]

Steps to reproduce

Bind a delegate event handler to the document where the selector contains a pseudo-class.

Expected result:

The number of internal calls for the handler is only dependent on the DOM depth of the element that receives the event.

Actual result

The number of internal calls is dependent on the total size and complexity of the DOM.

Example

If a delegate event handler is added to the document like so:

$(document).on('click', 'button', function() {});

The number of calls internally in jQuery when an element is clicked is only dependent on the DOM depth of the element clicked.

But if such a handler is added for a pseudo selector:

$(document).on('click', ':submit', function() {});

The number of internal calls is now suddenly dependent on the total size of the DOM and grows very quickly.

Open the following jsfiddles in Firefox (+firebug):

 http://jsfiddle.net/mennovanslooten/DySJq/4/ This fiddle has a single paragraph and results in 223 internal calls.

 http://jsfiddle.net/mennovanslooten/EZGys/5/ This fiddle has 10 paragraphs and results in 295 internal calls. Even though the paragraphs are in parts of the DOM that are not affected by the event.

 http://jsfiddle.net/mennovanslooten/u9Cmr/1/ This fiddle uses the jquery.com homepage source as an example of a more complex DOM and results in 2993 calls.

The number of calls grows very quickly. For a project I'm currently working on it's almost 12K on some pages resulting in seconds of latency.

[timmywil]

The expected result is invalid. You're delegating from the document, which is the entire DOM. Take advantage of event delegation by limiting it to a narrower context and it won't have to check every parent.

However, the fiddles made me notice something else. Rather than doing a selection and using .index(), perhaps .is() with contains() would be faster.

[dmethvin]

Ouch! Yes, we should get matchesSelector goodness where possible. Although it seems like the killer here is just unwise selector and delegation point choice. The importance of both is  documented in the Event Performance discussion, I gave a  presentation on it last year, I've warned about it in other bug tickets, and mentioned it in  commit messages.

[menno.vanslooten@…]

The expected result is invalid. You're delegating from the document, which is the entire DOM. Take advantage of event delegation by limiting it to a narrower context and it won't have to check every parent.

I know it's bad practice but to say the expected result is invalid is not doing the issue justice, I feel. Neither 1.8.0 nor 1.7.2 show this behavior. Nor is it showing this behavior when a non-pseudo selector is used.

Although it seems like the killer here is just unwise selector and delegation point choice.

I won't deny that it's not the best idea I've ever had. To me the killer is that it looks like jQuery checks the entire DOM in stead of only the chain of ancestors whenever a pseudo selector is involved. Has something changed internally making this a worse idea than it was in the past? I just want to understand.

[josh@…]

Just hit the same issue on github.com. Upgraded to 1.8.1 earlier today and clicking anything on a big dom was painfully slow.

I can look into a patch to Sizzle if someone isn't already working on one.

[dmethvin]

@josh, what do your selectors look like? If they're standard CSS3 selectors the problem should be fixed when we switch back to .is() which will end up using the browser's matchesSelector.

[josh@…]

@dmethvin yeah, no custom pseudo extensions.

Reverting this commit basically fixes the issue for us.  https://github.com/jquery/jquery/commit/afd717df9e189d3e374ce938f03e2712f31c29e3

[Dave Methvin]

Fix #12436, make delegated events fast again. Close gh-923.

Retains the rooted-at-delegateTarget behavior fixed in #12383 by afd717df9e.

Changeset: 9b67b4c0ef311f96d6e035fd38bbfbe42a82b392

[mikesherov]

#12510 is a duplicate of this ticket.

[timmywil]

#12513 is a duplicate of this ticket.

[timmywil]

#12543 is a duplicate of this ticket.

[latchkey]

I think this just broke my site.

 http://jsfiddle.net/ZzPtf/

Works fine with 1.8.1, but breaks with 1.8.2.

[dmethvin]

@latchkey, that was never valid. The selector must be a string. This is a closed ticket. Ask for help on the forum.

[info@…]

The problem is still existent.

We use something like this in our code and you can feel the performance decrease since 1.8.1. (Commit:  https://github.com/jquery/jquery/commit/afd717df9e189d3e374ce938f03e2712f31c29e3)

$("#elementWithID").on('click','.elem:eq(1)', function(){...});

Changeset:  https://github.com/jquery/jquery/commit/9b67b4c0ef311f96d6e035fd38bbfbe42a82b392 didn't solve the performance issue.

Tested with: v1.8.3pre d70e64bff2660cf5a801fe818f1170d5ea93e4fe and 1.8.2

We can't use pseudo-selectors in our delegation events as long this issuse is present.

[mikesherov]

Timmy, thoughts? Needsdocs? Or is there still another fix here?

[dmethvin]

Please don't throw comments onto a closed ticket if a problem still exists. Instead, open a new ticket WITH a test case so we can investigate.

[timmywil]

The links don't help much. "All pseudo selectors" is an overstatement. It is only combinators (which will use qsa) and POS selectors (which won't) that still use a selection with .index. All other selectors will do a simple contextual match and no longer have the performance regression.