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

Select processing of large groups of selects takes too long #436

Open
1 task done
Westbrook opened this issue Apr 30, 2017 · 9 comments
Open
1 task done

Select processing of large groups of selects takes too long #436

Westbrook opened this issue Apr 30, 2017 · 9 comments

Comments

@Westbrook
Copy link
Contributor

Description

Not sure if this is ultimately an iron-list, or an array-selector, or a further up, issue, but selecting large groups of content, whether by the technique outlined in #124 or in #361 takes an inordinate amount of time. Explicitly, I'm seeing ~10s processing times for lists of 5000 items. I understand that's a lot of items, BUT that still seems crazy long.

My research points to this coming from the way selectItem applied code from array-selector When you follow that into the array-selector, you can see that it's pushing into it's selected array the subsequent property effects of which seems to be the cause of the processing delay.

Expected outcome

Selecting a large amount of items takes an virtually indistinguishably longer time than selecting one or a few.

Actual outcome

Selecting 5000 items at a time causes a ~10s block on the main thread.

Live Demo

https://jsbin.com/tobosaleki/1/edit?html,console,output

Steps to reproduce

  1. Visit the JS Bin demo
  2. Click 'select all'
  3. Wait, see the window.performance based timing display on completion
  4. Click 'select none'
  5. Wait, a really small amount of time, see the window.performance based timing display on completion
  6. Be in awe at the difference

Browsers Affected

  • All
@Westbrook Westbrook changed the title Select processing of large groups of selects take too long Select processing of large groups of selects takes too long Apr 30, 2017
@Westbrook
Copy link
Contributor Author

@keanulee any thoughts on how I might approach this problem in a way that would be enticing to the team?

@keanulee
Copy link
Contributor

keanulee commented May 9, 2017

Not super familiar with array-selector or how the selection logic actually works, but with iron-list#2.0-preview, there's a new selectIndex method that will let you more efficiently select multiple items (see https://github.com/PolymerElements/iron-list/tree/2.0-preview#changes-in-v2). You can use iron-list#2.0-preview with Polymer 1.0 too (since it's "hybrid").

@Westbrook
Copy link
Contributor Author

That is exciting, I'll have to look into this more deeply. However, a quick pass on testing that out yields only slightly better results why working with the "hybrid" version:

https://jsbin.com/buhewosuli/edit?html,console,output

And, while the test in this if is esoteric (to say the least), the comments seem to point out that in V1 I still can't get around using the underlying array-selector method from which the issue seems to originate:

https://github.com/PolymerElements/iron-list/blob/2.0-preview/iron-list.html#L1528

Am I reading that right? What's more, it seems that a full v2.0 demo actually has worse performance here:

https://jsbin.com/tanuzodire/1/edit?html,console,output

Still learning my polygit with v2, so I might have messed something up in that JS Bin, let me know if you see anything particularly weird there. I definitely have the move to Polymer 2.0 on my to-do list, but it's still too much of a moving target (especially re: these sorts of issues) to sell time on in to my office.

@keanulee
Copy link
Contributor

In that case maybe it's worthwhile to add a selectItems method, but I don't want to add it to 1.0/master since that means it'll need to be ported to 2.0 (where Collection doesn't exist). Feel free to make a 2.0 version, but for it to be merged it'll have to be hybrid (work with Polymer 1 and 2).

@Westbrook
Copy link
Contributor Author

I'll have to catch up on some of my 2.0-isms anyways, so here's as good a time as any. Let me see what I can do.

@Westbrook
Copy link
Contributor Author

@keanulee I've got a branch coming together for this that's gonna rely on some PRs to the various Polymer branches, but I wanted to share the progress so far:

V1: https://jsbin.com/fiqanewevo/1/edit?html,console,output
V2: https://jsbin.com/wolilumeni/4/edit?html,console,output

In that the PR for this would rely on the referenced changed to the Polymer library, would it be better if I wait till those are accepted or should I go ahead and put in the iron-list PR now?

@keanulee
Copy link
Contributor

You could put it in a PR now, but I don't think anyone would look at them until the Polymer ones are merged.

@dman777
Copy link

dman777 commented Apr 26, 2019

Any follow ups to this? I am hitting the same issue.

@Westbrook
Copy link
Contributor Author

Sorry @dman777 I never got around to doing this work. The upward facing pressure on the Polymer library did outline a path forward, however. In conjunction with this issue, the conversation in #457 there should be more than enough information to pick up where I left off.

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

No branches or pull requests

3 participants