Pseudo leak with large objects in the store.

[jvanderberg]

Please see: http://jsfiddle.net/3xwkg3p6/80/. Follow the instructions and track the memory usage in the Chrome task manager and you will quickly get over a 1 GB, even though the only 'live' data in the mini-app consumes about a tenth of that amount of memory.

This is a somewhat extreme example, but it shows the problem clearly. There are ten checkboxes which subscribe only to their own checked state. The refresh button, when clicked, generates a massive random number array and stores it in the state variables 'data'. Clicking a checkbox causes react-redux to save a reference to the store's state at the time the checkbox was clicked in the checkbox wrapper's own local state. Subsequent clicks of the refresh button don't update the the props of the checkboxes, preserving those stale references to previous state, making it impossible for GC to clean up the old array data, which nothing in the application is actually using.

This isn't a memory leak per-se, as memory consumption is still bounded, but a side effect of how connect.js handles refreshing components by calling setState({storeState}) (connect.js:262). Note that this line executes only if there was a change that affected the component's own props. This causes every component to keep a reference to the entire store state as of the last time its own props changed. This isn't a problem if the store is small, but it becomes a serious issue if you are storing large chunks of data in the store and you have many components that don't subscribe that that large state variable.

We noticed this when we moved to calculating some large analytic results in a reducer and putting the results in the store, rather than calculating these results dynamically in the views.

We very well could be Doing It Wrong, and I'd love for there to be a simple solution that I am overlooking, but it's not immediately obvious to me.

One solution is to store large data results outside of redux. We'd like to avoid that. We also thought of creating a result wrapper class where all instances point to a single static array. This seems a bit of a hack and would make replay impossible (though replay presents its own memory issues with large amounts of data in the store).

We could also have fewer connected components - connect at a parent level and pass props down. In the jsfiddle example this would involve creating a connected wrapper which subscribes to the checkbox state and the data array and passes props down to the checkboxes and refresh button. We'd like to avoid this as well, as we feel it defeats one of the primary benefits of using redux - not having to plumb props down through views.

[jimbolla]

Would you mind trying this against react-redux@next and seeing if it makes a difference?

[jvanderberg]

I've tried it with our application locally and it did not seem to impact memory utilization (it still seemed to be using as much memory as master). Even though the changes in the @next branch are relatively concise, I found it hard to track through the code to see if it was preserving stale state references. I'd imagine you'd be the expert on that.

That JSFiddle is just a copy of another react-redux example I found and I am not an expert on those various CDN links, do you know how I'd replace the CDN links to react-redux@next?

[timdorr]

You can use unpkg.com for that: https://unpkg.com/react-redux@5.0.0-beta.1/dist/react-redux.js

[jvanderberg]

Thanks, that seems to be much better: http://jsfiddle.net/3xwkg3p6/81/

I revisited our app, and I think I maybe had a babel cache issue with node modules when I tried out @next. I cleared out the babel cache, rebuilt and memory usage seems to be much improved.

I guess just consider this a verification that @next really is a huge improvement.

[timdorr]

Cool. I'll close this out as 5.0 should be rolling out soon-ish. Just have to get some infrastructure things in place and we'll be good to go. But the code itself is pretty solid so far.