Huge performance issue when dispatching hundreds of actions #263

Closed
elado opened this Issue Jan 21, 2016 · 7 comments

Comments

3 participants
@elado

elado commented Jan 21, 2016

On boot, the app I'm building gets a stream of data from a server. This stream can have hundreds of messages and entities and each message is handled separately.

It takes a tremendous amount of time to dispatch actions after UI has rendered, because every state change re-renders the @connected component.

I wrote a short example that shows dispatching 1000 actions before render (5-15ms) and 500 after render (~2.5s). So this is clearly not a redux issue.

https://jsbin.com/ruziwa/edit?js,console,output

Obviously component doesn't need to re-render every single dispatched action, but re-rendering should be throttled (say, every 50ms). Is there an established approach for that or does it need to be integrated in @connect?

@elado

This comment has been minimized.

Show comment
Hide comment
@elado

elado Jan 21, 2016

Well, adding

ItemListContainer.prototype.render = _.throttle(ItemListContainer.prototype.render, 20)

really boosts performance.

Updated https://jsbin.com/ruziwa/11/edit?js,console,output

Probably messing with React's methods is not the right approach though.

elado commented Jan 21, 2016

Well, adding

ItemListContainer.prototype.render = _.throttle(ItemListContainer.prototype.render, 20)

really boosts performance.

Updated https://jsbin.com/ruziwa/11/edit?js,console,output

Probably messing with React's methods is not the right approach though.

@epeli

This comment has been minimized.

Show comment
Hide comment
@epeli

epeli Jan 21, 2016

Collaborator

Try this: Collect those those messages into an array and flush it with store.dispatch in a single tick with every 50ms (or whatever is required). You must use redux-batched-updates to avoid rendering on every dispatch on the tick.

https://github.com/acdlite/redux-batched-updates

Collaborator

epeli commented Jan 21, 2016

Try this: Collect those those messages into an array and flush it with store.dispatch in a single tick with every 50ms (or whatever is required). You must use redux-batched-updates to avoid rendering on every dispatch on the tick.

https://github.com/acdlite/redux-batched-updates

@epeli

This comment has been minimized.

Show comment
Hide comment
@epeli

epeli Jan 21, 2016

Collaborator

Probably messing with React's methods is not the right approach though.

Yeah that will break sooner or later...

pseudo code

var store = createStoreWithBatchMiddleware();
var messages = [];

stream.on("message", msg) => messages.push(msg));

setInterval(() => {
  messages.forEach(msg => store.dispach(handleMessage(msg)));
  messages = [];
}, 50);
Collaborator

epeli commented Jan 21, 2016

Probably messing with React's methods is not the right approach though.

Yeah that will break sooner or later...

pseudo code

var store = createStoreWithBatchMiddleware();
var messages = [];

stream.on("message", msg) => messages.push(msg));

setInterval(() => {
  messages.forEach(msg => store.dispach(handleMessage(msg)));
  messages = [];
}, 50);
@elado

This comment has been minimized.

Show comment
Hide comment
@elado

elado Jan 22, 2016

Thanks! I integrated https://github.com/tappleby/redux-batched-subscribe (redux-batched-updates uses deprecated api)

I used _.debounce, not unstable_batchedUpdates (it didn't seem to work at all)

This is the amazing result: https://jsbin.com/wurower/7/edit?js,console,output

Would it make sense to integrate to react-redux? Or to write a note about it in the docs?

elado commented Jan 22, 2016

Thanks! I integrated https://github.com/tappleby/redux-batched-subscribe (redux-batched-updates uses deprecated api)

I used _.debounce, not unstable_batchedUpdates (it didn't seem to work at all)

This is the amazing result: https://jsbin.com/wurower/7/edit?js,console,output

Would it make sense to integrate to react-redux? Or to write a note about it in the docs?

@epeli

This comment has been minimized.

Show comment
Hide comment
@epeli

epeli Jan 22, 2016

Collaborator

I'm glad it solved your issue but you should note that debouncing and update batching are completely different things.

With debouncing you end up just completely ignoring most of the messages. That can be ok but if your store state requires to see every message, for example you need sum up some numbers from them, that will not work as expected.

React update batching and the setInterval trick I presented will dispatch every message to the store but will only do it batches to avoid excessive rendering.

Collaborator

epeli commented Jan 22, 2016

I'm glad it solved your issue but you should note that debouncing and update batching are completely different things.

With debouncing you end up just completely ignoring most of the messages. That can be ok but if your store state requires to see every message, for example you need sum up some numbers from them, that will not work as expected.

React update batching and the setInterval trick I presented will dispatch every message to the store but will only do it batches to avoid excessive rendering.

@elado

This comment has been minimized.

Show comment
Hide comment
@elado

elado Jan 22, 2016

@epeli not sure I understand the issue, can you please explain with code?

So far it seems that the issue of multiple unnecessary renders is gone, due to debouncing it, and store is filled correctly after every single dispatch action (it doesn't wait for the debouce time to execute a bunch of actions together).

elado commented Jan 22, 2016

@epeli not sure I understand the issue, can you please explain with code?

So far it seems that the issue of multiple unnecessary renders is gone, due to debouncing it, and store is filled correctly after every single dispatch action (it doesn't wait for the debouce time to execute a bunch of actions together).

@elado elado referenced this issue in reduxjs/redux Jan 22, 2016

Closed

Immutable data + bad performance #1262

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jan 25, 2016

Collaborator

I'm closing as this doesn't appear to be an issue with this repo—rather an issue with a particular approach. Please feel free to continue discussing!

Collaborator

gaearon commented Jan 25, 2016

I'm closing as this doesn't appear to be an issue with this repo—rather an issue with a particular approach. Please feel free to continue discussing!

@gaearon gaearon closed this Jan 25, 2016

@broadsw0rd broadsw0rd referenced this issue in broadsw0rd/react-redux-perf May 2, 2016

Closed

Research #1

@Tyler-Zhang Tyler-Zhang referenced this issue in waterloop/goose2-dashboard Jun 21, 2017

Closed

Optimize rendering #6

@reduxjs reduxjs deleted a comment from maxmax1992 Jul 20, 2017

@rrag rrag referenced this issue in rrag/react-stockcharts Jan 9, 2018

Closed

performance issue when loading several charts on a page #438

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment