Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#9879 closed task (fixed)

[cla/patch] dijit.byId optimization

Reported by: Les Owned by: bill
Priority: high Milestone: 1.4
Component: Dijit Version: 1.3.2
Keywords: Cc:
Blocked By: Blocking:

Description

How about this function to optimize the frequntely used dijit.byId?

dijit.byId = function(/*String|Widget*/id){
   return dijit.registry._hash[id] || id;
}

Here's the current dijit.byId:

dijit.byId = function(/*String|Widget*/id){
	// summary:
	// Returns a widget by it's id, or if passed a widget, no-op (like dojo.byId())
	return (dojo.isString(id)) ? dijit.registry.byId(id) : id; // Widget
};

The new dijit.byId() is well over 2x faster in IE8 and 50% faster in FF 3.5 or Chrome 3.5.

Change History (9)

comment:1 Changed 10 years ago by Les

Chrome 4, not 3.5

comment:2 Changed 10 years ago by Les

My tests:

http://archive.dojotoolkit.org/nightly/dojotoolkit/dijit/tests/test_Menu.html

Run:

var byId = function(/*String|Widget*/id){
   return dijit.registry._hash[id] || id;
}

console.time('new');
for(var i=0; i<100000; i++) {
   byId('copy');
}
console.timeEnd('new')

console.time('old');
for(var i=0; i<100000; i++) {
   dijit.byId('copy');
}
console.timeEnd('old');

var widget = dijit.byId('copy');
byId(widget);

Results:

FF 3.5 (Win) 686ms --> 352ms
Safari 4 (Win) 73ms --> 57ms
IE 8 1484ms --> 625ms
Chrome 4 397ms --> 251ms

Results show remarkable performance improvement for all tested browsers.

comment:3 Changed 10 years ago by Les

IE 6: 2047ms --> 1109ms
IE 7: 1648ms --> 801ms

comment:4 Changed 10 years ago by bill

Hmm, but it's failing the unit test (manager.html) since dijit.byId("nonexistant") returns "nonexistant" rather than null.

comment:5 Changed 10 years ago by lipik

I bet the following will preserve the speed enhancement for common usage, while being correct (though slower) for the rare cases when either a widget is passed, or the id is non-existent:

dijit.byId = function(/*String|Widget*/id){
   return dijit.registry._hash[id] || (!dojo.isString(id) && id);
}

comment:6 Changed 10 years ago by Les

This is should work:

var byId = function(/*String|Widget*/id){
   return (typeof id == "string") ? dijit.registry._hash[id] : id;
}

I still see improvement:

IE6: 2062ms --> 1250ms
FF 3.5 465ms --> 237ms
Chrome 4: 63ms --> 44ms
Safari 4 (Win) about the same
IE8: 1000ms --> 453ms

comment:7 Changed 10 years ago by bill

Milestone: tbd1.4
Owner: set to bill
Status: newassigned

Switching the dojo.isString() to typeof id == "string" will definitely improve performance. BTW this is listed in #9131. Could alternately follow dojo.byId()'s lead and do:

return id.id ? dijit.registry._hash[id] : id;

(or something like that, id.attributeMap etc.)

I like lipik's suggestion to improve the common case but I've seen problems indexing into a hash using a (non-string, non-integer) Object. StackContainer used to do this and I saw some bugs, hence [19970].

So I'll just change it to use typeof for now.

comment:8 Changed 10 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [20107]) Dijit performance optimizations from Les Szklanny (CLA on file), thanks! Fixes #9885, #9875, #9879, refs #9131. !strict

comment:9 Changed 10 years ago by bill

Type: enhancementtask

seems like these aren't really enhancements from the users' point of view, labeling as tasks instead

Note: See TracTickets for help on using tickets.