Big Memory Leak / Retained Memory

[Fabbok1x]

When unmounting, and mounting the MUIDataTable, using
https://codesandbox.io/embed/q8w3230qpj?autoresize=1&hidenavigation=1
about 1.3 mb of memory is not free'd. This is problematic despite one-page apps and multiple mounts and unmounts.

This image illustrates the issue:
https://i.imgur.com/zLAdUbi.png

This is a look at the FiberNodes:
https://i.imgur.com/UwZelkZ.png

Each peak is one mount (with a previous unmount).

tableRef seems to play a role in the memory retaining.

[Fabbok1x]

Could render it to the core of the issue.
The core of this happening is inside the render return function of MUIDataTable,
to be precise this snippet causes the memory to not get freed.

      {selectedRows.data.length ? (
         <TableToolbarSelect
           options={this.options}
           selectedRows={selectedRows}
           onRowsDelete={this.selectRowDelete}
           displayData={displayData}
           selectRowUpdate={this.selectRowUpdate}
         />
       ) : (
         showToolbar && (
           <TableToolbar
             columns={columns}
             displayData={displayData}
             data={data}
             filterData={filterData}
             filterList={filterList}
             filterUpdate={this.filterUpdate}
             options={this.options}
             resetFilters={this.resetFilters}
             searchText={searchText}
             searchTextUpdate={this.searchTextUpdate}
             
             title={title}
             toggleViewColumn={this.toggleViewColumn}
             setTableAction={this.setTableAction}
           />
         )
       )}

Looking further into it.

[gabrielliwerant]

Hello @Fabbok1x, thank you for looking into this.

What action are you taking to cause the table to unmount and remount? Your example just looks like a static version of the default table, so I'm unclear on the exact steps you're using.

[Fabbok1x]

I'm loading some components dynamically like this:
let { default: Test } = await import(../../components/${loadComponentName});
then rendering

I may write a small codesandbox to illustrate the issue. I'm not yet exactly sure what causes this but the above snippet certainly has something to do with this, possibly also this line

ref={this.tableContent}

in the same render function, a change to
ref={(ref) => ref = this.tableContent}
may help contribute to the freeing of the memory.

Additionally these binds in the TableBody
onClick={this.handleRowClick.bind(null, row, { rowIndex, dataIndex })}
seem a bit suspicious to me, but it's not yet enough to verify completely.

Only thing I came to verify so far is the TableToolbar definetly playing a role in this.

[Fabbok1x]

Came to verify the issue to some extend.

So one thing that is safe to say is that something within styled.js is causing the RAM leak.

Modding TableToolbar to not use the styled HOC will definetly resolve the leak.

Didn't have enough time to dig into what's exactly causing this within styled, however disabling the HOC and removing the classes does seem to do a sufficent job for me at the moment.

Result (after about 4 remounts):
https://i.imgur.com/rLWWIQM.png
vs.
https://i.imgur.com/zLAdUbi.png

In case someone wants to have a look at styled:

import React from 'react';
import PropTypes from 'prop-types';
import merge from 'lodash.merge';
import { withStyles } from '@material-ui/core/styles';

/*
 *  Material-UI does not yet support ability to grab props within style()
 *  https://github.com/mui-org/material-ui/issues/7633
 *
 *  This is a workaround provided from the thread
 */

const styles = (theme, props, style) => {
  return typeof style === 'function' ? style(theme, props) : style;
};

class StyledComponent extends React.Component {
  static propTypes = {
    classes: PropTypes.object.isRequired,
    className: PropTypes.string,
  };

  render() {
    const { classes, className = '', WrappedComponent, ...passThroughProps } = this.props;

    return <WrappedComponent classes={classes} className={className} {...passThroughProps} />;
  }
}

const styled = (WrappedComponent, customProps = {}) => {
  return (style, options = {}) => {
    const HOCProps = WrappedComponent => {
      return class _HOCProps extends React.Component {
        constructor(props) {
          super(props);
          this.FinalComponent = withStyles(theme => {
            const defaultStyles = styles(theme, props, style);
            const mergedStyles = merge(defaultStyles, props.styles ? props.styles : {});
            return mergedStyles;
          }, options)(StyledComponent);
        }

        render() {
          const { styles, ...otherProps } = this.props;
          return <this.FinalComponent {...customProps} {...otherProps} WrappedComponent={WrappedComponent} />;
        }
      };
    };
    return HOCProps(WrappedComponent);
  };
};

export default styled;

In case you have an idea what could be causing the leak feel free to let us know, any help here would be greatly appreciated.

[gabrielliwerant]

It will be difficult for me to isolate without an example I can continually use to check the problem. If it's specific to dynamically loaded components, that seems far enough away from traditional use cases, that it might be hard to pin the problem entirely on this library. So I'd need either a reproducible example or something where it's an issue without the dynamic loading.

[patorjk]

I decided to take a look into this and I noticed a few things:

• styled.js is only used in one file: TableToolbar.js
• It is not needed in that file. The check that occurs (checking if props.options.responsive is true) should always be true per the valid values of "responsive" in the documentation ("stacked" and "scroll").

I've put in a PR that refactors TableToolbar to use withStyles instead of styled.js. I also added a warning if someone puts in an invalid value for options.responsive. Unless I'm missing something, that should hopefully clear up this memory issue.

I've looked over TableToolbar several times and can't see any other reason styled.js was being used.

[patorjk]

I can confirm this memory leak.

I was able to re-produce this in my work environment and have created a sandbox the simulates the problem:

https://codesandbox.io/s/muidatatables-custom-toolbar-5o1hr

Here's how to re-produce the issue:

• The table in the sandbox does NOT have a toolbar. It appears or disappears every 5 seconds.
• Open Chrome Dev tools
  • Select the Memory tab.
  • Select the "Allocation instrumentation on timeline" option.
  • Select the "5o1hr.csb.app" VM
  • Start recording.
• Watch the memory as the table appears and disappears several times, the blue bars represent memory that was allocated at a particular time yet has not been released.
• Stop recording.
• For a table with no toolbar everything looks great. Now modify the example to include a "title" (or turn on one of the header actions like search) so that the table is rendered with a toolbar. Do the above steps again, you'll see that the memory isn't completely being released.

For this small example it's not terrible (around 1.3MB like @Fabbok1x said), but for larger tables there's a lot of memory that doesn't appear to be being released (on my workstation I had a table that was 10MB).

Edit: A more easily reproducible example is here: #829 (comment)

I haven't tested this against the PR I submitted last night. I'll try and do that tonight.

Also, as a use case for when this would happen: The use case for me is an interface that uses tabs. My app has a sidebar that has buttons that open up a tab with a datatable. A user interacts with the table and then closes the tab.

[patorjk]

Sorry for posting on this so much. I've updated the example in my previous comment to be more clear (ie, you need to select a particular VM or you won't see the memory leak). The example in my PR is a little more straight forward, but confusingly it requires the selection of a different VM - I'm guessing codesandbox sets up the VMs differently when you create a sandbox from a repo.

You'll know you're watching the right VM if the memory spikes up to around 1.3MB when the table appears.

[gabrielliwerant]

Thanks for your diligence on this @patorjk and for opening the issue @Fabbok1x!

@patorjk's fix is in 2.9.0, so see if that helps you @Fabbok1x.

[gabrielliwerant]

Closing this, as the additions have been in the library for a few releases now. If there's good evidence that there's still an issue that is related to the things in discussion here, I can re-open.