Opened 11 years ago
Closed 10 years ago
#10150 closed task (fixed)
optimizations around attribute type computation
Reported by: | bill | Owned by: | bill |
---|---|---|---|
Priority: | high | Milestone: | 1.7 |
Component: | Parser | Version: | 1.4.0b |
Keywords: | Cc: | Les | |
Blocked By: | Blocking: |
Description
There might be some room for optimization in the parser in the code that gets the types of each widget attribute.
Currently this function is used to compute the type of a widget attribute:
function val2type(/*Object*/ value){ // summary: // Returns name of type of given value. if(d.isString(value)){ return "string"; } if(typeof value == "number"){ return "number"; } if(typeof value == "boolean"){ return "boolean"; } if(d.isFunction(value)){ return "function"; } if(d.isArray(value)){ return "array"; } // typeof [] == "object" if(value instanceof Date) { return "date"; } // assume timestamp if(value instanceof d._Url){ return "url"; } return "object"; }
It's probably faster if we can get the type in one operation rather than having a bunch of checks. As Les suggested in #10139 toString() seems useful. I prototyped this and it seems to work but needs to be tested on all browsers and benchmarked:
function val2type(/*Object*/ value){ // summary: // Returns name of type of given value. var ts = Object.prototype.toString.call(value); switch(ts){ case "[object String]": return "string"; case "[object Number]": return "number"; case "[object Boolean]": return "boolean"; case "[object Array]": return "array"; case "[object Date]": return "date"; case "[object Function]": return "function"; default: if(value instanceof d._Url){ return "url"; } return "object"; } }
The other change is that the first time the parser sees a class (like dijit.form.Button) it scans the prototype to get a list of attributes and computes the type of each attribute. But we don't actually need to know the type of an attribute unless it's specified in markup.
So there may be a performance gain by deferring the val2type() call until needed (specifically only doing it on attributes that are specified in markup). That would avoid computations to find out that, for example, postCreate() is a function. It would still be good to cache the results of that computation (I think) but just wait to do it until needed.
Change History (11)
comment:1 Changed 11 years ago by
comment:2 Changed 11 years ago by
I modified this code for testing:
window.countString1 = 0; window.countNumber1 = 0; window.countBoolean1 = 0; window.countArray1= 0; window.countDate1 = 0; window.countFunction1 = 0; window.countObject1 = 0; function val2type(/*Object*/ value){ // summary: // Returns name of type of given value. var ts = Object.prototype.toString.call(value); switch(ts){ case "[object String]": window.countString1++; return "string"; case "[object Number]": window.countNumber1++; return "number"; case "[object Boolean]": window.countBoolean1++; return "boolean"; case "[object Array]": window.countArray1++; return "array"; case "[object Date]": window.countDate1++; return "date"; case "[object Function]": window.countFunction1++; return "function"; default: window.countObject++; if(value instanceof d._Url){ return "url"; } return "object"; }
Here are the results:
countArray1 87 countBoolean 621 countDate1 3 countFunction1 3307 countNumber1 339 countObject1 0 countString1 1295
Based on this info, I'd change the order of the case statements to further improve performance:
switch(ts){ case "[object Function]": return "function"; case "[object String]": return "string"; case "[object Boolean]": return "boolean"; case "[object Number]": return "number"; case "[object Array]": return "array"; case "[object Date]": return "date"; default: if(value instanceof d._Url){ return "url"; } return "object"; }
comment:3 Changed 11 years ago by
Ah good test, didn't realize there were so many functions (presumably the internal functions starting with underscore are already filtered out)... about the order of the case statements, if changing the order really makes a difference then we should probably be using a hash instead of a switch(), something like
var _typeHash = { "[object Function]": "function", ... }; function val2type(/*Object*/ value){ var ret = _typeHash[ts]; if(ret){ return ret; }else{ if(...) ... return "object"; }
comment:4 Changed 11 years ago by
I created a test case. There is a performance gain. Replacing the case statement with a hash lookup improved performance. Actually, not replacing the case statement would decrease performance.
http://archive.dojotoolkit.org/nightly/dojotoolkit/dojo/tests/NodeList-manipulate.html
Run:
function val2type(/*Object*/ value){ // summary: // Returns name of type of given value. if(dojo.isString(value)){ return "string"; } if(typeof value == "number"){ return "number"; } if(typeof value == "boolean"){ return "boolean"; } if(dojo.isFunction(value)){ return "function"; } if(dojo.isArray(value)){ return "array"; } // typeof [] == "object" if(value instanceof Date) { return "date"; } // assume timestamp if(value instanceof dojo._Url){ return "url"; } return "object"; } var arr = []; // 87 var stub = function(){}; // 3307 var num = 10; // 339 var str = "xxx"; // 1295 var bool = true; // 621 var i; console.time('old'); for(i=0; i<87; i++) { val2type(arr); } for(i=0; i<3307; i++) { val2type(stub); } for(i=0; i<10; i++) { val2type(num); } for(i=0; i<1295; i++) { val2type(str); } for(i=0; i<621; i++) { val2type(bool); } console.timeEnd('old'); var _typeHash = { "[object Function]": "function", "[object String]": "string", "[object Boolean]": "boolean", "[object Number]": "number", "[object Array]": "array", "[object Date]": "date" }; function val2typeNew(/*Object*/ value){ var ts = Object.prototype.toString.call(value); var ret = _typeHash[ts]; if(ret){ return ret; }else{ if(value instanceof dojo._Url){ return "url"; } return "object"; } } function val2typeNew2(/*Object*/ value){ var ts = Object.prototype.toString.call(value); switch(ts){ case "[object Function]": window.countFunction1++; return "function"; case "[object String]": window.countString1++; return "string"; case "[object Boolean]": window.countBoolean1++; return "boolean"; case "[object Array]": window.countArray1++; return "array"; case "[object Number]": window.countNumber1++; return "number"; case "[object Date]": window.countDate1++; return "date"; default: window.countObject++; if(value instanceof d._Url){ return "url"; } return "object"; } } console.time('new'); for(i=0; i<87; i++) { val2typeNew(arr); } for(i=0; i<3307; i++) { val2typeNew(stub); } for(i=0; i<10; i++) { val2typeNew(num); } for(i=0; i<1295; i++) { val2typeNew(str); } for(i=0; i<621; i++) { val2typeNew(bool); } console.timeEnd('new'); console.time('new2'); for(i=0; i<87; i++) { val2typeNew2(arr); } for(i=0; i<3307; i++) { val2typeNew2(stub); } for(i=0; i<10; i++) { val2typeNew2(num); } for(i=0; i<1295; i++) { val2typeNew2(str); } for(i=0; i<621; i++) { val2typeNew2(bool); } console.timeEnd('new2');
Results:
IE6 old: 78ms new: 47ms new2: 109ms IE8 old: 31ms new: 16ms new2: 62ms FF 3.5 old: 27ms new: 22ms new2: 43ms
comment:5 Changed 11 years ago by
I forgot to remove the counters. The numbers should be:
IE8 "old:" "47ms" "new:" "15ms" "new2:" "16ms" IE6 "old:" "78ms" "new:" "32ms" "new2:" "47ms" FF 3.5 old: 27ms new: 23ms new2: 23ms
So this change shaved off 45ms on the slowest browsers. Not bad.
comment:6 Changed 11 years ago by
I see, looks like the browsers other than IE6 are smart enough to convert the switch() into a hash internally, but it does help IE6. How does a hash using typeof instead work (for the function/string/number/boolean case)?
BTW looks like there's also room for optimization by inlining the val2type() code, looks like it's only used in one place.
comment:7 Changed 11 years ago by
PS: as Eugene suggested in #10139, should define a variable
var opts = Object.prototype.toString;
outside of the val2type() method to avoid the hash lookups on every call to val2type().
comment:8 Changed 11 years ago by
Milestone: | 1.5 → 1.6 |
---|
comment:9 Changed 10 years ago by
Milestone: | 1.6 → future |
---|
(sadly) punting seemingly abandoned ticket and meta tickets to future
comment:10 Changed 10 years ago by
Milestone: | future → 1.7 |
---|
I'm going to checkin a hash version against "typeof value" for #12476.
comment:11 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [24185]) Refactor parser to allow attributes (for a single node) to be partly specified in data-dojo-props, and partly specified directly ex: value=123. Uses node.attributes to detect which attributes are specified on a node, or for older versions of IE calls cloneNode(false) followed by some regex's on clone.outerHTML.
Due to lowercase/uppercase issues (ex: tabIndex, onClick), and for type conversion, the code still introspects each widget to get it's attribute metadata. In the future, would like to defer/avoid that in the common case. Fixes #10153, #12423, #12476, #10150, #9823, refs #11490 !strict.
Also fixes the problem on IE8 where a button without type=... defaults to type=submit rather than whatever the widget defines the default as, fixes #10163, refs #9334, #8946.
Future updates will be attached to ticket #12476.
PS: the vast majority of widget attributes are string, number, boolean, or function, so if we can detect all of those w/a single
typeof value
that might be faster. I thinktypeof value
will work for functions even on FF and safari, since we no longer support FF2 and since widgets never have a NodeList in their prototype.