Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

perf(*): use Object.create instead of creating temporary constructors #10058

Closed
wants to merge 1 commit into from

Conversation

jbedard
Copy link
Contributor

@jbedard jbedard commented Nov 14, 2014

For inherit: http://jsperf.com/inherit-extend-vs-ocreate

For constructing an object without calling the constructor: http://jsperf.com/create-constructor/2
Note that caching the constructor is actually faster but I'd rather avoid the extra logic and caching. Maybe that will be done in #9772 though.

@gkalpak
Copy link
Member

gkalpak commented Nov 14, 2014

Note that the slowing factor is having to call extend twice.
If (for whatever reason) one would not want to use Object.create, replacing

return extend(Object.create(parent), extra);

with:

var constr = function() {};
constr.prototype = parent;
return extend(new constr(), extra);

is almost equally fast.

Of course, Object.create() is consicer and preferrable.
I am just mentioning this in case it has a noticable perf impact and we want to backport it to 1.2.x.

@jbedard
Copy link
Contributor Author

jbedard commented Nov 14, 2014

I find creating temp objects like that quite ugly. It also seems to perform much worse (see the second benchmark) unless you cache the constructor which makes it even uglier.

@gkalpak
Copy link
Member

gkalpak commented Nov 14, 2014

The second benchmark does not prove that it is much slower than Object.create. They are very close (in comparison to caching the contructor), but the difference is negligible given extend will take up most of the time: http://jsperf.com/inherit-extend-vs-ocreate/4

In any case, as I already said, there is no reason to not use Object.create() in 1.3.x.
I olny mentioned the (more verbose) alternative, in case we want to port it back to 1.2.x (due to Object.create's not being available in IE8).

@caitp
Copy link
Contributor

caitp commented Nov 14, 2014

microbenchmarks don't mean a whole lot, but I feel like we are gaining a bit of simplicity using Object.create instead, so it seems like a worthwhile refactoring

@caitp
Copy link
Contributor

caitp commented Nov 14, 2014

@petebacondarwin can you rubber stamp this?

@caitp caitp added this to the 1.3.4 milestone Nov 14, 2014
@petebacondarwin
Copy link
Contributor

LGTM

@PatrickJS
Copy link
Member

Object.setPrototypeOf is a thing. When will angular 1.x update the codebase to es6? 1.4 or 1.5?
http://maximilianhoffmann.com/posts/object-based-javascript-in-es6

@lgalfaso
Copy link
Contributor

At the moment there are no plans to change angular 1.x code to ES6

Object.setPrototypeOf is a thing. When will angular 1.x update the
codebase to es6? 1.4 or 1.5?

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

Successfully merging this pull request may close these issues.

8 participants