Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#8764 closed enhancement (fixed)

_Widget.connect performance is horrible

Reported by: Nathan Toone Owned by: Nathan Toone
Priority: high Milestone: 1.3
Component: Dijit Version: 1.3.0b1
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

The connect uses an extra dojo.hitch in all cases - when it really isn't necessary. This really hurts performance, and in IE it is extremely poor.

A patch follows. Timings on my machine (prior to patching in parenthesis):

IE8 100 "onblur" connects - 16ms (31ms)
IE8 100 "ondijitclick" connects - 63ms (63ms)
IE8 all 200 disconnects - 16ms (16ms)
FF3 100 "onblur" connects - 13ms (20ms)
FF3 100 "ondijitclick" connects - 69ms (65ms)
FF3 all 200 disconnects - 20ms (17ms)

This yields nearly 50% improvement for the common case of non-'ondijitclick' connections. The "ondijitclick" function could probably be further refactored for better performance as well.

Attachments (2)

_WidgetConnectPerformance.patch (6.3 KB) - added by Nathan Toone 11 years ago.
Patch and test case for _Widget.connect performance improvement
_WidgetConnectPerformance.2.patch (3.8 KB) - added by Nathan Toone 11 years ago.
Lower-risk patch which improves IE8 performance to 15, 47, and 16ms and FF3 performance to 13, 52, 16ms

Download all attachments as: .zip

Change History (9)

Changed 11 years ago by Nathan Toone

Patch and test case for _Widget.connect performance improvement

comment:1 Changed 11 years ago by bill

Description: modified (diff)
Milestone: tbd1.4
Owner: set to Nathan Toone

Hmm, thanks for the patch/tests.... I agree that the dojo.hitch() is unneeded. Looks like someone was overzealous in byte shaving.

Although it does look like the patch is adding a lot of branching for ondijitclick vs onclick (changing the return type so you have to test if(dojo.isArray(c))), that might hurt us more than helping us, as it's increasing the code size.

In any case let's add this for 1.4.

Changed 11 years ago by Nathan Toone

Lower-risk patch which improves IE8 performance to 15, 47, and 16ms and FF3 performance to 13, 52, 16ms

comment:2 Changed 11 years ago by Nathan Toone

Also - since we don't need the parameter normalization that's provided us by dojo.connect, we can speed up even more by calling dojo._connect directly.

comment:3 Changed 11 years ago by bill

Talked to Nathan over email about previous comment. It means that dojo.connect adds (above and beyond dojo._connect) various function signatures - so that you can call either:

dojo.connect(obj, "fx", obj2, "fx2")
dojo.connect(obj, "fx", "fx2")

Since _Widget.connect always are calling it with 4 parameters, we can call into dojo._connect directly and avoid the array manipulation that comes with dojo.connect.

comment:4 Changed 11 years ago by bill

Patch looks good to me.

comment:5 Changed 11 years ago by Nathan Toone

Resolution: fixed
Status: newclosed

(In [16901]) Fixes #8764 - add in some performance improvements to the connect function in _Widget.js !strict

comment:6 Changed 10 years ago by Adam Peller

Milestone: 1.41.3

comment:7 Changed 10 years ago by bill

In [16933], fixed the code above.

Note: See TracTickets for help on using tickets.