Skip to main content

Bug Tracker

Side navigation

#2655 closed enhancement (fixed)

Opened April 05, 2008 05:28AM UTC

Closed April 22, 2008 05:27AM UTC

Last modified April 22, 2008 02:29PM UTC

jQuery.event module optimizations

Reported by: flesler Owned by:
Priority: minor Milestone: 1.2.4
Component: event Version: 1.2.3
Keywords: event handle fix Cc:
Blocked by: Blocking:
Description

Hi, here are a few modifications, that I think they will improve perfomance, and some make the code shorter, I'll explain them in case the reason for any change is not clear.

jQuery.event.fix

  • The same event object, can't be fixed more than once (it happens when triggering.

jQuery.event.handle

  • Removed the {} fallback when calling jQuery.event.fix, event.type is used in the next line, so IF you were to fall on the last case, the next line would alert 'event.type is not defined' seems like the 3rd case can't be reached.
  • Avoided the need for Array.prototype.slice.call( arguments, 1 ) and args.unshift().
  • Replaced args[0] for event when looping through the handlers, those were 2 array lookups per handler.
  • Same for the namespace (parts).
  • Simplified the val/ret part, ret is not needed and the code is now shorter.
  • The forced call to preventDefault and stopPropagation is only checked and done once, after the loop.
Attachments (3)

2nd version

  • handle-fix.3.diff (3.2 KB) - added by flesler April 10, 2008 02:40PM UTC.

    3rd Version

  • handle-fix.diff (2.1 KB) - added by flesler April 05, 2008 05:28AM UTC.

    Changes

  • Change History (6)

    Changed April 05, 2008 06:17AM UTC by flesler comment:1

    Merged the first diff with 2 more changes.

    jQuery.event.handle

    • the copy of data and the handler to the event object, is only done if the handler is really going to be called, small change but it's correct and %0.001 faster.
    • I saw the new function that's gets bound and saved into $.data('handle'), I simplified it a bit, it was kinda redundant :)

    Changed April 10, 2008 02:43PM UTC by flesler comment:2

    The third version adds 2 things.

    • It caches the originally: !parts[0] && !event.exclusive in a variable. Those were 2 boolean casting and 2 boolean checks on each iteration.
    • Instead of calling twice jQuery.data, an empty hash if used. It shorter and should be faster.

    Changed April 10, 2008 02:48PM UTC by flesler comment:3

    ugh.. forget the typos in the previous post.. I'm still sleepy.

    Changed April 22, 2008 02:32AM UTC by brandon comment:4

    description: Hi, here are a few modifications, that I think they will improve perfomance, and some make the code shorter, I'll explain them in case the reason for any change is not clear. \ \ jQuery.event.fix \ * The same event object, can't be fixed more than once (it happens when triggering. \ \ jQuery.event.handle \ * Removed the {} fallback when calling jQuery.event.fix, event.type is used in the next line, so IF you were to fall on the last case, the next line would alert 'event.type is not defined' seems like the 3rd case can't be reached. \ * Avoided the need for Array.prototype.slice.call( arguments, 1 ) and args.unshift(). \ * Replaced args[0] for event when looping through the handlers, those were 2 array lookups per handler. \ * Same for the namespace (parts). \ * Simplified the val/ret part, ret is not needed and the code is now shorter. \ * The forced call to preventDefault and stopPropagation is only checked and done once, after the loop.Hi, here are a few modifications, that I think they will improve perfomance, and some make the code shorter, I'll explain them in case the reason for any change is not clear. \ \ jQuery.event.fix \ * The same event object, can't be fixed more than once (it happens when triggering. \ \ jQuery.event.handle \ * Removed the {} fallback when calling jQuery.event.fix, event.type is used in the next line, so IF you were to fall on the last case, the next line would alert 'event.type is not defined' seems like the 3rd case can't be reached. \ * Avoided the need for Array.prototype.slice.call( arguments, 1 ) and args.unshift(). \ * Replaced args[0] for event when looping through the handlers, those were 2 array lookups per handler. \ * Same for the namespace (parts). \ * Simplified the val/ret part, ret is not needed and the code is now shorter. \ * The forced call to preventDefault and stopPropagation is only checked and done once, after the loop.

    Changed April 22, 2008 05:27AM UTC by brandon comment:5

    resolution: → fixed
    status: newclosed

    Mostly applied in Rev [5276].

    Just a couple of things different:

    • The val returned from a handler can be a non-boolean value.
    • An event must always be "fixed" in IE due to its global event object... at least that seems to be the case with my testing so far. I'm holding off on optimizing the event.fix method for lots more testing.

    BTW ... I was seeing over 200% increase in performance of jQuery.event.trigger in IE.

    http://brandonaaron.net/jquery/tickets/2655/baseline-performance-test.html

    http://brandonaaron.net/jquery/tickets/2655/patched-performance-test.html

    Changed April 22, 2008 02:29PM UTC by flesler comment:6

    The val returned from a handler can be a non-boolean value

    It says:

    val = handler.apply( this, arguments ) '''!== false '''&& val;
    

    That means that, unless it is explicitely false, it remains true.

    The val is not returned so it doesn't matter what it is, as long as it's not false...

    I might not be getting it, can you clarify me please ?

    An event must always be "fixed" in IE

    You can set event._fixed_ = null in IE, along with the event.target and the others.

    BTW ... I was seeing over 200% increase in performance of jQuery.event.trigger in IE.

    You mean with a change(s) I proposed ?