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

very slow when it comes to +20 lines with +10 columns ? #189

Closed
chaouiy opened this issue Feb 23, 2017 · 23 comments
Closed

very slow when it comes to +20 lines with +10 columns ? #189

chaouiy opened this issue Feb 23, 2017 · 23 comments

Comments

@chaouiy
Copy link

chaouiy commented Feb 23, 2017

it makes all the app slow ! any ideas of how to optimis performance ? is theire way to stop watching the source (one time binding) ?

@nnixaa
Copy link
Contributor

nnixaa commented Feb 24, 2017

@damnko would you like to take a look? Sounds like a challenge.

@damnko
Copy link
Contributor

damnko commented Feb 24, 2017

@nnixaa Yep it will indeed be a challenge.
#184 will also benefit from this solution so I would like to give priority to this issue.

@damnko
Copy link
Contributor

damnko commented Feb 24, 2017

@lexzhukov or @nnixaa do you remember what's the use case of this method?

ngOnChanges(changes): void {
setTimeout(() => this.renderCustomValue());
}

I wanted to focus first on all ngOnChanges to see if they can be removed/replaced with something less obtrusive, otherwise I will try to change the detectionStrategy but I noticed quite a difference when I removed that method.
Thanks

@nnixaa
Copy link
Contributor

nnixaa commented Feb 24, 2017

@damnko there is a feature when you can specify a renderer function to a column, which is not really a renderer, but just some function which accepts the rendered cell HTML into it and can change this HTML in the runtime (so that you can access cell HTML element in this function and do with it whatever you want, bypassing angular).
Basically it's more of a workaround rather than a good solution, we planned to replace this with a custom cell viewer component feature.

@sla-shi
Copy link

sla-shi commented Feb 24, 2017

@damnko thanks for the workaround, it really helped. I commented out the line 21 in view-cell.component.ts while waiting for the fix in the coming update.

@damnko
Copy link
Contributor

damnko commented Feb 24, 2017

@nnixaa thanks for sharing that info, I was also thinking why do we have to check for that on every ngOnChanges cycle? How does it feel if I start to work on a custom component to eventually render the cell content so we can get rid of this entirely?

@nnixaa
Copy link
Contributor

nnixaa commented Feb 24, 2017

@damnko this sounds perfectly right! 👍
I believe we can utilize the same approach as for the custom editor component.

@damnko
Copy link
Contributor

damnko commented Feb 24, 2017

@nnixaa yep I believe so, will keep you posted

@Deilan
Copy link
Contributor

Deilan commented Mar 22, 2017

How is it going?

@yangwen2
Copy link

yangwen2 commented Apr 10, 2017

I have the smart table component injected into the DOM conditionally via an *ngIf. When the smart table is part of the DOM anywhere in the screen, the mouseOver related UI updates drops from ~60fps to ~2fps. This test was carried out with ~150 rows x 5 columns in the smart table, with pagination disabled.

Looking further with Chrome timelines, it appears when smart table is rendered on the screen, a tremendous amount of processing is done in Angular 2's change detection logic. ApplicationRef_.tick() specifically.

Does this problem sound familiar for anyone else?

@kkapil24
Copy link

Is there update on this issue fix?

@Vingtoft
Copy link

Vingtoft commented May 4, 2017

Just spend hours configuring the table before I noticed this issue. The table is basically unusable until this is fixed.

Is there any progress?

@kyrodabase
Copy link

looks like critical issue :/ anyway - well done guys, loving this tool

@j-soliduslink
Copy link

we like the smart table a lot, would be great if you could fix the performance issue soon

@vlapo
Copy link

vlapo commented May 17, 2017

Same problem here.

@damnko @nnixaa I think I found performance problem:

let level = deepExtend({}, object);

getDeepFromObject is called over getSetting and deepExtend really slow searching. Why did you clone object? I think use lodash _.get function should be better solution.
There is also question why getSetting is called so much in endless loop.

@nnixaa
Copy link
Contributor

nnixaa commented May 18, 2017

Hey Guys, we are investigating the issue, and hopefully planning to release a fix soon.

@lexzhukov
Copy link
Contributor

This bug is fixed in 69e883c. Try to update to the latest 1.2.0 version of ng2-smart-table.

@vlapo
Copy link

vlapo commented May 25, 2017

Hi @lexzhukov, thnx for fix, but I have inifinite loop problem now:

angular.js:13236 TypeError: this.inputControl.valueChanges.skip is not a function
    at SelectFilterComponent.ngOnInit (table.umd.js:1615)
    at checkAndUpdateDirectiveInline (provider.ts:275)
    at checkAndUpdateNodeInline (view.ts:456)
    at checkAndUpdateNode (view.ts:417)
    at debugCheckAndUpdateNode (services.ts:235)
    at debugCheckDirectivesFn (services.ts:294)
    at Object.eval [as updateDirectives] (FilterComponent.html:3)
    at Object.debugUpdateDirectives [as updateDirectives] (services.ts:273)
    at checkAndUpdateView (view.ts:345)
    at callViewAction (view.ts:700)

I think it is something like this #229

@lexzhukov
Copy link
Contributor

@vlapo this is fixed. Try to install v1.2.1.

@vlapo
Copy link

vlapo commented May 25, 2017

@lexzhukov greate work!👍

I think performance is better :) Of course, I don't have so many columns but with 6 columns and 30 rows there is no lagging.

@lvqing1122
Copy link

lvqing1122 commented Jun 16, 2017

Hi @lexzhukov ,
After update to v1.2.1, I still have performance issue in IE11 when switch to page contains smart table. I have 9 columns and 30+ rows in smart table with pagination. 7 of columns are custom. But there is no performance issue in chrome.

@cyborgdoom
Copy link

We had this very same issue in another app it turns out the use of ngFor is the root cause in IE 11 you need to add trackBy: trackByFn ... trackByFn being a function in your component like so

trackByFn(index, item) {
return index; // or item.id
}

Once I add that into my component the page improved quite a bit from 2-3 minute load time down to 2-3 seconds.. Lots of records :)

@AbelValdez
Copy link

@cyborgdoom Where are you to add trackBy function if you don't have access to ngFor statement could you please explain me how to do it?

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