#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 11 years ago by
comment:2 Changed 11 years ago by
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:4 Changed 11 years ago by
Hmm, but it's failing the unit test (manager.html) since dijit.byId("nonexistant") returns "nonexistant" rather than null.
comment:5 Changed 11 years ago by
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 11 years ago by
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 11 years ago by
Milestone: | tbd → 1.4 |
---|---|
Owner: | set to bill |
Status: | new → assigned |
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 11 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:9 Changed 11 years ago by
Type: | enhancement → task |
---|
seems like these aren't really enhancements from the users' point of view, labeling as tasks instead
Chrome 4, not 3.5