Bug Tracker

Opened 6 years ago

Closed 5 years ago

#14147 closed bug (migrated)

jQuery.event.fix() may force layout

Reported by: anonymous Owned by:
Priority: low Milestone: 1.next/2.next
Component: event Version: 1.10.2
Keywords: Cc: paul.irish
Blocked by: Blocking:

Description

Accessing some of the event properties while copying them (like jQuery.event.fix() does in event[ prop ] = originalEvent[ prop ] at https://github.com/jquery/jquery/blob/master/src/event.js#L511) may force style recalculation and layout.

Screenshots of Timeline in Chrome Developer Tools: http://i.imgur.com/Zuh6WDE.png http://i.imgur.com/JWLYF2N.png

Change History (11)

comment:1 Changed 6 years ago by dmethvin

I could certainly believe this happens for events like mousemove where we populate and normalize the x/y position in our Event object. Browsers may defer doing that work until something asks for the position, and in this case that something is jQuery. Not sure it can be avoided though if we want this information to be in our object.

Adding paul.irish for his insight.

comment:2 Changed 6 years ago by dmethvin

Blocking: 14164 added

comment:3 Changed 6 years ago by timmywil

Cc: paul.irish added
Component: unfiledevent
Priority: undecidedlow
Status: newopen

comment:4 Changed 6 years ago by dmethvin

Blocking: 14164 removed

(In #14164) It's blocked by an open ticket, but it is probably better to handle the others on a case-by-case basis rather than a meta-ticket.

comment:5 Changed 6 years ago by jbedard

I recently ran across this issue as well, which lead me here.

How about using ES5 getters to delay the copying/calculating of event properties? Would that be an option for 2.0?

Something similar to what was previously discussed on the forum and more recently the original post author blogged about a solution.

comment:6 Changed 6 years ago by dmethvin

The problem is that ES5 getters are (still) slow in just about every browser. There are also some bugs and quirks, even in our list of 2.x-supported browsers. https://bugzilla.mozilla.org/show_bug.cgi?id=626021

It's true that if you never get any event properties, or perhaps only get one property one time, it can be faster. But in the case of a mousemove event for example, I suspect it's going to be extremely common that the caller's own event handler will want that information and would have forced the layout anyway.

Additionally, the layout is forced here only if you enter with a "dirty" layout from some other DOM change caused by a previous event or handler.

comment:7 Changed 6 years ago by jbedard

ES5 getters are definitely slow still, but after trying this out I think this might still be worth it.

Don't you think most event handlers only access a few event properties? Yet the $.event.fix loop accesses them all. The fix loop is often a bottleneck on its own without a forced layout.

The scenario I've seen this bug occur a few times is toggling a class on mouse enter/leave. So this bug forces a layout, then the event listener simply toggles a class based on the event.type which sets the dirty flag again. In this case that extra layout is completely unnecessary, even the $.event.fix loop copying everything was unnecessary.

The only problem would be the ES5 getters. But at least in FF/chrome when only calling those getters 0-10 times it seems worth it.

Would you be interested in a pull request to at least see it in action?

comment:8 Changed 6 years ago by dmethvin

I think this could be examined as part of #12031 which I wanted to tackle for 1.12/2.2. There are some issues to be decided there, but potentially you could just use the native event object for 2.x and skip "fixing" entirely.

comment:9 Changed 6 years ago by jbedard

If #12031 provides a new event interface which skips $.event.fix that would be great, especially if it still provides things such as event namespaces & selectors.

But if anyone cares to look at what I tried: https://github.com/jbedard/jquery/commit/fd617744638ae16299c10cbcdb2935e3f45c5f27

This turns all hooks into es5 getters instead of filters and prop copying. I think the es5 setter is actually slower then just calling the getter over and over (needs testing). This avoids the forced layout and removes the $.event.fix loop, but does make accessing the properties more expensive due to the getter. In the cases I tried this was well worth it but I assume the change is too big/risky for such a stable API...

comment:10 Changed 6 years ago by dmethvin

Milestone: None1.next/2.next

comment:11 Changed 5 years ago by m_gol

Resolution: migrated
Status: openclosed
Note: See TracTickets for help on using tickets.