Opened 9 years ago

Closed 8 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 9 years ago by bill

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 think typeof 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.

comment:2 Changed 9 years ago by Les

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 9 years ago by bill

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 9 years ago by Les

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 9 years ago by Les

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 9 years ago by bill

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 9 years ago by bill

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 9 years ago by Adam Peller

Milestone: 1.51.6

comment:9 Changed 8 years ago by bill

Milestone: 1.6future

(sadly) punting seemingly abandoned ticket and meta tickets to future

comment:10 Changed 8 years ago by bill

Milestone: future1.7

I'm going to checkin a hash version against "typeof value" for #12476.

comment:11 Changed 8 years ago by bill

Resolution: fixed
Status: newclosed

(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.

Note: See TracTickets for help on using tickets.