Bug Tracker

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#13804 closed bug (patchwelcome)

removing quickIs from event.js make selectors matching much slower during event delegation

Reported by: mshipov@… Owned by:
Priority: low Milestone: None
Component: event Version: 2.0.0
Keywords: Cc:
Blocked by: Blocking:

Description

I found commit, where quickIs is moved to the core.js https://github.com/jquery/jquery/commit/713cc8609637a943f77039726d96f5e9f7a36f2b#src/event.js

and related to that commit ticket (http://bugs.jquery.com/ticket/11826)

Here is jsperf test with params to jQuery.find, which normally be added to it in case of selectors matching when delegating to document.body http://jsperf.com/jquery-1-7-2-quickis-vs-jquery-1-9-1-find

Change History (9)

comment:1 Changed 6 years ago by gibson042

Component: unfiledevent
Priority: undecidedlow
Status: newopen

Thanks for the report! Unfortunately, your comparison case had a flaw in it; the second argument to quickIs was not a string but a regular expression match array.

Updating and including the surrounding code shows a much smaller (but still very significant) performance difference: http://jsperf.com/jquery-1-7-2-quickis-vs-jquery-1-9-1-find/3. As you can see there, the obvious improvement of using matchesSelector instead of querySelectorAll is actually slightly slower on current browsers.

We got a lot of file size savings from removing the quick* oversimplifications, but on the other hand delegated target calculation can be a fairly hot code path. I'll leave this open for discussion.

comment:2 Changed 6 years ago by anonymous

Sorry for incorrect test. But you made a mistake also :). quickMatch must be calculated only once, when listener added. But it is not give any significant perfomance improvement. http://jsperf.com/jquery-1-7-2-quickis-vs-jquery-1-9-1-find/4

Let me describe situation, wich reveal the problem. In app, that we build for mobile devices, removing of quickIs increase the delay between user move finger for scroll and time, when page actualy begin scroll. There also were some touchmove event listeneres and with them scroll lag all the time. Of course delay was before, but it was very small. Now it is big enough, so user can feel it.

comment:3 Changed 6 years ago by gibson042

Question: why are you using delegated events for that case? It sounds like you'd get a significant speedup from direct binding.

comment:4 Changed 6 years ago by michail13

I have single page web app with multiple views, each of wich have about 500 same components with same listeners. There is big benefit from binding only one listener for all this components when page initially loading, instead of binding listener for each component, and than rebinding them after page content refresh. And, like i said earlier, there was no problems with event delegating perfomance.

comment:5 Changed 6 years ago by gibson042

Event delegation incurs a substantial penalty, with or without quickIs. If performance matters, you really should find a way to take advantage of direct binding and optimize the hell out of your handlers: http://jsperf.com/jquery-1-7-2-quickis-vs-jquery-1-9-1-find/5

$( container ).on({
  bubblingEvent: function( evt ) {
    // Check for match
    var target;
    jQuery( evt.target ).parents().each(function( el ) {
      if ( fastCheckExpression ) {
        // Store and stop checking
        target = el;
        return false;
      }
    });
    if ( !target ) {
      return;
    }

    // Handler body here
  },
  //…
});

comment:6 Changed 6 years ago by dmethvin

I have single page web app with multiple views, each of which have about 500 same components with same listeners.

That's a great scenario for delegated events, but if this is a mobile page you don't really have all 500 components on the page at once do you? If so, that's the problem.

Definitely quickIs is a faster path for the subset of selectors it handles, but it is hard to find real-life scenarios where that extra speed makes a difference. And when it does, they tend to be pathological like having 500 of something on the page at once on a device the size and speed of a phone.

If you need to use delegated events for those situations, something like the code gibson042 shows will be even faster than quickIs, especially since it might be able to just check event.target for example whereas we have to go all the way up the DOM tree looking for matches.

comment:7 Changed 6 years ago by michail13

component - in my case it's not a jquery ui component or something, that require dom object to self initialization. They are light enough and quite fast in creation.

They are just provide same behaviour. And they dont handle same event all together at one moment. Just one of them, were event actualy started.

So amount of components on the page is not the problem in case of event delegation perfomance.

Answering on your suggestion: i don't want duplicate delegation functionality from jquery because i absolutely satisfied of its perfomance but only with quickIs. Solution for me at this moment is localy restore quickIs. It is much simplier than implementing delegating functionality from scratch.

comment:8 Changed 6 years ago by gibson042

Resolution: patchwelcome
Status: openclosed

comment:9 Changed 6 years ago by dmethvin

If a patch is submitted I'd also like to see a clear case of code where the performance diff mattered, not just a jsperf.

Note: See TracTickets for help on using tickets.