olifolkerd / tabulator

Interactive Tables and Data Grids for JavaScript
http://tabulator.info
MIT License
6.75k stars 818 forks source link

Performance issue on release 4.2+ #1831

Closed adamxi closed 5 years ago

adamxi commented 5 years ago

Hej Oli,

So the issue seems rather subtle and night not be noticeable in all cases. However, i noticed that going from 4.1.5 to 4.2+, actually decreases rendering performance.

This is especially present when having a large table combined with a frozen column and side scrolling. While side scrolling, it's as if the header row is lagging behind while the frozen column is jittering along trying to render at the right position. It generally feels as if the whole table is rendered at 5 fps. The worst experience is of course in IE 11, however it is also slightly less noticeable in Chrome. Disabling the frozen column however, does make performance return almost to normal (compared with 4.1.5) in both browsers.

I can only imagine that this must be related to the virtual dom changes you have been working on. While this might not seem like a bug, the degraded user experience is serious enough that in terms of production quality, i cannot use the frozen column functionality.

Thanks

dcanning commented 5 years ago

I notice the header row lagging behind on rendering, I also have frozen columns on a wide table (not sure if that's what's meant by large by the OP). I have not tried unfreezing them to see how that affects performance of the rendering.

webdev23 commented 5 years ago

Yeah, there is a memory leak somewhere. As usual, using an external lib is an overhead. All fancy libraries like this one are just too much, there is silents errors handling everywhere, silents throw/catch:

The leaks starts as soon as the library is loaded, but no table pulled yet. Was using progressive loading, and of course it get worst with a lot of data .

About the php example code provided, it must be updated, it is throwing multiple warnings in php7, for beginners it can be nicer.

Lib is too heavy. Rolled my own alternative yesterday, and the room temperature decreased 4 degrees.

Thanks anyway, there is a lot to learn there.

Took a gif, from a tab opening, and this go up to full ram, then swapping, and machine freeze.

gif1551142167_optimized

olifolkerd commented 5 years ago

Hey @adamxi

Sorry to hear that, the issues you are experiencing there are not form the improvements to the virtual DOM , the virtual DOM only affects vertical scrolling.

But that is definitely a bug, it is actually related to another fix i put in that handles an edge case when tabbing through column header filters caused the headers to miss align, in this case it looks like the fix is fighting against itself which is slowing down the render performance. I will get a fix for that next weekend.

As for the memory leak, i am aware of a memory leak when new data is loaded via ajax using the replaceData or setData functions, this is on the list of things to fix in one of the upcoming patch releases in the next few weeks, but as it only really mounted up to significant issues if tables were refreshed with intervals of a minute or less i prioritised it lower than other updates.

Cheers

Oli :)

olifolkerd commented 5 years ago

Hey @webdev23

Looking at the example i would stay that your memory issues are happening because you have not set a height on your table therefore the virtual DOM is not being engaged, so Tabulator is attempting to load all of the table in one go, so it is consuming all your browsers memory. it is repeatedly mentioned throughout the Documentation that you MUST set a height on your table if you want it to render efficiently.

If you believe there are issues in the PHP example then i would be happy to update it, if you can point out any specific issues i can get those fixed asap.

The same goes for the memory leak you are experiencing, the table wont have any memory leak when sat idle as it doesn't do anything in an idle state, if im to stand a chance of isolating it i could do with a working example from yourself in the form of a JSFiddle or CodePen that shows how your tables are setup so i can understand where the issues lies in your tables specific configuration, although as i say looking at your example this is likely down to a lack of height being set on the table.

Big libraries do not always have memory leaks, in fact the main memory leak currently present (and unrelated to your issues which are likely down to mis-configuration of the table) is small and only occurs to any meaningful extent after many hours of constantly refreshing the table, was only introduced in the 4.1 update due to the removal of jQuery and a complete rebuild of the system, and was only brought to my attention a few weeks ago without enough time to get an update in for the 4.2 release.

There are absolutely no silent errors in Tabulator, it never silently absorbs an error, in most cases it will give a clear warning with an explanation of how to fix the setup issue, or in the case of a genuine issue it will fail with an error, there is no point to hiding errors it doesn't help anyone.

Also if you consider Tabulator too big, that is a bit of an odd sentiment, Tabulator is built in an entirely modular structure and while the default package comes with all the modules installed for ease, it is entirely possible to use the the core library which includes basic table render functions and then pull in only the modules you want to use separately, this is covered in detail in the Installation Guide

I hope that helps,

Cheers

Oli :)

adamxi commented 5 years ago

Thanks @olifolkerd, you're doing great work :) I noticed another thing which i believe is related. It doesn't always happen, but it is as if the first sidescroll you perform will cause a small hickup resulting in missing a few frames of horizontal movement. After this, sidescrolling performs as normal for a while at least. It doesn't happen on vertical scrolling. It feels mostly like a performance spike and it also wasn't present in 4.1.5 and occurs in Chrome and IE 11.

Rodbjartson commented 5 years ago

Totally agree with @olifolkerd, seems like you have not understood how to use tabulator @webdev23 I would call tabulator fairly lightweight now that it is using vanilla js and the modular approach

waruyama commented 5 years ago

Not sure if this is of any value, but I made the observation on Firefox that if I open the developer tools, the jitter during horizontal scrolling suddenly disappears.

olifolkerd commented 5 years ago

I have resolved the performance issues with the horizontal scrolling that were introduced in 4.2, these have been pushed to the master branch and will be included in sundays patch release.

The frozen columns are still stuttery on IE, im afraid there is very little i can do about this, IE simply dosn't have a lot of processing power and can barely cope with a lot of Tabulators features. While tabulator supports IE you will get a degraded experience due to the limitations of the browser.

Cheers

Oli :)