Performance downgrade after update to v.6.0.0

[m1lk1way]

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

In ver.6.0.0 current state passing into context provider, it leads to update on every consumer (every connected component (on context.consumers)). Wrapped component isn't updates but React has work to be done and its influence on performance is greatly negative.
Our case when there is connected input to store and there are a lot of connected components on page and user starts typing fast (like long keypress), in this case we see lags (like input freezes and throws updates by portions).

What is the expected behavior?

As it was in v5.1.1.

Which versions of React, ReactDOM/React Native, Redux, and React Redux are you using? Which browser and OS are affected by this issue? Did this work in previous versions of React Redux?

  "react-dom": "16.7.0",
  "react-redux": "6.0.0",
  "redux": "4.0.1",

As an example, you can check this fiddle
https://x3yqx3ov1q.codesandbox.io/ - (upper component with greetings mustn't be updated when new ToDoItem added)
Source code of fiddle
https://codesandbox.io/s/x3yqx3ov1q

UPD it can be related to this issue in react repo facebook/react#13739
If so, what is the workaround (only one we came up with - downgrade of react-redux to v5.1.1)?

[timdorr]

We have a benchmark suite we have run against any major release to ensure we don't have severe performance regressions. 6.0 is slightly slower than 5.1.

If you're running into performance issues with 6.0, it's likely you were close to the edge of acceptable performance before. For instance, you absolutely should not be hooking up an input to the store. That is too high bandwidth and isn't state that affects the rest of the app. If you're doing typeahead-style search, you should instead batch dispatches with a debouncer. That's good for everyone, not just React Redux.

But in your example, that Test component you added isn't rendering when the store changes. We use much of the same methods for avoiding renders as before. There is an attempt to render, but it is blocked when there is no change detected.

[m1lk1way]

@timdorr firstly thx for answering this. Component isn't rendering, like you said, but react has a lot to be done (to build virtual dom and compare) and it takes huge part of lag time as Chrome Profiler says. And batch dispatches, you mentioned, seems like a crutch, not a fix. Anyway we have only one fix for this - downgrade (as we have no problems with previous versions, even if we multiply *5 number of connects on the page).

[PashaPash]

@timdorr we got a total performance screw on v6 upgrade.

you absolutely should not be hooking up an input to the store

That is exactly what redux-form does. They do connect every field to the store, and that causes mass updates for all connected components with new react-redux.
Live apps have forms field connected to the store. Moreover, connecting each list item to the store is a common performance optimization - that efficiently prevented updates and re-renders in v5.

I afraid that the tests are synthetic and does not show the real impact of the change. Most of them have just few connect calls.

For our prod app v6 just killed the performance. There are no re-renders, but there are mass updates on connected components and that somehow slows everything down. It seems that each extra connect slows down the entire page, so list and forms page are just slow. Extra connects were free in v5.

Do you have any ideas on how to track what exactly is slow, as there are no re-renders, just connect-level updates?

[markerikson]

@PashaPash , @m1lk1way : there's not much we can do unless you can provide projects that specifically demonstrate the performance issues. If you can provide repos that I can just clone and run, I might be able to look into things.

[markerikson]

I'm going to go ahead and reopen this, since overall perf is something I'd like to look into further.

[markerikson]

One idea I just had is to memoize the children of <Provider>. React will still have to walk the tree to find the context consumers, but at least that ought to prevent the entire tree from completely re-rendering every time the store is updated.

In practice, I'm not sure how much of a difference this would make, because it's likely there's connected components high up in the tree acting as "blockers" for the cascade of re-rendering from <Provider>'s setState call, but I don't think it can hurt.

[markerikson]

Just did a bit of experimenting, and I'm not sure the "memoize provider children" idea will actually do anything:

https://codesandbox.io/s/jln1wjor25

What I'm seeing is that when the parent calls setState(), the child component doesn't actually re-render at all, because the parent's props.children are identical to the last time. In other words, they're already "memoized" becaues React kept them around from the previous render for us, and React skips rendering when elements are the same as last time.

[Jessidhia]

Yeah, that will work as long as there is nothing else in between children and the <Provider/>

[markerikson]

Here's the current code:

react-redux/src/components/Provider.js

Lines 63 to 71 in 6e0a106

	render() {
	const Context = this.props.context || ReactReduxContext
	return (
	<Context.Provider value={this.state}>
	{this.props.children}
	</Context.Provider>
	)
	}

[PashaPash]

@markerikson thanks for reopening. not sure about prod code access, will build a sample based on it.

[PashaPash]

@markerikson @Jessidhia here is a code sample https://github.com/PashaPash/connect-sample
Sorry for a lot of garbage/unused code, copypaste from the typescript prod until I got some minimal working example.
Most of reducers and components are unrelated, can minimize it to some clear code if needed, but that will take a while.

Here what on the page:

• There is a list of 100 connected grid rows. Each row is connected directly to the state. That seems to be a common optimization pattern, followed the https://blog.isquaredsoftware.com/2017/01/practical-redux-part-6-connected-lists-forms-and-performance/#connecting-the-pilotslist-component. (@markerikson, your blog is really awesome!)
• A text box that do unrelated state changes, used as a trigger. Could be any other action trigger as well.

Measured timings on dev machine, i7-7700k,

On 5.1.1: Only text box gets updated.
Total event time: ~6-5ms:
Interactive update (<1ms Tree Reconciliation + Changes commit): ~1.5ms,

Timeline:

On 6.0.0: All connected rows gets updated (but not re-rendered)
Total event time: ~15-16ms:
Interactive update (10ms Tree Reconciliation + Changes commit): ~11ms

Timeline:

There is 6x slow down for the interactive update part. 10ms difference may not be an issue, please note that I'm measuring on dev pc, and components tree is quite simple.
Things are getting worth on average client PC. Typical consumer Core i3 is 4x times slower

Setting up 4x slowdown in chrome performance tools results in 80-100ms total event time (and 60-80 ms of Tree Reconciliation caused by connected components update).

We have a bit more components connected in the live app. So we are getting ~80-100ms lags on everything, including server-side pushes and API responses.
Some patterns, like multiple fields + save on blur are really laggy now - as user gets api response from the previous field save while typing a value in new field.
App feels to unpredictable lagging for the end user.

The root cause seems to be state passed via store provider in v.6 that leads to full tree traversal on store change in React 16

Any ideas how to fix that? Right now the only workaround is to stay on 5.1.1.

---

Another drawback of v6 behavior is mass unrelated updates highlight in React dev tools, as it breaks the common way of finding unwanted components updates, but that is not as critical as performance degradation for me.

[markerikson]

@PashaPash : Thanks for putting that together. I'm trying to juggle several different priorities atm, so I may not get to this one right away, but it's on my list.

If anyone else would like to help out by doing some analysis on this, I'd appreciate it.

[richie-south]

Hey :) We are seeing similar results :( when updating an isolated redux value all components are updated causing large slowdowns.

Isn't this what observedBits is supposed to solve?

---

We will stay on 5.1.1 until this is resolved