Model.set optimization for speed

[gaperton]

• Optimized comparison function for primitive types
• Optimized previous attributes clone operation

As measured in Chrome, Model.set now:

• 10x faster with primitive types
• 3x faster with other types

[akre54]

This is a pretty smart idea, but isn't Underscore the better place to make this change?

We typically like to see benchmarks run through jsperf. Mind creating a simple test case?

[braddunbar]

Yea, that was my thought as well. Looks like a nice addition to _.isEqual.

[jridgewell]

This'll hit issues with NaN, since it isn't reflexive.

[megawac]

This won't work for 0 === -0 or NaN. I expect this to be faster for cases where _isEqual(a, b) is false but roughly the same when a === b (minus JIT optimizations)

[gaperton]

0 === Nan
false

0 === -0
Are you kidding?

[gaperton]

_.isEqual is terribly slow on typical Model.set use cases, like assignments of primitive values like integers. This is not just "faster", but in fact it's 10x faster (open profiler and see), no matter what someone might expect. No one can really predict how JIT works on real program.

[gaperton]

Also, NaN is not serializable to JSON, so it's not an issue for Models. Although, it won't hit issues with NaN either. NaN !== NaN, so what. Bound case your application isn't supposed to work in either, what's the deal if you'll receive extra change event which won't brake the semantic of update. That's the question against which case you optimise - assignment of NaNs or normal numbers. I used to assign numbers and would appreciate it would be10x faster. Any other issues with NaN beside that one?

[gaperton]

Ok, I will create the test for jsper, when I will have some spare time. Not sure how can I do it with a framework patch, though. But may be, later, if I won't be lazy...

Or you can make local test as I did, and see the difference I reported. Just try to make model.set( 'attr', 5 ) 1 million times. I would, if I would be interested. In fact, I did it in test suite for my NestedTypes plugin :)

Or you can add performance test to the backbone test suite.

[megawac]

Just use raw git and a noconflict on jsperf to add your branch and
backbone 1.2. Im on my phone or I'd give an example
On May 15, 2015 3:26 PM, "Vlad Balin" [email protected] wrote:

Ok, I will create the test for jsper, when I will have some spare time.
Not sure how can I do it with a framework patch, though. But may be, later,
if I won't be lazy...

Or you can make local test as I did, and see the difference I reported.
Just try to make model.set( 'attr', 5 ) 1 million times.

Or you can add performance test to the backbone test suite.

—
Reply to this email directly or view it on GitHub
#3605 (comment).

[gaperton]

Ok, I will do it. Thanks.

[megawac]

@gaperton here's an example of how I've done it in the past http://jsperf.com/trigger-with-for-loop-vs-slice/6

Also you may want to fork underscore instead and change isEqual for simple values of a and b

[gaperton]

I'm not sure is it ok for underscore or not. For backbone it's ok since we need deep comparison in case if we have objects and arrays only, and we shouldn't care too much for other types in model's attributes. Because, everything you have in model attributes must be serializable somehow, so requirements are less restrictive here.

[gaperton]

Here.
http://jsperf.com/backbone-model-set-performance2

For some reason appears to be 10x faster in my local test (probably, that's because jsperf uses 'eval'). But still, jsperf shows 4x improvement in Chrome, and about 2x improvement in Safari.

In Firefox it's about 25% faster. Firefox is terrifically slow with backbone, I'm going to do something about it too.

[megawac]

An underscore patch to _.isEquals could be done like (untested)

_.isEqual = function(a, b) {
   var typeA = typeof a;
   return typeA != typeof b || typeA == 'object' || a === 0 || a != a ? eq(a, b) : a === b;
}

I'd be interested in seeing how such a change would affect the performance relative to your patch

[jridgewell]

With the other contributors, I'd love to see this change made in underscore so everyone can benefit.

[gaperton]

I wouldn't merge this isEqual to underscore. undescore's isEqual approach the comparison in much more paranoid way, and it's done intentionally. In contrast, this is optimized application-specific comparison for the special case of model.set changed attributes detection. So in this particular case it's not a problem if it will treat 0 and -0 as equal values (no difference between them in JSON and most type systems anyway), or it wil always fire change on assignment of NaN (it's not serializable either).

The same story about _.clone - there's a special case of cloning in model.set which may and worth to be optimized. Both optimizations has significant impact on Model.set performance, so... Please, decide. For me it doesn't make significant difference; we're not using backbonejs directly, so it's even easier for me to incorporate these optimizations to Backbone.NestedTypes plugin. I'm frankly surprised this small change caused such a discussion.

[megawac]

Did you see the amendment I suggested? Something like that could make it
into underscore
On May 16, 2015 4:58 PM, "Vlad Balin" [email protected] wrote:

I wouldn't merge this isEqual to underscore. undescore's isEqual approach
the comparison in much more paranoid way, and it's done intentionally. In
contrast, this is optimized application-specific comparison for the special
case of model.set changed attributes detection. So in this particular case
it's not a problem if it will treat 0 and -0 as equal values (no difference
between them in JSON and most type systems anyway), or it wil always fire
change on assignment of NaN (it's not serializable either).

The same story about _.clone - there's a special case of cloning in
model.set which may and worth to be optimized. Both optimizations has
significant impact on Model.set performance, so... Please, decide. For me
it doesn't make significant difference; we're not using backbonejs
directly, so it's even easier for me to incorporate these optimizations to
Backbone.NestedTypes plugin.

—
Reply to this email directly or view it on GitHub
#3605 (comment).

[gaperton]

Yes, I did.

( typeA != typeof b ) is obviously enough to decide that values are not equal. Also, we might optimize for nulls. So...

var typeA = typeof a;
if( typeA != typeof b ) return false;
if( typeA == 'object' ) return a && b ? eq( a, b ) : a === b;
return a === 0 || a != a ? eq(a, b) : a === b;

Looks good?

[gaperton]

Now, look at this :)
http://jsperf.com/underscore-isequal

[gaperton]

Ok, created pull request with heavily optimized _.isEqual(). It's freaking fast, 1,5-2 times faster than lodash implementation.

jashkenas/underscore#2179

[megawac]

typeA != typeof b

thats actually not sufficient for cases like new String('a') == 'a' ;)

[gaperton]

Yes, I have noticed it doesn't pass unit test. That's what I was afraid of.

But if you do something, it's better to do it right. So, take look at refined implementation. Performance is terrific, much better than lodash. And it still pass unit test.

jashkenas/underscore#2180

[gaperton]

Ok, I removed isEqual optimization from this pull request. There's only inlined optimized clone function.

Are you ready to merge? With my underscore patch applied, it gives 20% more performance in all browsers. In fact, there should be more, cause jsperf use 'eval'.

http://jsperf.com/backbone-model-set-performance2/2