[Performance] Components cause re-rendering

[oreqizer]

Description

Components create new identities that cause unnecessary re-rendering (sometimes hundreds, see 2nd screenshot).

Images & references

Small example:

A lot of:

This was just the page render and a hover action.

Problem description

Creating new identities within components causes hundreds of re-renders.

Link to minimally-working code that reproduces the issue

A minimal example. Shows better when coupled with more components:
http://www.webpackbin.com/Vy-83jCbz

Versions

• Material-UI: 0.16.4
• React: 15.4.0
• Browser: Chrome

[oliviertassinari]

That raises an interesting question for the next branch.
Should use add a pure logic on lower-level component?

That's a tradeoff to take between :

• saving some time in the render method when properties are the same.
• increasing the time spent in the render method when properties are not the same.

react-bootstrap has made his mind on this and his not doing it.

Better at any rate for the higher-level component that is calling into React Bootstrap to implement the relevant pure render logic

react-bootstrap/react-bootstrap#633 (comment)

I pretty much agree, I think that the pure logic is only bringing value in rare case for Material-UI.

Following a past discution cc @nathanmarks

[oreqizer]

I made sure all my components don't do unnecessary re-rendering, see oreqizer/reactizer#20, so that the measurements aren't influenced by my code. All those re-renders were from Material-UI itself (from a hover action to be precise).

[oliviertassinari]

All those re-renders were from Material-UI itself (from a hover action to be precise).

I get your point. The rerendering was triggered in a deep node on the tree by a Material-UI component.
In that case, I think that the performance impact is not significant. As it's rerendering a very small part of the app. I can't see how that can introduce jitters or lags in the UI.
I still think that not introducing a pure logic is the best approach in order to reduce CPU cycles when a big chunk of the app is rerendered and those CPU cycles matters.
That vision is theorique. We would need benchmarks to make a better tradeoff.

[nathanmarks]

@oliviertassinari With the current master, I have seen some pretty bad performance problems using TextField in forms.

The problem is aggravated by the fact that people often have multiple TextField elements in a rendered in a form component.

Grab a popular redux form library such as redux-form and you suddenly have multiple TextField components that are all re-rendering every single time you type a character into an input.

When you account for the redux dev tools overhead replaying state, the component re-rendering IIRC is a big contributor to making the form pretty unusable. This is also due to the fact that the current components recalculate the inline styles every single render.

Toss in some Dividers between the TextFields, perhaps some Tooltips, and you quickly run into big trouble.

I was able to fix this by wrapping TextField myself using a fairly aggressive shouldComponentUpdate handler.

I think we should benchmark this scenario using components that are better performing than the current master branch.

[oliviertassinari]

I have seen some pretty bad performance problems using TextField in forms.

I have seen it myself too. I ended up connecting each field independently to avoid the rendering of a large part of the form. That's also the pattern we use at @doctolib, but this time, with RxJS not redux.

As your form grow, soon or later, pure logic won't be performant enough and you gonna have to connect lower in the tree.

Still, I agree. I think that we should apply a different tradeoff with form components.

[ArcanisCz]

From our experience with redux-form i can say, you always have some wrapper(s) for field elemens either way, so they can have some pureRender with no problems. Additionaly, you can use Immutable even for redux-form (redux-form/immutable) where you receive same references for non-changed properties, so cheap shallowCompare is sufficient.

[oliviertassinari]

From what I can understand of the situation, the best path going forward seems to not implement any pure logic by default as it can potentially degrade the performance. Then, when users need to prune the rendering tree, they have the following tools available:

• immutable data structure and pure logic, that can be implemented with Immutable and pure()
• subscribe to updates lower in the rendering tree. Effectively for a form and redux that means connecting each input.

[robininfo-edsx]

I disagree with oliviertassinari is about the tradeoff he spoke about.
Checking if some properties change will take only few CPU instructions, where as a single TextField take around 1ms and most forms are using more thant 2 TextFields and it's already worse with 2 TextFields.
The workaround of connecting individually ? If you don't want to work with something that manage a global state like redux and use the relation between Parent and Children you can't do that.
An other point is the release of React 16 which now check if properties change on PureComponents without needing to write a function for that and that on every properties however PureComponent are not available before React 15.3.