edcarroll / ng2-semantic-ui

Semantic UI Angular Integrations (no jQuery)
https://edcarroll.github.io/ng2-semantic-ui/
MIT License
615 stars 223 forks source link

Extreme slowness with sui-multi-select with alpha version, investigating #403

Open gabrieldoty opened 6 years ago

gabrieldoty commented 6 years ago

Please read https://github.com/edcarroll/ng2-semantic-ui/blob/master/CONTRIBUTING.md and search existing issues (both open and closed) prior to opening any new issue and ensure you follow the instructions therein.

Bug description:

When using the sui-multi-select and select more than one item, there is an extreme lag

Version of Angular, ng2-semantic-ui, and Semantic UI:

Angular: 6

ng2-semantic-ui: Alpha3

gabrieldoty commented 6 years ago

This seems to be related to larger datasets, not sure if worth attempting to optimize or recommending using a async service lookup to accommodate performance issues.

Slowness begins when data set > 100

edcarroll commented 6 years ago

Is this with any data or specifically more complex objects?

gabrieldoty commented 6 years ago

Even with the simple strings listed in the demo.. If you up the number of values in the options array to 100+ you can start seeing the performance decrease. If you get up to 200+ it is extremely slow.. In my project I was able to significantly improve the performance by employing the optionsLookup even without an actual async call to the API... i.e.


public searchCode = (searchString): Promise<any> => {
        return new Promise((resolve, reject) => {
            if (searchString.length < 2) {
                resolve([]);
            }
            else {
                resolve(
                    _.filter(this.items, (item) => {
                        return item.name.toLowerCase().indexOf(searchString.toLowerCase()) > -1;
                   });
                );
            }
        });
};
edcarroll commented 6 years ago

I wonder if changing the change detection type of the select component has an effect here? I'm really keen to move all of the components to the push strategy, though am as of right now unsure what effects that will have on the components...

gabrieldoty commented 6 years ago

This has been completely resolved for me by using the optionFilter instead of the optionLookup, not sure if worth pursuing. But the keyboard navigation bug fix on this PR is still open and needs merged

kerren commented 6 years ago

Hey @gabrieldoty

Could you please elaborate on that a bit? Did you kill the optionsLookup binding and implement the optionsFilter binding? In the docs it says that you cannot have both.

I've done both and they both seem extremely slow for me. My dataset is easily 500 entries, sometimes it can go up to 3000.

I did some Chrome performance analysis and I think a lot of the load comes down to rendering the templates. So I'm trying to remove all templates at the moment to see if that improves performance.

gdoty commented 6 years ago

@Kerren-Entrostat hey Kerren - I did switch to optionFilter only - but my data sets were still relatively small comparatively (50 items) if I bump that amount to 100+ I still get extreme slowness

kerren commented 6 years ago

Howzit :) Thanks so much for the response @gdoty

Do you have any suggestions for larger datasets? I'm hoping there's a way to speed up the sui-select otherwise I might need to use standard selects or something else? The problem is that there is sometimes a 5 second wait time from when you click to when it opens :/

hugo-dlb commented 6 years ago

In my case I had big performance issues (5-10 seconds freeze) when I had >50-100 items in my select. The reason was because I had many conditions like *ngIf="isAdmin()" which called a simple function that returns a boolean.

It looks like the select triggers a change detection round for each item in the select. Which basicly means if I have 10 times this ngIf condition in a component and I have 100 items in my select, it calls the ngIf condition 10 * 100 = 1000 times.

It is also interesting to note that those *ngIf conditions were in my root app.component.html component whereas my select was in a completely different component (at least 3-4 hierarchical level above).

kerren commented 6 years ago

Yea I agree, I think there are points where things become slow because we aren't binding the value directly so it's not running optimally. The problem that I have is that the performance lag is extremely noticeable but I swapped out the selects with a different library and it was lightning fast after that.

In my case, I wasn't calling any functions, I had precalculated values and plugged them into an ng-template. I think the template was slow because when I ran Chrome's performance tests on it there was a lot of rendering taking place. This is totally chilled when you're using a select for a couple of items (and 99% of the time I think that's the case for most people) but for big datasets, it doesn't work.

For people using it on bigger datasets, I'd recommend not using templates and using the labelName field only.

andremartin commented 5 years ago

Concerning the slowness of the select component, I did some investigation and I saw that Chrome was very busy with timer tasks. Looking at the code a bit, I saw that select spawns a lot of setTimeout tasks here: https://github.com/edcarroll/ng2-semantic-ui/blob/61ec6286f67d7064e42d534042b93e1de336e12c/src/modules/select/classes/select-base.ts#L305-L311 basically with every item that is in the list. Moving the outer foreach loop inside the setTimeout task solved the problem for me.

kerren commented 5 years ago

That makes a lot of sense! Is it not worth creating a PR with that fix? I don't think there are adverse effects by knocking the group into the next event loop vs knocking each individual entry out of the current event loop.

andremartin commented 5 years ago

@Kerren-Entrostat Done

zortx233 commented 5 years ago

I ran into this issue with larger data sets, and also with forms containing many dropdowns. Applying your change fixed it. Thanks!