Opened 13 years ago
Closed 13 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 13 years ago by
comment:2 Changed 13 years ago by
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 13 years ago by
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 13 years ago by
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 follow-ups: 6 7 Changed 13 years ago by
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 Changed 13 years ago by
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 Changed 13 years ago by
Replying to doughays:
See Flanagan's comments. I wrote some code that confirms his explanation. I tried it in FF 3.5.
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 13 years ago by
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 throughObject.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:
- Let O be the result of calling
ToObject
passing the this value as the argument. - Let class be the value of the
[[Class]]
internal property of O. - 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 13 years ago by
Milestone: | tbd → 1.4 |
---|---|
Owner: | changed from anonymous to James Burke |
Typo fixed (run this code):