Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use instanceof operator instead of class comparison optimization #2804

Merged
merged 1 commit into from
Feb 17, 2016
Merged

use instanceof operator instead of class comparison optimization #2804

merged 1 commit into from
Feb 17, 2016

Conversation

marijaselakovic
Copy link
Contributor

It is significantly faster to use instanceof operator for type checking instead of class comparison (jsperf test:http://jsperf.com/functiontype ). The optimization is to put instanceof check at first place.

@seven-phases-max
Copy link
Member

I wonder if the code in the linked jsperf-snippet is actually fair and well formed. There the functions are defined too close to the test code itself plus there're only 3 functions. So technically, certain JS engine can just track all this right at the first iteration and then simply move instance of evaluation out of the loop (thus achieving infinite performance improvement).

But I'm not really jsperf-snippet writing expert so any further insights are welcome.

@marijaselakovic
Copy link
Contributor Author

Of course, the actual performance improvement depends on how often you call the modified code and what is the input, but the real reason why instanceof performs better on for example V8 is because the engine disables optimizations for toString and Object accesses.

@lukeapage
Copy link
Member

This makes sense to me.. I would merge.

@seven-phases-max
Copy link
Member

This makes sense to me.. I would merge.

OK. Then the only question is: if we add arg instanceof Function should not we remove Object.prototype.toString.call(arg) === '[object Function]' then at all? As far as I understand the only reason to use it was some old Chrome/WebKit versions that returned "typeof 'function' for RegExp", and if we put instanceof, this toString.call(arg) thing is not going to ever trigger...

@lukeapage
Copy link
Member

I was going to suggest that.. but wanted to research what that effects. If its super old chrome only, lets remove it.

@seven-phases-max
Copy link
Member

As far as I understand the issue applies to typeof (ref.) only and instanceof will work as expected in either version (and anywhere).

@marijaselakovic
Copy link
Contributor Author

I found something like this in several sources: http://javascript.info/tutorial/instanceof
The point is if you have objects created in different frames, they do not share the same prototype, so instanceof fails:
var iframe = document.createElement('iframe');
document.body.appendChild(iframe);
xArray = window.frames[window.frames.length-1].Array;
var arr = new xArray(1,2,3);
arr instanceof Array; // false

@seven-phases-max
Copy link
Member

The point is if you have objects created in different frames,

And this just can't ever happen inside the parser. Any arg value passed to this function is created within the same parser instance and it's simply impossible to get there from different window frame.
(Basically if it was the issue it would also apply to every other instanceof usage within the parser and the compiler in general, thus breaking everything).
So I guess I'll simply merge it and remove the redundant condition.

seven-phases-max added a commit that referenced this pull request Feb 17, 2016
use instanceof operator instead of class comparison optimization
@seven-phases-max seven-phases-max merged commit 67c3875 into less:master Feb 17, 2016
@seven-phases-max
Copy link
Member

And as usual... Thanks! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants