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

core-list is inefficient in data initialization #631

Closed
wiltzius opened this issue Jul 14, 2014 · 6 comments
Closed

core-list is inefficient in data initialization #631

wiltzius opened this issue Jul 14, 2014 · 6 comments
Labels

Comments

@wiltzius
Copy link

Two related problems: the bigger one is core-list will re-initialize all of its elements if you add data to the list, even if that new data is far offscreen and isn't physically present.

e.g. if you have a core-list with data bound to a 100 element array, and then call .push({}) on that array, the core-list viewport will be re-initialized which is very expensive (~300ms in the example I'm looking at on a Nexus 5). For true infinite scroll we want to be able to load data from the network (or elsewhere) and add it to the list before its needed.

The other problem is that this re-initialization is itself so expensive, largely because every one of the template instances is individually laid-out -- so there is a requestAnimationFrame called for each, a layout call, and some other overhead that's done for every single element; a form of layout thrashing. It would be much more efficient to batch this somehow...

@wiltzius
Copy link
Author

It's because core-list.initialize() is bound to core-list.data changing, and core-list.initialize re-does everything -- core-list.initializeViewport, core-list.initializeItems, and core-list.initializeData (which is undefined as far as I can tell?)

This should be more fine-grained-- only changes to the viewport should trigger viewport re-initialization, and only changes to the physical data (including changes to the physical data count triggered by viewport re-init) should re-compute physical data. As far as I can tell nothing needs to be done on core-list.data changing.

The physical item layout is the expensive repeated layouts I was referring to earlier, by the way. Each template child is individually laid-out. I don't know if the templating system is flexible enough for this, but it would be nice to batch that.

@sorvell
Copy link
Contributor

sorvell commented Jul 17, 2014

if you have a core-list with data bound to a 100 element array, and then call .push({}) on that array, the core-list viewport will be re-initialized

This is definitely a bug and will be fixed soon.

Each template child is individually laid-out.

I'll have to look into that before I can comment as to why.

@wiltzius
Copy link
Author

I realize my bug description is a little dense, so if you want an example
page or want to talk about this I'm happy to

On Thu, Jul 17, 2014 at 11:11 AM, Steve Orvell notifications@github.com
wrote:

if you have a core-list with data bound to a 100 element array, and then
call .push({}) on that array, the core-list viewport will be re-initialized

This is definitely a bug and will be fixed soon.

Each template child is individually laid-out.

I'll have to look into that before I can comment as to why.


Reply to this email directly or view it on GitHub
#631 (comment).

@sorvell
Copy link
Contributor

sorvell commented Jul 17, 2014

The update issue is totally clear.

It's not clear why individual elements are laying out. If they are custom elements that do something like measure themselves when attached, that would explain it, but otherwise, it's not expected.

@wiltzius
Copy link
Author

Hmmm, the example I was using does use custom elements. I will check if
they're doing something when attached or ready; I think they may be
updating their model when created which might explain it (and maybe is
avoidable if I time it better)?

On Thu, Jul 17, 2014 at 11:17 AM, Steve Orvell notifications@github.com
wrote:

The update issue is totally clear.

It's not clear why individual elements are laying out. If they are custom
elements that do something like measure themselves when attached, that
would explain it, but otherwise, it's not expected.


Reply to this email directly or view it on GitHub
#631 (comment).

@tjsavage
Copy link
Contributor

tjsavage commented Aug 1, 2014

auto-moving this issue to googlearchive/core-list#9 and closing this one

@tjsavage tjsavage closed this as completed Aug 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants