isEqual performance optimization #2180
Conversation
This rolls back to commit 4e2d94d.
Improved performance in FF JIT by 1/4, and by 10% in Chrome.
| return eq(a, b); | ||
| if (a === b) return a !== 0 || 1 / a === 1 / b; | ||
| if (typeof a != 'object' && typeof b != 'object') return a != a && b != b; | ||
| return a != null && b != null && deepEq(a, b); |
There was a problem hiding this comment.
You might wonder why eq function is inlined here.
Answer is that it gives 25-30% more performance in firefox, and about 10-15% in Chrome.
There was a problem hiding this comment.
Why is this not implemented as eq(a, b) looks like a lot of redundant code
There was a problem hiding this comment.
Cause it's 25-30% slower. I have implemented it in eq initially, and measured the result. Was not good in FF. Intentionally redundant code (3 lines), which makes firefox significantly faster.
|
I think this is getting more complicated than is needed. @megawac's original code is able to get the same performance with common usage without another function or duplicating all the checks already in |
|
@jridgewell, in fact, it's quite opposite. This is an only line of additional logic this change adds, all other are optimizations of existing code for modern browser's JIT. And in fact, "original code" is 2-3 times slower than this _.isEqual in every use case in Chrome. It's slower in every browser except FF. Different browsers have different JITs, you can't optimize just for one. |
|
Hey. Looks like lodash guys are already testing the fix, improving performance to match this function. :) http://jsperf.com/underscore-isequal/6 That's enough. You do whatever you would like to do. I'm switching to lodash. Bye. |
|
Ok, that was my last commit. Now you have one of the fastest recursive deepEq implementations currently available. Unfortunately, I have no time to deal with that issue any more. |
|
Should this be closed then? |
| if (a === b) return a !== 0 || 1 / a === 1 / b; | ||
| var typeA = typeof a; | ||
| return typeA != 'function' && typeA != 'object' && typeof b != 'object' ? | ||
| a != a && b != b : a != null && b != null && deepEq(a, b); |
There was a problem hiding this comment.
This is going to fail _.isEqual(null, null).
There was a problem hiding this comment.
Of course it's not. _.isEqual(null, null) will go through 1208, which in fact is the first line of the original eq function unchanged.
Sometimes it's so hard to find a valid reason to close PR, right? :) Then some bullshit in comments comes to help. And then, brilliant idea instantly comes to your head and you create your own similar PR. Good job :) Gratz :)
|
Yap. Micro-opt is micro-opt. |
Optimized isEqual for speed on primitive types. Now it's multiple times faster than lodash in all browsers.
Performance of isEqual is critical for Backbone.Model.set performance.
http://jsperf.com/underscore-isequal/5
Integer comparison:
Chrome: 70x faster
Firefox: 50x faster
Safari: 140x faster