Skip to content

Commit

Permalink
Perf tweak as suggested by @cjihrig
Browse files Browse the repository at this point in the history 
Double negation considered slower than a straight null check.
  • Loading branch information
C J Silverio committed Feb 12, 2015
1 parent b0ed45b commit aac9a5e
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion lib/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ exports.isRegExp = isRegExp;

function isObject(arg) {
var type = typeof arg;
return type === 'function' || type === 'object' && !!arg;
return type === 'function' || type === 'object' && arg !== null;

This comment has been minimized.

Sign in to view

Copy link
@Fishrock123

Fishrock123 Feb 12, 2015

if we are going for perf tweaks, there was a discussion recently on something similar which noted that setting a var is less efficient. (not setting one allows it to inline better I think.)

ala

return typeof arg === 'function' || typeof arg === 'object' && arg !== null;

Edit: here's a source: nodejs#847 (comment)

This comment has been minimized.

Sign in to view

Copy link
@chrisdickinson

chrisdickinson Feb 18, 2015

That's unlikely to yield a ton of improvement. V8 is pretty good about allocating registers and reusing values. Omitting vars for the sake of V8 sacrifices readability in tribute to the JIT micro-optimization god, who is a very fickle, uncaring god indeed.

This comment has been minimized.

Sign in to view

Copy link
@petkaantonov

petkaantonov Feb 18, 2015

The relative difference in performance is massive (5x in 32-bit V8 3.30.33.16) while the readability is not affected that much. For such a low level operation it would be unreasonable trade-off to do here.

This comment has been minimized.

Sign in to view

Copy link
@petkaantonov

petkaantonov Feb 18, 2015

The reason for the huge difference is that V8 is basically hard-coded to recognize typeof expr === CONSTANT_STRING and compile that to intrinsic type check, where as other patterns actually allocate the type string somewhere and do string comparison.
}
exports.isObject = isObject;

Expand Down

0 comments on commit aac9a5e

Please sign in to comment.