Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Various component perf improvements #12286

Merged
merged 5 commits into from
Sep 4, 2015

Conversation

wycats
Copy link
Member

@wycats wycats commented Sep 3, 2015

Don't merge yet. I need to squash the commits. I'm making a PR so I can get line comments.

@@ -1,7 +1,7 @@
{
"features": {
"features-stripped-test": null,
"ember-htmlbars-component-generation": null,
"ember-htmlbars-component-generation": true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to be changed back to null before merging.

chancancode and others added 5 commits September 3, 2015 13:24
Previously, we tried to restrict which attributes we set eagerly, and
clean up the rest using unknownProperty. However, over time, making that
work correctly has gotten more and more complicated, and the cost of the
laziness is more than the benefits.

This commit goes back to an eager setting strategy, eliminates
unknownProperty and the reliance on _internalDidReceiveAttrs. It leaves
a setter interceptor for propagating sets to mutable cells.
Similar to `meta`, this commit restructures the scope away from
pervasive use of `Object.create` into a custom JavaScript object.

This has two benefits:

1. We only need a single parent pointer, rather than having every map
   inherit from its parent map.
2. v8 penalizes us for using `Object.create` by often assuming we are
   creating something more class-like and eventually blowing up methods
   that use scopes.

Eventually, this scope refactoring will probably replace some of the
hooks in HTMLBars.
@rwjblue rwjblue merged commit 43b39f8 into emberjs:master Sep 4, 2015
@rwjblue
Copy link
Member

rwjblue commented Sep 4, 2015

Cleaned up comments and merged in #12289.

@stefanpenner
Copy link
Member

are there some perf #'s? Im nervous to merge perf work, without actual metrics to go along

@rwjblue
Copy link
Member

rwjblue commented Sep 4, 2015

@stefanpenner - Sorry for not including them initially. I added the numbers to the description in #12289, but here is a snapshot:

1.12.1 - 105ms
1.13.9 - 194ms
2.0.1 - 163ms
2.1.0-beta.2 - 157ms
canary-a3fb7a1d - 173ms <- Just before this PR.
canary-eeb8ee90 - 138ms <- Including this PR.

@stefanpenner
Copy link
Member

Thanks

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

Successfully merging this pull request may close these issues.

4 participants