fix(ng): in angular.copy do not call hasOwnProperty on objects with not ... #10116

mohsen1 · 2014-11-18T23:20:34Z

...prototype

angular.copy can be called with an object that has no prototype. Call
Object.prototype.hasOwnProperty instead of object.hasOwnProperty to
safely check if object has a property

… prototype angular.copy can be called with an object that has no prototype. Call Object.prototype.hasOwnProperty instead of object.hasOwnProperty to safely check if object has a property

lgalfaso · 2014-11-18T23:27:39Z

Would it be possible to get some numbers to know if this will have some
performance implications?

mohsen1 · 2014-11-19T00:04:21Z

@IgorMinar see this JSPerf http://jsperf.com/hasownproperty-context

gkalpak · 2014-11-19T00:13:46Z

Calling it on the same item might allow browser optimizations that hide the actual performance.

In practice, it will be called on many different objects (and will correspond to a small percentage of the overall processing time, so I don't expect it to have any significant impact.

Tried with 1,000,000 different objects and the results are equal: http://jsperf.com/instance-vs-prototype-hasownproperty
At 100,000 the instance method was faster (by a varying percentage of 10 - 20).

gkalpak · 2014-11-19T01:27:06Z

Here is another jsperf featuring 2 alternative versions of copy() (along with the original one):

Using hasOwnProperty.call().
Using Object.keys() (since IE8 is no longer an issue).

http://jsperf.com/angularjs-copy-with-hasownproperty-call

lgalfaso · 2014-11-19T20:56:17Z

I would not be happy to have a 10% performance hit...

@petebacondarwin @caitp WDYT?

mohsen1 · 2014-11-19T21:12:30Z

There isn't a performance difference really. @gkalpak tests are not using JSPerf right. JSPerf does iterations itself. If you put a huge loop in your snippet the result might become less accurate.

I've added .apply for comparison to the JSPerf. They are all the same
http://jsperf.com/hasownproperty-context/2

googlebot · 2014-11-19T21:12:32Z

Thanks for your pull request.

It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits.

Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name.

gkalpak · 2014-11-19T21:58:46Z

I don't have a loop in my last jsperf: http://jsperf.com/angularjs-copy-with-hasownproperty-call

I implemented the whole copy() function to get more "realistic" results.
(But I am not a jsperf guy, so there is a good chance I might be doing something wrong).

mary-poppins · 2014-11-19T22:09:35Z