Opened 10 years ago

Closed 10 years ago

#10139 closed task (fixed)

[patch/cla] dojo.isFunction simplification

Reported by: Les Owned by: James Burke
Priority: high Milestone: 1.4
Component: Core Version: 1.4.0b
Keywords: isFunction Cc:
Blocked By: Blocking:

Description

The current dojo.isFunction() is quite complicated. Here's a new dojo.isFunction() which is much simpler and it passes all dojo and jQuery isFunction() tests.

dojo.isFunction = function(it){
   return Object.prototype.toString.call(it) === "[object Function]";
};

I tested it on Windows (Safari 4, Chrome 4, Opera 10, IE8, IE6, and FF 3.5). Depending on the browser performance is about the same or marginally worse. Robustness is the same, but the new code is much shorter and it has no browser-specific branches. Inlining improves performance.

dojo.isFunction() is used frequently in dojo. For example, the themeTester application calls it well over 20,000 times when it loads.

Test setup:
http://archive.dojotoolkit.org/nightly/dojotoolkit/dojo/tests/NodeList-manipulate.html

Run:

dojo.isFunction2 = function(it){
   return Object.prototype.toString.call(it) === "[object Function]";
};

var newf = new Function();
var dojof = dojo.isFunction;
var func = function(){};
var ele = dojo.doc.getElementsByName("html");
var obj = dojo.doc.createElement("object");
var o = {x:15, y:20};
var str = "function";

console.log(dojo.isFunction(newf) === dojo.isFunction2(newf));	
console.log(dojo.isFunction(dojof) === dojo.isFunction2(dojof));
console.log(dojo.isFunction(ele) === dojo.isFunction2(ele));
console.log(dojo.isFunction({}) === dojo.isFunction2({}));
console.log(dojo.isFunction(obj) === dojo.isFunction2(obj));	
console.log(dojo.isFunction(o) === dojo.isFunction2(o));
console.log(dojo.isFunction(func) === dojo.isFunction2(func));
console.log(dojo.isFunction(str) === dojo.isFunction2(str));
console.log(dojo.isFunction(null) === dojo.isFunction2(null));   // false

console.time('new');
for(var i=0; i<20000; i++) {
   dojo.isFunction2(obj);
   dojo.isFunction2(o);
   dojo.isFunction2(str);
   dojo.isFunction2(func);
   dojo.isFunction2(null);
}
console.timeEnd('new');

console.time('old');
for(var j=0; j<20000; j++) {
   dojo.isFunction2(obj);
   dojo.isFunction2(o);
   dojo.isFunction2(str);
   dojo.isFunction2(func);
   dojo.isFunction2(null);
}
console.timeEnd('old');

console.time('inline');
for(var k=0; k<20000; k++) {
   Object.prototype.toString.call(obj) === "[object Function]";
   Object.prototype.toString.call(o) === "[object Function]";
   Object.prototype.toString.call(str) === "[object Function]";
   Object.prototype.toString.call(func) === "[object Function]";
   Object.prototype.toString.call(null) === "[object Function]";
}
console.timeEnd('inline');

Change History (10)

comment:1 Changed 10 years ago by Les

Typo fixed (run this code):

dojo.isFunction2 = function(it){
   return Object.prototype.toString.call(it) === "[object Function]";
};

var newf = new Function();
var dojof = dojo.isFunction;
var func = function(){};
var ele = dojo.doc.getElementsByName("html");
var obj = dojo.doc.createElement("object");
var o = {x:15, y:20};
var str = "function";

console.log(dojo.isFunction(newf) === dojo.isFunction2(newf));	
console.log(dojo.isFunction(dojof) === dojo.isFunction2(dojof));
console.log(dojo.isFunction(ele) === dojo.isFunction2(ele));
console.log(dojo.isFunction({}) === dojo.isFunction2({}));
console.log(dojo.isFunction(obj) === dojo.isFunction2(obj));	
console.log(dojo.isFunction(o) === dojo.isFunction2(o));
console.log(dojo.isFunction(func) === dojo.isFunction2(func));
console.log(dojo.isFunction(str) === dojo.isFunction2(str));
console.log(dojo.isFunction(null) === dojo.isFunction2(null));   // false

console.time('new');
for(var i=0; i<20000; i++) {
   dojo.isFunction2(obj);
   dojo.isFunction2(o);
   dojo.isFunction2(str);
   dojo.isFunction2(func);
   dojo.isFunction2(null);
}
console.timeEnd('new');

console.time('old');
for(var j=0; j<20000; j++) {
   dojo.isFunction(obj);
   dojo.isFunction(o);
   dojo.isFunction(str);
   dojo.isFunction(func);
   dojo.isFunction(null);
}
console.timeEnd('old');

console.time('inline');
for(var k=0; k<20000; k++) {
   Object.prototype.toString.call(obj) === "[object Function]";
   Object.prototype.toString.call(o) === "[object Function]";
   Object.prototype.toString.call(str) === "[object Function]";
   Object.prototype.toString.call(func) === "[object Function]";
   Object.prototype.toString.call(null) === "[object Function]";
}
console.timeEnd('inline');

comment:2 Changed 10 years ago by bill

BTW the current code has FF2 processing in it that can be removed, the node.nodeType check:

//Firefox thinks object HTML element is a function, so test for nodeType.
return it && (t == "function" || it instanceof Function) && !it.nodeType; // Boolean

(On FF3+ typeof returns "object".)

But that still leaves a special safari version of the code so probably your way is better.

Do you have numbers on where the most calls to dojo.isFunction() come from?

comment:3 Changed 10 years ago by Les

7536 isFunction() calls come from this code:

var _attrReg = {},
	getAttrReg = function(dc){
	if(!_attrReg[dc]){
		var r = [],
			attrs,
			proto = dojo.getObject(dc).prototype;
		for(var fxName in proto){
			window.countProto++;
			if(dojo.isFunction(proto[fxName]) && (attrs = fxName.match(/^_set([a-zA-Z]*)Attr$/)) && attrs[1]){

1789 calls come from this function:

function safeMixin(target, source){
...

comment:4 Changed 10 years ago by bill

It's interesting.

As for getAttrReg(), which has been renamed to getSetterAttributes(), if you trace up a few levels to _applyAttributes() all that we are really want to do is to loop through and call this.attr(name, value) on every widget attribute that occurs in either attributeMap or has a custom setter (or both, which happens due to subclassing). So there are a lot of ways to do that which I was playing with last night, many of which don't use isFunction() or regexp's at all. I filed #10152 to track that.

As for the isFunction() calls in parser I think there's also some room for improvement there so I filed #10150.

comment:5 Changed 10 years ago by Douglas Hays

2 observations: 1) the following appears to be faster:

function isFunction(it){ return !!it && it.constructor == Function; }

2) neither way is as fast as the typeof it == "function" short-circuit for the normal case of function fubar(){}. So the extra bytes to check the typeof can be far more performant for the more common "function" case and a penalty for the less common "Function" case.

comment:6 in reply to:  5 Changed 10 years ago by Les

Replying to doughays:

I wasn't able to find an example where your isFunction wouldn't work. I took my implementation from a Resig's book. Also, Flanagan provided the same isFunction in his blog, see http://www.davidflanagan.com/2009/08/typeof-isfuncti.html

comment:7 in reply to:  5 Changed 10 years ago by Les

Replying to doughays:

See Flanagan's comments. I wrote some code that confirms his explanation. I tried it in FF 3.5.

Go here: http://www.officemax.com/technology/digital-cameras-digital-video-cameras-digital-camera-accessories/digital-cameras/product-prod2740180

Click the Reviews tab and let the reviews load.

jQuerify the page: http://www.learningjquery.com/2008/06/updated-jquery-bookmarklet

Run in Firebug:

var fn = jQuery('#BVFrame')[0].contentWindow.$bv; // function

function isFunction(it){ return !!it && it.constructor == Function; }

function isFunction2(it){
   return Object.prototype.toString.call(it) === "[object Function]";
}

console.log(isFunction(fn)); // false

console.log(isFunction2(fn))   // true

You will see that isFunction() returns false (wrong). isFunction2() returns true (correct).

comment:8 Changed 10 years ago by Eugene Lazutkin

Let's take a look at the spec (I use Ecma/TC39/2009/025 --- the final draft of ECMA-262 5th edition).

The value of the [[Class]] internal property is defined by this specification for every kind of built-in object. The value of the [[Class]] internal property of a host object may be any String value except one of "Arguments", "Array", "Boolean", "Date", "Error", "Function", "JSON", "Math", "Number", "Object", "RegExp", and "String". The value of a [[Class]] internal property is used internally to distinguish different kinds of built-in objects. Note that this specification does not provide any means for a program to access that value except through Object.prototype.toString (see 15.2.4.2).

And in 15.3.5:

Properties of Function Instances

The value of the [[Class]] internal property is "Function".

That's how 15.2.4.2 is defined:

Object.prototype.toString ( )

When the toString method is called, the following steps are taken:

  1. Let O be the result of calling ToObject passing the this value as the argument.
  2. Let class be the value of the [[Class]] internal property of O.
  3. Return the string value that is the result of concatenating the three strings "[object ", class, and "]".

So this definition:

dojo.isFunction = function(it){
  return Object.prototype.toString.call(it) === "[object Function]";
};

appears to be fully legal and "by the book".

Probably we should "precalculate" the function to make it a little bit faster (remember that it may be called multiple times):

var opts = Object.prototype.toString;
dojo.isFunction = function(it){
  return opts.call(it) === "[object Function]";
};

I think we can even inline the check in performance critical pieces --- it is a simple one-liner after all.

dojo.isString() could be done in the same manner.

comment:9 Changed 10 years ago by James Burke

Milestone: tbd1.4
Owner: changed from anonymous to James Burke

comment:10 Changed 10 years ago by Eugene Lazutkin

Resolution: fixed
Status: newclosed

Fixed in [20644].

Note: See TracTickets for help on using tickets.