emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.47k stars 4.21k forks source link

Poor performance of {{#each}} when views and/or components are involved. #10261

Closed benlesh closed 8 years ago

benlesh commented 9 years ago

I have an issue where a table I'm creating via {{#each}} can take up to 7 seconds to render initially, and/or again if it's sorted.

There are only about 200 rows. That's a requirement. But each row has a dozen components, some of which have nested components. I've discovered that for each one of those components a parseHTML is done, multiplied by the number of rows. The performance is really bad. If I'm merely updating the objects in the rows after that, the performance is quite good, but on a sort it takes another 7 seconds. Some of that 7 seconds seems to be GC from whatever is going on internally as well.

I've already discussed this issue with @ebryn, but I wanted to make sure the issue was recorded on GitHub so more eyes can land on it and track it's progress.

I'm facing down rewrites and refactors to work around Ember's performance with arrays with ListView. Which isn't an ideal solution for my requirements.

If there are any in-flight efforts I can help with, I am very happy to do so as resolving this performance issue is at the top of my list.

cc @jayphelps

jayphelps commented 9 years ago

Just to clarify--we're aware of ember list-view but it does not suit our needs and is more of a workaround to the underlying poor performance of CollectionView, which is badly in need of a rewrite (came from SproutCore). It destroys all views when the underlying content changes, even if it's just a sort, instead of just moving the views around which is one of the reasons it performs so poorly.

stefanpenner commented 9 years ago

@ebryn magicked collection view work may be interesting here. Any news on it Erik?

jesenko commented 9 years ago

We have also encountered this perf issue and are currently looking into possible workarounds. See here for details and jsbin demonstrating the issue.

stefanpenner commented 9 years ago

@jesenko although not ready yet: https://github.com/emberjs/ember.js/pull/10501 is our big effort that aims to address this.

Also yes, this isn't the work-around you are looking for. I may have time later this week to dig into your examples, if noone has by this weekend feel free to pester me.

jesenko commented 9 years ago

@stefanpenner great, I was hoping that Glimmer would also improve initial render times. Waiting for Glimmer is also one of workarounds that we are considering. In version used in dbmonster app however the performance issue is still there - I forked dbmonster repo and added the problematic component to each row; initial rendering time for 100 rows went from ~300ms to ~1000ms.

stefanpenner commented 9 years ago

@tomdale @wycats some more tests cases for you two^

jesenko commented 9 years ago

@stefanpenner The safest work-around would be probably to just use raw Handlebars templates with POJO objects as context for rendering cells. https://github.com/ember-cli/ember-cli/issues/733 would make this easier, however I guess it was not implemented. I will look into it, if I will be able to come up with something worth sharing I will extract it as an add-on.

stefanpenner commented 9 years ago

@jesenko sounds good :)

sandstrom commented 9 years ago

We've rewritten large parts of our app towards the Ember 2.0 philosophy of components. Overall it's nice, I like the direction Ember is taking with 2.0.

However, one view has gotten quite a bit slower. Basically it's a long list of components and nested components within them. Jesenko's fork seems like a good proxy for our case too.

It is also my understanding that glimmer will primarily affect updates, not initial rendering.

jayphelps commented 9 years ago

As Glimmer approaches "all tests passing" phase, re pinging @tomdale and @wycats, since this is still an issue and haven't heard from any core member (I've asked 3) whether this is addressable.

rwjblue commented 9 years ago

@jayphelps - I would like to poke around at this, can you provide a reduced idiomatic example that demonstrates the issue here? The JSBin above uses createElement (I'm sure for ease of JSBin'ing) which is not idiomatic and doesn't have the ability to take advantage of rerendering improvements.

rwjblue commented 9 years ago

Would printing a table of 200 rows with 12 cells in each (where each cell is a component) be representative of the reported issue?

sandstrom commented 9 years ago

@rwjblue Here is an example which should be representative for us. Don't know if this is similar to @jayphelps case, but I guess it might be.

https://github.com/sandstrom/initial-render-components

jayphelps commented 9 years ago

@rwjblue Agreed, we're not doing anything shady IRL. The key is multilevel nesting components in loops. @sandstorm's example seems like a great representation.

stefanpenner commented 9 years ago

One thing I noticed was the extra binding in the each from data source to content. Can often sync after the initial render phase, causing the nested look to essentially be a re render over just a nice straight though. Render. Other issues exist, like the shear amplification of existing costs. Post glimmer I have plans to implement the ideas from my ember conf talk. As it will make it easier to measure

benlesh commented 9 years ago

@stefanpenner found some perf issues in ember-nf-graph that I'm not sure I would have caught. That said, I've seen this same issue when graphs weren't at all present. Even once the graphs are fixed, I don't think in my example of 60 graph components in an each will be faster than 5-6 seconds in 1.11.1

stefanpenner commented 9 years ago

Yes. Buts phrase non the less, as I am uninterested in doing Perf work on soon to be legacy work.

Glimmer work isn't a quick fix for all problems. It makes it possible for people to write idiomatic code that is dramatically faster in some common pathological cases.

It also lays the foundation for many future improvements.

The things I caught in nf-graph are part of the problem. When they are fixed and running on glimmer I'll gladly do another pass. We can use that example as a way to continue performance work.

I suspect the result will be a combination of nf-graph fixes and ember fixes.

sandstrom commented 9 years ago

@stefanpenner I've put together an example (using the glimmer branch). Perhaps it can be useful. It's similar to @blesh example, except it's (a) 100% ember code (no external components) and (b) runs on glimmer.

https://github.com/sandstrom/initial-render-components

benlesh commented 9 years ago

When they are fixed and running on glimmer

@stefanpenner I'm all for this... I don't care at all what version of what my projects need to be on. I simply care that the components I've worked hard to craft are usable in lists without completely destroying the user's experience. If there's any way at all I can assist in that, I'm game, 8 hours a day, for however many days it takes. I'm tasked with making our app performant, and this one issue rears it's head a lot in our apps.

stefanpenner commented 9 years ago

I'm game, 8 hours a day, for however many days it takes. I'm tasked with making our app performant, and this one issue rears it's head a lot in our apps.

Awesome. I unfortunately wont have any consistent time till I'm back from my vacation. My suggestion in the interim, is to try and break down the performance problem into smaller chunks. For example, attempting to isolate the nested #each initial render with 1 basic component with 1 basic binding.

This approach takes time, but it will allow the output to become more predictable, and actionable then when making incremental fixes you will hopefully notice reduction in specific method calls.

Hopefully several smaller examples can be derived that isolate the problem from different perspectives.

One thing to be aware of, is that when seeing many calls to X, one needs to be sure that X should actually be called that many times. It can easily be the case, that that noise all stems from several small but isolated issues.

And finally, I would be sure to do the tests and examples with the glimmer code. Otherwise we may be targeting entirely different problems.

I am glad you have some resources to help.

chadhietala commented 9 years ago

@blesh Have you tried @ebryn's metal-components? While I'm not sure what the compatibility is with glimmer due to the internals it touches, this was preliminary work to prove out that UI backing objects need to be much less expensive to create and destroy. This is more aligned with @stefanpenner's first bullet point from his EmberConf talk, which talked about doing less stuff.

benlesh commented 9 years ago

@stefanpenner or anyone else... where can I find documentation or an overview of the latest rendering architecture? I'd like to read a primer.

mixonic commented 9 years ago

@blesh some references Robert and I often use:

benlesh commented 9 years ago

@mixonic thanks.

stefanpenner commented 9 years ago

@blesh source code is typically a good start :trollface:

benlesh commented 9 years ago

@stefanpenner ... if only Ember core was developed with an opinionated framework I'd know where to look.

fivetanley commented 9 years ago

@stefanpenner ... if only Ember core was developed with an opinionated framework I'd know where to look.

the sickest of burns

benlesh commented 9 years ago

heh... jerks.

sandstrom commented 9 years ago

@blesh @stefanpenner just curious if either of you found anything when looking at this? [friendly prod]

stefanpenner commented 9 years ago

@sandstrom unfortunately, I have been focused on ember-cli lately. I hope to circle around soon (unless someone beats me to it)

jayphelps commented 9 years ago

@sandstrom There's definitely been progress on various fronts both internally in Ember and external efforts. The current biggest take away is that you can get major perf boosts in canary if you strictly follow the latest and greatest conventions (some of which are only available in canary) @blesh has been keeping his example repo up to date: https://github.com/blesh/is-ember-really-fast-yet

And the running example comparisons:

Ember: http://blesh.github.io/is-ember-really-fast-yet/dist/ React: http://blesh.github.io/is-react-really-fast-yet/

@ebryn made a very notable change here

mixonic commented 9 years ago

We also track performance by benchmarking with the https://github.com/eviltrout/ember-performance repo. I encourage you to contribute use-cases to that project.

sandstrom commented 9 years ago

I've created a pretty plain example running on canary, basically a bunch of small components. @jayphelps Is there anything from canary that you know would speed it up?

sandstrom commented 9 years ago

@stefanpenner sorry to prod, feel free to ignore :) just wondering if you have any thoughts on component performance.

We're in the process of migrating to from 1.7 to 1.13, and doing so we're leaning heavier on components (moving away from controllers and views). The vast majority of changes in 1.13 are certainly for the better (it's been an awesome Ember year so far!), some of our component-heavy parts have gotten quite a bit slower.

I've but together a generic example, if you've got time to look at it that would be much appreciated: https://github.com/sandstrom/initial-render-components

stefanpenner commented 9 years ago

@sandstrom haven't had a chance to work on it yet, but have many ideas. As soon as I can dedicate some time to this instead of ember-cli I absolutely will.

That being said, I have been able to spend time removing IE8 related cruft and deprecated code Which is helping reduce the complexity in some of the offending code. The goal of this work is to make it easier to implement my Perf ideas.

I also know that the rest of the team also shares the same goals.

Anyways. Thanks for the demo app, I will use it once I can dedicate my time to it.

sandstrom commented 9 years ago

@stefanpenner Sounds great!

I feel bad for asking — ideally I should dig in and try to improve performance myself. Unfortunately I'm not well-versed in perf optimizations + working all awake hours on a small startup.

Happy that IE8 was dropped. Looking forward to see the perf magic that you'll conjure! :honey_pot: :fireworks:

rwjblue commented 8 years ago

A ton of work has been done here (and much is still ongoing). See https://github.com/emberjs/ember.js/issues/11502 for just one example. Ember 2.2+ performance is on par with 1.11/1.12, and the next version of the rendering engine (Glimmer v2) which is currently underway should continue to cut down initial render times in these scenarios.

I'm going to close this issue for now, as there is little ongoing conversation here and there is a clear path forward that many folks are rallying behind and working on...