TanStack / table

🤖 Headless UI for building powerful tables & datagrids for TS/JS - React-Table, Vue-Table, Solid-Table, Svelte-Table
https://tanstack.com/table
MIT License
24.51k stars 3.03k forks source link

fix: reduce memory leak by remove memo function #5514

Open KonghaYao opened 2 months ago

KonghaYao commented 2 months ago

I recently used table-core to complete a virtual table. I rendered it with a dataset of 1000*1000, and found that the memory usage of table-core was unreasonably high. Since no one had fixed this issue(#5352), I attempted to debug it myself. Upon investigation, I discovered that memo was causing memory leaks in many places. This prevented JavaScript from reclaiming a massive number of rows and cells. Once I removed them, memory could be reclaimed again.

https://github.com/TanStack/table/assets/68177907/4ed24f52-c93f-48c4-bead-3d5d93ba5b80

KevinVandy commented 2 months ago

removing memo in these places actually has a lot of performance implications here. If there is a memory problem, we should try to address that, but not at the expense of losing all of the render performance optimizations.

Also, this PR changes a lot of the code style for no reason, as far as I can tell. Why function() with this? Was that part of the memory leak fix, or just personal preference?

KonghaYao commented 2 months ago

I'm very sorry. At first, I thought that creating a loop reference to itself during object creation would cause a memory leak. After removing the memo, I forgot to handle it. Now I have tested it and found that the reason for the memory leak was the use of memo. The new commit has removed the use of function and style.

KonghaYao commented 2 months ago

Deleting the memo did indeed cause a high memory usage issue. I will investigate the reason for the memory leak caused by the memo.

KonghaYao commented 2 months ago

In the example /examples/react/virtualized-columns, horizontal scrolling does not cause any memory issues, but vertical scrolling leads to memory leaks. The function getVisibleCells is called, which accesses several functions that I modified. However, the specific location and trigger of the memory leak have not yet been identified.

KonghaYao commented 2 months ago

image This is not a caching issue caused by memo. I deleted all references within the cell instance on the original branch and did not find any obvious memory leaks. However, when I restored cell.column, the memory leak was very severe. It seems that the strong memory leak was caused by the circular reference between cell and column.

Stax124 commented 2 weeks ago

I can verify that this bug is present. We have a table solution written in Vue and swapping pages or refreshing data in any way leads to insane memory leaks - sometimes even causes page crashes as the heap cannot take it anymore.