Allow opting out of enumerable-safety#9702
Conversation
…pt-out of the enumerable safety. Unfortunately at this time there is not way to create fast non-enumerable properties. Historically ember has this to allow people to merge, extend and JSON.stringify. Unfortunately some things like Ember.Views should do not need this but still suffer from the pretty hefty overhead. This PR Reduces the cost of view allocation by 300% -> 400%. Micro bench (creating 10,000 views): before: 120ms after: 35ms
stefanpenner
force-pushed
the
__defineNonEnumerable
branch
from
b83bd3c to
40d0b0d
Compare
There was a problem hiding this comment.
Is this comment needed? I thought perhaps it was from a refactor/test and might be able to be removed now....
There was a problem hiding this comment.
So sorry, better leave it in then. 😉
There was a problem hiding this comment.
What is really going on here? Why not use the commented line instead of defineProp? Mixins don't care if they have extra enumerable props, yes? This line is responsible for a very large number of defineProp calls in my testing.
|
Seems pretty darn good. |
There was a problem hiding this comment.
this isn't included in my above #'s but it helps
There was a problem hiding this comment.
I believe that this was already merged in https://github.com/emberjs/ember.js/pull/9701/files.
|
@stefanpenner status? |
|
likely a final set of benchmarks, maybe expand it to other objects. |
|
Let's merge this bad boy! |
|
as |
In the meeting we discussed this as a full canary cycle change, if @wycats is already with it being rushed to beta, thats ok. |
|
Ah, did not realize that was the consensus. |
|
@eviltrout got time to run this through your benchmarks? |
Allow opting out of enumerable-safety
7 participants
Introduce a
__defineNonEnumerablehook. This allows parts of Ember to opt-out of enumerable safety (marking internal properties as non-enumerable).Unfortunately at this time there is no way to create fast, non-enumerable properties. Historically Ember has this to allow people to
merge,extendandJSON.stringifyEmber objects, which requires being careful about enumerability.Unfortunately, classes like
Ember.Viewshould do not be used in these contexts, but still suffer from the pretty hefty overhead.This PR Reduces the cost of view allocation by 300% -> 400%.
Micro bench (creating 10,000 views):
the 25ms of the remaining is mostly funkyness we do in Ember.View and CoreView ourselves. We can be careful and clean lots of that up still.