olifolkerd / tabulator

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

updateData function consumes all the available memory #4056

Closed ch3sn3k closed 1 year ago

ch3sn3k commented 1 year ago

Describe the bug I have a table with roughly 500 rows, where only 50 are visible. Few cells in the row are static, and few of the periodic changes are based on the data from the server. I am reading numeric (float) values and some strings. I am using updateData function to update selected rows based on the current data. This function is called every 2000ms. The problem is that after a while, the page consumes all available memory and the browser shuts down the tab.

Tabulator Info

Working Example https://jsfiddle.net/zag41ted/32/

To Reproduce Just run the example and watch the Task manager. You can adjust the setInterval delay to speed up the memory consumption.

Expected behavior Do not create new cell objects?

Screenshots image

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

olifolkerd commented 1 year ago

You need to use replaceData not updateData

ch3sn3k commented 1 year ago

@olifolkerd Thank you for the fast response. Still, I think this is some kind of issue. I am glad I can use your project, but the problem with your solution is a huge performance issue. In my case, I need to store the data for the table in a separate object. This is OK. But replaceData takes only the whole data, not just some rows. If I update the whole table with the replaceData function every 2000ms, it takes more than 1400ms to do the update. Can you advise me on how to solve my issue?

ch3sn3k commented 1 year ago

Just to provide some more info. I am updating only some values in the table, not the whole row. In my example above the values are either even or odd indexes.

olifolkerd commented 1 year ago

Hey @ch3sn3k thanks for the additional info.

Can I ask how your table is usable when using updateData at that rate. Calling that function resets scroll position which will have a very bad affect on usability for anyone using the table

ch3sn3k commented 1 year ago

Hello @olifolkerd, I do not understand your question, or I do not struggle with scroll issues. Even in the fiddle, you can see that if I scroll the table and call the updateData function periodically, the component behaves as I would expect. The scroll position remains, and data are just updated Thanks to your VirtualDom I am able to have all the rows loaded. The problem here is memory consumption. I am trying to overcome memory consumption with the following approach:

Is there any other way how to alter the table data?

jurafa commented 1 year ago

I think that the issue is in the Row.js in the function getCell(column). If updateData is called for rows that are not yet visible the condition

if(!this.initialized){
    this.generateCells();
}

always causes new cells to be generated. The cells keep references to detached HTML items that therefore cannot be garbage-collected causing memory-leaks.

If the condition is adjusted to check if the cells were already generated, the leak seems to be gone:

if(!this.initialized && this.cells.length === 0) {
    this.generateCells();
}
JohnL404 commented 1 year ago

@ch3sn3k We desperately need a fix to this. The solution provided in the PR by @jurafa does work, but we've had to pull down the Tabulator source and incorporate it into our project directly at this time.

ch3sn3k commented 1 year ago

@olifolkerd Do you think that the suggested solution from @jurafa is sufficient for this issue? As @JohnL404 requested I am not only one suffering with this issue. Can you consider the merge request?

olifolkerd commented 1 year ago

Hey, sorry for the delay in getting back to you on this one. Not ignoring things, just been dealing with a family emergency the last couple of weeks.

Normal service will resume soon, will look at getting a patch release out with these fixes in later this month.

Thanks for your patience

Oli