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

[Performance] Always render a consistent number of elements #112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

g-cassie
Copy link

TLDR: this modification improves performance when an ember-collection is scrolled for the first time after being rendered.

Forgive my loose terminology below - let me know if I am not making sense.

Assume a browser displays a vertically scrolling ember-collection that has 20 elements on screen and has a buffer value set to 5. Currently, when this collection first renders, it will "initial-render" 20 elements on screen and an additional 5 elements off the bottom of the screen. As soon as the user scrolls 5 elements down, it will have to "initial-render" an additional 5 elements off the top of the screen. The other 25 elements that are already rendered will be "recycle-rendered" which is much faster. The rendering of these 5 new elements can result in noticeable lag when the user first scrolls in a collection consisting of complex applications.

This PR changes the behaviour so initial 20 elements are rendered on screen and 10 are rendered off the bottom of the screen. As you scroll down the 10 elements off the bottom of the screen reduce down to 5 and 5 elements are moved off the top off the screen. So there will consistently be 30 elements rendered at all times.

In my use case, "initial render" of an element takes about ~20ms whereas "recycle-render" takes about ~3ms. That 5 x ~20ms when scrolling results in choppy scrolling. Scrolling is much smoother with this change in place. I'm not really sure how I can quantify this more empirically.

I am submitting this for feedback before I do the work to fix the broken unit tests. If there is support I will go ahead and get it ready for merge. It may also make sense to implement the corresponding behaviour when starting at the end of the collection - though obviously this is more of an edge case.

@g-cassie
Copy link
Author

After looking at this further I think you could get even better performance by also ensuring that ember-collection never renders less cells than it did in its previous render cycle. This is important in any list that has variable heights/widths such that you may have 20 elements on screen or 40. In this case you will get much smoother scroll if you always leave 40 cells rendered as there will be no "initial renders" after the ember-collection renders for the first time.

@ewwilson
Copy link

ewwilson commented Aug 8, 2016

This appears to have corrected some issues that I was experiencing so it would be nice to get this change in an official release.

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.

2 participants