Fix shaping #181
Fix shaping #181
Conversation
| @@ -55,7 +63,7 @@ function getTransitionByIntent(intent, isIntermediate) { | |||
| } | |||
|
|
|||
| // No-op. No need to create a new transition. | |||
| return new Transition(this); | |||
| return this.activeTransition || new Transition(this); | |||
There was a problem hiding this comment.
This change doesn't appear to be shaping related.
There was a problem hiding this comment.
Yea I don't know what is going on here. Was not intended. The even weirder thing is that it's only in the dist builds.
There was a problem hiding this comment.
You may already know this, but with some git magic you could fix this. If you...
- checked out a branch from master
- built and PR'd that
- came back here, reset the changes to dist
- rebased this PR off that other PR
...it would make few a cleaner diff here and more atomic commit history assuming the issue is what @stefanpenner suggested below
There was a problem hiding this comment.
@jmeas sounds reasonable, that way the diffs are cleaner when we have to do a bisect.
Unrelated to this PR, we should likely explore not committing built assets to the repo.
|
Ah. Then maybe some of the last changes forget to publish dist |
| @@ -19,6 +19,13 @@ function Router(_options) { | |||
| this.delegate = options.delegate || this.delegate; | |||
| this.triggerEvent = options.triggerEvent || this.triggerEvent; | |||
| this.log = options.log || this.log; | |||
| this.dslCallBacks = []; // NOTE: set by Ember | |||
There was a problem hiding this comment.
hmmm...Ember-specific code in route-recognizer? 😕
There was a problem hiding this comment.
this sounds like a short-term thing, ideally ember should track this state itself in ember-router instance, a sibling to router instance.
|
What's the reasoning for these updatez? |
|
@jmeas It's important that you do not mutate the shape of the object post creation time. For each property on the object a hidden class is allocated for it. You should allocate these upfront so that the engine can optimize. The
Now if you create a new
In comparison if you "slot" all of the fields up front you end up using the same hidden class for all instances instead of having an explosion of types. Below is an illustration of this, also you might find these articles interesting.
|
|
Ah, okay, @chadhietala . I know a bit about V8 optimizations in regards to shaping – thanks for the detailed reply! ✌️ |
|
@chadhietala - OK, I just updated dist, can you rebase and remove the changes to |
|
Ping @chadhietala |
|
Tis done. |
|
Thanks @chadhietala! |
Hmmm not really sure why the comments changed. Anyways, looks like there was much shape-shifting in this dude.
@krisselden or @stefanpenner can you review.