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

Growing snapshot heap on continuous data update with data containing children #4668

Closed
mkempf opened this issue Jan 16, 2025 · 4 comments
Closed
Labels
Possible Bug A possible bug that needs investigation

Comments

@mkempf
Copy link

mkempf commented Jan 16, 2025

Describe the bug
Continuously setting new data or slightly changed new data by using setData method increases the snapshot heap of the site. It only happens when the rows contain child rows.

Tabulator Info

  • 6.3.0

Working Example

To Reproduce

  1. Go to 'https://jsfiddle.net/b3rqk9f7/'
  2. Create a snapshot heap in browser dev tools
  3. Click on 'Update Data'
  4. Create a snapshot heap in browser dev tools
  5. Compare the snapshots in browser dev tools

There are detached div elements. Most probably from child cells / rows.
Repeating step 3 several times makes the growing memory more obvious.

Expected behavior
Snapshot heap is not continuously growing when pressing 'Update Data' several times

Screenshots
The following screenshot shows the detached elements in a comparison of two heap snapshots after clicking once on the 'Update Data' button. Be aware that the heap snapshot is done locally as in JSFiddle the snapshot heap of the iFrame seems not to be captured.

Image

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser chrome
  • Version 131.0.6778.265 (Official Build) (64-bit)

Additional context
We figured the leak only because of a table with lots of and nested children rows that are continuously updated. The site is currently running out of memory quite fast.

@mkempf mkempf added the Possible Bug A possible bug that needs investigation label Jan 16, 2025
@mkempf
Copy link
Author

mkempf commented Jan 16, 2025

Our current fix is in RowManager.

wipeChildren(rowComponents) {
	rowComponents.forEach(child => {
		if (child.getTreeChildren().length > 0) {
			this.wipeChildren(child.getTreeChildren());
		}
		child._getSelf().wipe();
	});
}

whereas the method is called in _wipeElements:

this.rows.forEach((row) => {
	this.wipeChildren(row.getComponent().getTreeChildren())
	row.wipe();
});

@mkempf
Copy link
Author

mkempf commented Jan 17, 2025

The HTML file used for the local reproduction is:

<!DOCTYPE html>
<html>
<head>
    <meta http-equiv="content-type" content="text/html; charset=UTF-8">
    <script type="text/javascript" src="https://unpkg.com/[email protected]/dist/js/tabulator.min.js"></script>
    <link rel="stylesheet" type="text/css" href="https://unpkg.com/[email protected]/dist/css/tabulator.min.css">
</head>
<body>
<div id="table">
</div>
<button id="button">
    Update Data
</button>


<script type="text/javascript">

    //Sample Data
    var tableData = [
        {
            name: 'Doe',
            _children: [
                {firstname: 'Jane', age: 3},
                {firstname: 'Joe', age: 4}],
        },
        {
            name: 'Bond', _children: [
                {firstname: 'James', age: 3},
                {firstname: 'Cloe', age: 4}],
        },
        {
            name: 'Callagher', _children: [
                {firstname: 'Jamie', age: 3},
                {firstname: 'Jimmy', age: 4}],
        },
    ];

    var newData = [
        {
            name: 'Doe',
            _children: [
                {firstname: 'Bobby', age: 3},
                {firstname: 'Peter', age: 4}],
        },
        {
            name: 'Bond', _children: [
                {firstname: 'Jessy', age: 3},
                {firstname: 'Jack', age: 4}],
        },
        {
            name: 'Callagher', _children: [
                {firstname: 'Robert', age: 3},
                {firstname: 'Hugh', age: 4}],
        },
    ];

    //Example Table
    var table = new Tabulator('#table', {
        data: tableData, //load data into table
        dataTree: true,
        dataTreeStartExpanded: true,
        height: 200, //enable the virtual DOM
        columns: [
            {title: 'Name', field: 'name', width: 200},
            {title: 'FirstName', field: 'firstname', width: 200},
            {title: 'Age', field: 'age', width: 200},
        ]
    });

    document.getElementById('button').addEventListener('click', updateData);

    var newDataFlag = true;

    function updateData() {
        if (newDataFlag) {
            table.setData(newData);
            newDataFlag = false;
        } else {
            table.setData(tableData);
            newDataFlag = true;
        }
    }

</script>
</body>
</html>

@olifolkerd
Copy link
Owner

Hey @mkempf

Thanks for the headsup, i have pushed a fix for this to the master branch and will include it in todays patch release.

Cheers

Oli :)

@mkempf
Copy link
Author

mkempf commented Jan 22, 2025

Hi @olifolkerd

thanks a lot for the fast fix and the patch release.
I was just able to check it with the simplified snippet and it looks fixed.

thanks again
Martin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Possible Bug A possible bug that needs investigation
Projects
None yet
Development

No branches or pull requests

2 participants