Material-ui: Improve Material-UI performance

[Bessonov]

First, thank you very much for this awesome component library! It's great!

I added a drawer in my new app. Mostly, I copypasted drawer example. Just for PoC I multiplied

        <Divider />
        <List>{mailFolderListItems}</List>

section.

After that it feels very slow, especially on mobile device (nexus 4, cordova with crosswalk 20). I use dev mode, but prod mode doesn't speed up much.

Through react dev tools I noticed that components in mailFolderListItems rendered on every link click in react-router or even menu open. It takes sometime up to 50-60ms to rerender ONE {mailFolderListItems}. I use

const modalProps = {
    keepMounted: true, // Better open performance on mobile.
};

To eliminate uncertainty with other app components, I converted mailFolderListItems to Component and disable rerendering:

class MailFolderListItems extends React.Component<{}, {}> {

    shouldComponentUpdate() {
        return false;
    }

    render() {
        return (
            <List>
                <Link to={Routes.Startpage.path}>
                    <ListItem>
                        <ListItemIcon>
                            <InboxIcon />
                        </ListItemIcon>
[...]


                <Divider />
                <MailFolderListItems />

After that this part feels OK.

I found https://github.com/mui-org/material-ui/issues/5628 . I suggest to revisit it. Optimizing shouldComponentUpdate is fundamental and most important step to gain performance. PureComponent is just most common shortcut.

Furthermore, I noticed that very much time (1-2ms and more for EVERY material-ui component) is spended in WithStyles.

• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

I'm expecting to get most of possible react performance for this great library.

Current Behavior

The app get slower with every material-ui component.

Steps to Reproduce (for bugs)

I don't provide reproducing example yet, because I just copypasted from component demo page, but if needed I can provide codesandbox demo. For browser it's noticeable, if browser slowed down by factor >=5x in performance setting.

Your Environment

| Tech | Version |
|--------------|---------|
| Material-UI | 1.0.0-beta.38 |
| Material-UI Icons | 1.0.0-beta.36 |
| React | 16.2.0 |
| browser | cordova crosswalk 20 (equals android chrome 50) |

[oliviertassinari]

I'm expecting to get most of possible react performance for this great library.

@Bessonov Performance will be a focus post v1 release. We have tried to keep the core of the library as fast as possible. It should be good enough for +90% of the cases. At least, it's my experience using the library so far. You haven't provided any external reference, aside from raising our attention around the fact that performance is important, we can't make this issue actionable. If you are able to identify a performance root limitation of Material-UI that we can investigate (with a reproduction codesandbox or repository), please raise it. Until then, I'm closing the issue.

[Bessonov]

@oliviertassinari thank you for your quick response! I'm very happy to hear, that performance will be focus after release.

It should be good enough for +90% of the cases. At least, it's my experience using the library so far.

On desktop - yes, it is. But on mobile it is really slow. Just Drawer and some buttons makes app laggy. It's not responsive and consumes more power as needed.

You haven't provided any external reference, aside from raising our attention around the fact that performance is important, we can't make this issue actionable.

I provided a reference to already raised issue and a reference to react documentation.

If you are able to identify a performance root limitation of Material-UI that we can investigate (with a reproduction codesandbox or repository)

As I said, I can do it. Here is a comparison of pure and non pure component. Steps to reproduce are:

1. Use chrome and go to https://codesandbox.io/s/r1ov818nwm
2. Click "Open in a new window" in preview window
3. Open dev tools and go to "performance" tab
4. Throttle your CPU by at least 5x (depending on your PC - may be 10x) to reflect mobile device
5. Click on burger and see how it lags.

And now:

1. Go to index.js
2. change pure to true
3. Save
4. Repeat steps from above
5. Enjoy it!

This example is a litte bit unrealistic because I've inserted too much List elements. But as I said, on mobile it get very quickly to point where you "feel" every action.

[Bessonov]

Here is my issue with WithStyles. It's on Desktop with CPU throttling. I would like to post screenshots from real device on Monday.

Time for WithStyles(ListItem):

Time for ListItem:

The difference is 1.26 ms just for a ListItem component. With 13 components like this you can't render with 60fps anymore. But I noticed on Desktop, that this duration is not always here.

[Bessonov]

Here is a comparison of pure and non pure component

Btw I don't say that PureComponent is THE solution for all performance issues. I just say, that it can be one of the helpers to make material-ui usable on mobile.

[oliviertassinari]

The difference is 1.26 ms just for a ListItem component.

@Bessonov The WithStyles component should be very cheap for repetitive instance of the same component. We only inject the CSS once.
Maybe you are reaching React limitations and the cost of higher order components.
There is a range of solution to mitigate performance issue in React. For instance, you can use element memoization and virtualization. I'm keeping this issue open as the start point for future performance investigation.
I don't think there is much we can do about it right now. Thanks for raising the concern.

[Bessonov]

Ok, that's only one part. What do you think about more important part - optimizing shouldComponentUpdate?

EDIT: I want to split this issue in two parts. This part is about shouldComponentUpdate and the other part for WithStyles, if I can show more information. Is that OK for you?

[oliviertassinari]

optimizing shouldComponentUpdate

@Bessonov We don't implement such logic on purpose on Material-UI side for two reasons:

1. Most of our components accept react elements, a react element reference change at each render, making the logic systematically waste CPU cycles.
2. The closer the shouldComponentUpdate logic is from the root of the React tree, the more efficient it's. I mean, the more pruning you can do on the tree, the better. Material-UI components are low level. There low leverage opportunity.

The only opportunity we have found is with the icons. Let us know if you can identify new ones.

the other part for WithStyles

+90% of the time spent in WithStyles is actually spent in JSS, there very few we can do about it on Material-UI side.
At least, I have been identifying a caching opportunity for server-side rendering lately to greatly improve the performance on repetitive rendering. But it's server-side only.

[Bessonov]

Most of our components accept react elements, a react element reference change at each render, making the logic systematically waste CPU cycles.

That's depending on usage and can be improved. I don't benchmark myself, but I'm sure that right component usage and optimized shouldComponentUpdate (with shallow comparison) is less costly than not optimized component, especially for immutable structures (and immutable.js isn't required btw). Related ticket: https://github.com/mui-org/material-ui/issues/4305

For example in https://github.com/mui-org/material-ui/blob/v1-beta/docs/src/pages/demos/drawers/PermanentDrawer.js#L68 this leads to new objects and makes PureComponent senseless:

        classes={{
          paper: classes.drawerPaper,
        }}

because it returning every time a new object. But not:

const drawerClasses = {
    paper: classes.drawerPaper,
};
[...]
        classes={drawerClasses}

Same for components.

The closer the shouldComponentUpdate logic is from the root of the React tree, the more efficient it's. I mean, the more pruning you can do on the tree, the better.

Yes, that's true.

Material-UI components are low level. There low leverage opportunity.

That's partially true. As you can see in https://codesandbox.io/s/r1ov818nwm I just wrap List in PureComponent. Nothing else and it speed up Drawer by significant factor. I would claim that this is true for every component, at least which use {this.props.children}. I created another demo: https://codesandbox.io/s/my7rmo2m4y

There is just 101 Buttons (pure = false) and Buttons wrapped in PureComponent (pure = true, see where Button import from). Same game result, if I click the "Click"-Button:

Normal Button:

Wrapped Button (with overhead and so on):

As you can see, there is 637ms vs. 47ms for just normal Buttons! Do you still think that optimize shouldComponentUpdate (or just PureComponent) not worth it? :)

EDIT: fix wording at beginning

[Bessonov]

@oliviertassinari @oreqizer I noticed interesting thing. It seems like extends PureComponent performs better as Component with

  shouldComponentUpdate() {
    return false;
  }

EDIT: I think this is one of react internal optimization.

[oliviertassinari]

As you can see, there is 637ms vs. 47ms for just normal Buttons! Do you still think that optimize shouldComponentUpdate (or just PureComponent) not worth it? :)

@Bessonov It's demonstrating the potential of the pure logic. Yes, it can be very useful! It's a x13 improvement. But I don't think it's close to real-life conditions:

• Instead of 100 buttons, you will find 10 buttons at most on a page. Still, you will find 10 grids, 10 X, etc.
• Instead of simple properties provided to the lower level elements, you will use something like react-intl, the check between the previous and new props will always be false. You will waste CPU cycles. So x13 -> x0.8 or so

[Bessonov]

@oliviertassinari as a great developer, how you can write things like this? Why you use _your personal assumptions_ as arguments and no facts? I think, I presented enough facts above. I did everything to show it, because I like this project and want to contribute. It's not enough? Ok, then more facts and _no_ assumptions.

Just for you I reduce to 10 Buttons. 10! It's mean any 10 components (worse: containers) from material-ui slows down entire app until the app is not usable! Tested on real device with crosswalk 21/chrome 51 without any throttle:

Normal button:

PureButton:

This still an 8x improvement! It's huge! Can you imagine how important this on mobile device is?

Instead of 100 buttons, you will find 10 buttons at most on a page. Still, you will find 10 grids, 10 X, etc.

I used Button because it is a one of simplest component! It shows that material-ui is broken from performance point of view. Now, imagine a container components like a AppBar, Toolbar, List, Drawer! This is even worse! You get very quickly to 20 components/containers on every page. And because you don't expire any slowness on your powerful desktop/mac it's not mean that material-ui isn't incredible slow.

react-intl, the check between the previous and new props will always be false. You will waste CPU cycles. So x13 -> x0.8

Show me an example on codesandbox. I don't see why this should be happen. I'm sure, this happens only on wrong usage of react components. And official example shows wrong usage, because it seem like react-intl not applied context subscribers. But there is a lot of other components which are aligned with react guidelines and best practices to stay performant.

BTW: WithStyles consume up to 2,27ms for the Button on mobile device. 8 components and you under 60fps.

[oliviertassinari]

Why you use your personal assumptions as arguments and no facts?

What makes you think it's a personal assumption? I was trying to perform critical thinking. Conceptually speaking, the extra prop traversal will slow down the pure version compared to the non pure version, it has to prune something to worth it. Same reasoning as #5628 or https://github.com/react-bootstrap/react-bootstrap/issues/633#issuecomment-234749417 or https://github.com/reactstrap/reactstrap/pull/771#issuecomment-375765577

With pure:

Without pure:

The reproduction is the following.

• Open https://codesandbox.io/s/715p7661k1
• Start the profiler on Chrome.
• Click 5 times on the render button

[Bessonov]

@oliviertassinari are you sure that codesandbox makes everything right for your test? Because my results are very different:

Without pure (even without throttle it's slow):

Pure (after change to true and save I get new url for codesandbox):

[Bessonov]

Since it doesn't check context changes and also forces users to ensure all of the children components are also "pure", I don't believe this to be a good fit for this library. This library needs to be as flexible as possible and I believe this will make it more difficult to use.

I see the point. But it's a very interesting tradeoff. On one side even material-ui should be "as flexible as possible", but on the other side current performance makes it _unusable_ at all.

May be you should think about providing pure and non-pure version of components. I do it now for my app to get some usable perfomance even on desktop.

[oliviertassinari]

@Bessonov The codesandbox wasn't saved correctly. Sorry. It was missing the following diff. I have updated the link:

<Button>
+ <i>
    Button
+ </i>
</Button>

[Bessonov]

I don't understand why it should produce different results? I get better chart, but non-pure version is significantly slower.

EDIT: ok, I see. Try to figure out what going on...

[Bessonov]

OK, I understand now. Just the same "new object on every render"-thing. I didn't noticed it before. For some cases it can be improved with constants trough babel plugin automatically.

[oliviertassinari]

@Bessonov https://medium.com/doctolib-engineering/improve-react-performance-with-babel-16f1becfaa25

[Bessonov]

Well, then you know it already! :D Even there is not much benefit on it's own (you showed ~7%), it's still profitable in conclusion with pure components to avoid some flaws you mention above. I tested it now with Pure Wrapper + babel plugin + production build and it's impressive fast on the same mobile device!

As I said, I think it's best to offer both - non pure components for flexibility and pure components (wrappers are enough to keep it simple and maintable) for performance. But for me, I can live with just pure components, because overall performance improvement is much greater that performance drawbacks. Or better: I can't use material-ui without pure components.

Ok, for now I'm waiting for further input on this topic and create own wrappers in my app )

Thanks for insights!

[dantman]

I've never heard of transform-react-constant-elements actually being used and being really useful. It's ok as as random micro-optimization to throw in, but in practice you rarely write code simple enough to get much from it. Although, I suppose it wouldn't be a bad optimization for all the SVG-style icon components like <Add />.

Take a look at this example (click on Plugins in the side, search for "react-constant" and click the checkbox on "transform-react-constant-elements") and you'll see that barely anything has been optimized:

• The simple static InputAdornment has been moved to the top, yay.
• But the trivially simple submit button was no longer optimized the moment we put an event handler on it.
• And while the InputAdornment itself is now hoisted, the InputProps={{startAdornment: ...}} is still inline and creating a new object every render which makes PureComponent impossible.
• Likewise classes={{label: classes.runButtonLabel}} is making it impossible for PureComponent to optimize the Run button.

I personally like PureComponent and try to use it absolutely everywhere and optimize things as much as I can so that it works. But it doesn't look like MUI is at all made in a way that PureComponent would work.

• The *Props props like InputProps is a fundamental pattern to how MUI works. It's not just an advanced way to modify MUI internals when you need it, but rather something that is used regularly in simple use cases. This pattern often makes any leaf component that could normally be optimized in pure mode, unoptimizable.
• Likewise the classes={{...}} pattern also doesn't work with PureComponent and it is the way to add any styling to things in MUI. (And saying to use classes={classes} is in no way practical because a real life consumer likely has a different class name than the component's internal class and classes is likely to also include classes meant to style other elements in the same consuming component)
• Children are used a lot, instead of having 1 component a common pattern often makes use of 2-3 and only one of them can be pure optimized if any of them can.

If we want to optimize anything, these fundamental issues need to be dealt with, otherwise simply letting people enable a pure mode MUI won't actually optimize much at all. I can think of two possibilities.

We just allow pure mode to be enabled, consumers have to memoize or hoist objects manually to actually get optimizations

1. One way I can think of to do this would be some short of shallowMemoize which using the local this and a key (so you can use it on different bits of data) that memoizes the data as long as it's shallow equal
2. Another I can think of is using something like reselect to create selectors that will memoize data.
3. Another way of doing the shallowMemoize in 1. would be to pass it to render() with a decorator. This way we could pass in a new one each render and instead of needing key we can just check if any of the memoized objects from the last render should be re-used, then discard all the old values.

The problem of course is this makes consumers a lot larger and messier and requires logic to be manually hoisted up to places where it's far away from the code using it.

import {createSelector} from 'reselect';

class FormPage extends PureComponent {
  state = { example: '' };

  change = e => this.setState({example: e.target.value});
  submit = () => {
    console.log('Submit: ', this.state.example);
  };

  runButtonClasses = createSelector(
    props => props.classes.runButtonLabel,
    runButtonLabel => ({runButtonLabel}));

  render() {
    const {title} = this.props;
    const {example} = this.state;

    return (
      <form>
        {title}
        <TextField
          InputProps={this.shallowMemoize('a', {
            // This example assumes use of transform-react-constant-elements to make this object always the same
            startAdornment: <InputAdornment position="start">Kg</InputAdornment>,
          }}}
          onChange={example}
          value={example} />
        <Button classes={this.runButtonClasses(classes)}>Run</Button>
        <Button onClick={this.submit}>Submit</Button>
      </form>
    );
  }
}
// ...
  @withShallowMemoize
  render(memo) {
    const {title} = this.props;
    const {example} = this.state;

    return (
      <form>
        {title}
        <TextField
          InputProps={memo({
            startAdornment: <InputAdornment position="start">Kg</InputAdornment>,
          }}}
          onChange={example}
          value={example} />
        <Button classes={memo(classes)}>Run</Button>
        <Button onClick={this.submit}>Submit</Button>
      </form>
    );
  }

Instead of trying to opimize InputProps, classes, etc... we encourage people to make micro-components for all of their use casses

If this is the recommended way to use MUI, we may not even need a pure mode. As you can see, as soon as you start making small helper components for your common use cases those components themselves can easily be pure components. In the example WeightTextField now never get's re-rendered as long as value is still the same, completely skipping withStyles, any memoization work necessary for InputProps, or the InputAdornment setup. And when value does change, we have to re-render TextField anyways so the non-pure InputProps={{...}} doesn't matter.

I'm fine with this path. I like micro-components in theory; though I hate every currently valid syntax/pattern for writing them that I can think of. I don't want to MyComponent = enhance(MyComponent), I want to decorate them, but you can't decorate any of the short ways of writing a tiny component. I also dislike turning import {TextField} from 'material-ui'; into import WeightTextField from '../../../ui/WeightTextField;`.

```js
let WeightTextField = ({unit, InputProps, ...props}) => (
{...props}
InputProps={{
startAdornment: {unit},
...InputProps
}}
onChange={example}
value={example} />
);
WeightTextField = pure(WeightTextField);

RunButton = compose(
withStyles(theme => ({
label: {
fontWeight: '800',
},
})),
pure
)(Button);

const SubmitButton = pure(Button);

class FormPage extends Component {
state = { example: '' };

change = e => this.setState({example: e.target.value});
submit = () => {
console.log('Submit: ', this.state.example);
};

render() {
const {title} = this.props;
const {example} = this.state;

return (
  <form>
    {title}
    <WeightTextField
      unit='Kg'
      onChange={example}
      value={example} />
    <RunButton>Run</RunButton>
    <SubmitButton onClick={this.submit}>Submit</SubmitButton>
  </form>
);

}
}
````

[wilsonjackson]

I've got a use case where I need display 500–2000 checkboxes on a page in a big list. Using native browser checkboxes, performance is fine, but using the <Checkbox> component, performance is very poor and scales linearly with the number of checkboxes on the page. Example: https://codesandbox.io/s/5x596w5lwn

I'm using mui@next — is there some strategy I can employ _now_ to make this workable?

[dantman]

@wilsonjackson
First, don't do the following. This will create a new handler every checkbox every render, which will deoptimize any PureComponent optimizations you try to do.

  handleChange = index => event => {
    this.setState({

Secondly, make your own small component to wrap Checkbox and make that component pure. This has the extra benefit that you can add in any properties that are common to all your checkboxes. And, since you have the common issue of needing a different change event handler for each item we can use a class component and do that in the component instead of in the list container.

https://codesandbox.io/s/r7l64j6v5n

[oliviertassinari]

If we want to optimize anything, these fundamental issues need to be dealt with, otherwise simply letting people enable a pure mode MUI won't actually optimize much at all. I can think of two possibilities.

@dantman These API choices have been made in order to improve the DX as much as possible while trying to be fast enough.

Instead of trying to opimize InputProps, classes, etc... we encourage people to make micro-components for all of their use casses

Yes, we do. The wrapping pattern is definitely the encouraged way to customize the library. It can be extended to apply performance optimizations. It's easier on userland where they variability of the usage of the components is much lower. We could even add an FAQ question or guide section about this point in the documentation.

[dantman]

Yes, we do. The wrapping pattern is definitely the encouraged way to customize the library. It can be extended to apply performance optimizations. It's easier on userland where they variability of the usage of the components is much lower. We could even add an FAQ question or guide section about this point in the documentation.

Ok. In that case:

• We'll want to recommend libraries like recompose that make it easy to write small stateless components. Heck, I would welcome it if someone told me there is another pattern or library that is similar to recompose but makes building pure stateless functions like recompose's patterns but is as nice as using decorator syntax
• Like we list suggestions on how to use the various autocomplete libraries and css in js libraries with MUI, we'll probably also want to suggest the various existing folder organization patterns and path hacking libraries (the ones that let you turn imports of ../../../../ui/Foo into something like something-local/Foo) that can be used to make using your own local series of micro-components wrapping MUI as nice as using import {TextField} from 'material-ui'; is and not feel like a regression in developer ease.

[wilsonjackson]

@dantman fantastic, thank you.

[Pajn]

I have multiple times needed to apply sCU due to withStyles (or rather JSS) being very slow. I don't know the code of JSS but it feels like it could be optimized quite a lot. I usually use styled-components or glamorous and thus end up with JSS and one of the other in the app and both of them outperform JSS.

While these cases might be a bit annoying they are easy to work around with app level sCU or smarter state updates. I have yet to see a single MUI component be slow enough to be problematic and I have also yet to code actually inside MUI to take significant time.

Not to say that it couldn't be faster and sure it would be nicer if less oversight was needed but at least to what I see it would be better to spend time optimizing JSS directly than MUI in that case.

[oliviertassinari]

@Pajn Thanks for the feedback. It would be really great to see some situation where withStyles performance is problematic or where it's outperformed by styled-components.

[nimaa77]

did any one checked this repo https://github.com/reactopt/reactopt ?

$ click - button (text: مقالات ) => CssBaseline,TransitionGroup,TouchRipple,TransitionGroup,TouchRipple,ScrollbarSize,TransitionGroup,TouchRipple,Ripple,TransitionGroup,TouchRipple,TransitionGroup,TouchRipple,TransitionGroup,TouchRipple,TransitionGroup,TouchRipple,TransitionGroup,TouchRipple,TransitionGroup,TouchRipple,Main,ScrollbarSize,TransitionGroup,TouchRipple,Ripple,TransitionGroup,TouchRipple,TransitionGroup,TouchRipple,TransitionGroup,TouchRipple,TransitionGroup,TouchRipple,TransitionGroup,TouchRipple,TransitionGroup,TouchRipple,TransitionGroup,TouchRipple

these components do unnecessary re-rendering just for a simple click, why just don't give a try to Should Component Update life-cycle ?

[Bessonov]

@nimaa77

did any one checked this repo https://github.com/reactopt/reactopt ?
No, I use plain why-did-you-update and chrome/react dev tools capabilities.

these components do unnecessary re-rendering just for a simple click, why just don't give a try to Should Component Update life-cycle ?

See discussion above. This doesn't fit for every use case. For me, it seems like material-ui should offer both - pure and non pure versions of components. I see huge performance improvement with pure components.

@dantman

Take a look at this example (click on Plugins in the side, search for "react-constant" and click the checkbox on "transform-react-constant-elements") and you'll see that barely anything has been optimized:

This is just one way to address this issue. There are also other options, plugins and other hand crafted optimizations. Don't get me wrong, but I'm not interested in theoretical discussions which optimizations are good or bad. I'm interested in hands on optimization which are WORKS. At least in my setup. Everything I wrote above is WORKING for me and make my app at least usable on low-mid mobile devices. Although I'm forced to do more optimizations like reordering component trees, achieved performance wouldn't be possible without pure components and other automatic optimizations. And yes, I profile and optimize very much to get this done.

[saculbr]

@Bessonov maybe we can use a prop to make the shouldComponentUpdate method to do shallow compare (https://reactjs.org/docs/shallow-compare.html) OR always return false,
this will not increase the bundle size with two version of components (pure and normal)

[Bessonov]

@lucasljj I don't expect significant increase of bundle size if this done as a wrapper for stateful component like mention above. See also profiles above: just pure components are faster than return false; in sCU.

The problem with pure components or components implementing sCU is, if you use non pure component inside it or children. See statements above. Another issue to address is theme switching. Not tested, but I think this can be overcome at least with Context API.

[mschipperheyn]

@bossonov Pure Components are considered the should-have-been-default of react that are not because they came late to the party. But I agree that most people will create redux form style wrappers mitigating the lack of it on the subtree

the problem you reference regarding pure components and children, only occurs if the top level props don't propagate to the children. Every prop or state change will trigger a rerender through the child tree up to the level that the prop doesn't propagate.
Because the idea w pure components is that it rerenders on every prop or state change. I don't know the internals intimately but I was wondering In you couldn't use a per component change property that you pass through to every child. You might even use the new context api for this purpose so as not to have to pass down a prop through the entire tree.

[Bessonov]

@oliviertassinari

Performance will be a focus post v1 release.

Great, v1 is released :) Is there any idea when performance should be addressed? I've noticed that this issue isn't part of post v1 milestone.

[oliviertassinari]

@Bessonov I think that it would be great to spend some time to update the ROADMAP. Regarding the performance. I have two things in mind actionable, but I hope to discover more:

1. We can't improve something we can't see. We need a benchmark. I have seen two interesting libraries so far:
   
   
   
   • https://github.com/paularmstrong/react-component-benchmark
   
   
   • https://github.com/Rowno/react-benchmark
   
   
2. Rendering the CSS server side requires ~30% of what React needs to render. Because we don't support style = f(props) (well, we don't support it yet: #7633). We can implement a very efficient caching capability, cutting the cost near to 0% for repeated requests. I might work on that for the office if the SSR performance harms our business metrics.
   But it's not the only options, we can also think of Babel plugins to apply the costly JSS presets at the compilation time instead of at the runtime (to balance with bundle size implications) cc @kof.

[Bessonov]

Thanks for the urls.

Fully agree with 1 and linked #4305 above.

1. SSR can help with first page load, but doesn't help with slow rerendering or cordova. While personally I can ignore first time loading on desktop or even mobile, slow rerendering still affects mobile devices after first loading.

Compile time optimizations would be great and from my perspective should be preferred, because it can improve first time loading and rerender.

Another thing is sort of documentation or example project how to use mui (and babel etc.) to get better performance.

I don't know if jss-cache can be used.

[kof]

ISTF + preprocessing pipeline will be able to shave some ms.

On Sat, May 19, 2018, 19:06 Anton Bessonov [e-mail] wrote:

Thanks for the urls.

Fully agree with 1 and linked #4305
https://github.com/mui-org/material-ui/issues/4305 above.

1. SSR can help with first page load, but doesn't help with slow
   rerendering or cordova. While personally I can ignore first time loading on
   desktop or even mobile, slow rerendering still affects mobile devices after
   first loading.

Compile time optimizations would be great and from my perspective should
be preferred, because it can improve first time loading and rerender.

Another thing is sort of documentation or example project how to use mui
(and babel etc.) to get better performance.

I don't know if jss-cache http://cssinjs.org/jss-cache?v=v3.0.0 can be
used.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mui-org/material-ui/issues/10778#issuecomment-390418709,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADOWGrCxNGqrT4MijiX8r9Ad32z6RsJks5t0FEtgaJpZM4S4woq
.

[Bessonov]

May be helpful to setup benchmarks:
https://github.com/A-gambit/CSS-IN-JS-Benchmarks/blob/master/RESULT.md

[janhoeck]

Oh react-jss (Which is used by material-ui) seems to be pretty slow

[kof]

@janhoeck it isn't, and you won't be able to prove otherwise, I am sure.

[Bessonov]

@kof I'm pretty understand that micro-benchmarks doesn't say much, but 4x-6x in comparison to aphrodite is really slow. In fact, overall performance of material-ui isn't shiny.

[oliviertassinari]

4x-6x in comparison to aphrodite is really slow

@Bessonov The benchmark methodology is key. For instance, you could try it in your browser with:
https://twitter.com/necolas/status/954024318308007937?lang=fr

The results in my browser:
⚠️ with the incertitude

overall performance of material-ui isn't shiny

We might very well be bounded by React itself and the cost of components.

[kof]

@Bessonov whatever you used to check the performance, when it comes to aphrodite its not true, because it uses asap and delays the rendering, so most benchmarks measure cpu time but not final rendering performance. That being said the bench @oliviertassinari posted is most realistic.

[kof]

Regarding MUI performance, you can get faster when you don't use any abstractions at all, but thats not the point. MUI next is pretty fast. It is so fast that you should never worry about its performance unless YOU did something entirely wrong or misused the library.

[Bessonov]

@kof no, MUI isn't fast by default. We are forced to implement workarounds to get acceptable performance. Just for sure: did't you see that it's about mobile devices and not for high end macs?

never worry about its performance unless YOU did something entirely wrong or misused the library

Well, above are some codesanbox examples. If we just use MUI components in a container and save input values in container state, like it showed in component demos, then it get unusable very fast. Wrap in PureComponent, use uncontrolled components, tree reordering, constant elements, memoize and so on helping us to maintain some acceptable performance. You are welcome to show how to use MUI right, to get better results, for example in (atm we don't use drawer anymore):
https://codesandbox.io/s/r1ov818nwm

@oliviertassinari thanks for the link. Here are results for Chrome 66 on nexus 4. As you can see the results are 10x poorer. I think in crosswalk 21/chrome 51 it can be a little bit slower:

We might very well be bounded by React itself and the cost of components.

IMHO not necessarily, as mentioned by others too. Every avoided rerender is a big win for performance and battery. Don't forget, performance is the third point on your roadmap :+1:

[kof]

See it that way: if what you do with cssinjs is too slow on a mobile device, you shouldn't be using for this react and most other stuff either. If cssinjs slows you down, react component slows you down way more. You need to rethink the level ob abstraction or apply optimizations.

[kof]

@oliviertassinari is that evtl. possible that mui wrapper rerenders jss styles on update? There might be some leak potentially that does unneeded work.

[Bessonov]

@kof

See it that way [...]

I see your point. But react and react (pure)components aren't bottleneck on it's own and performs very well. For example, MUI doesn't use PureComponent (with pros and cons described above), but they are our life safer. @oliviertassinari mentioned that there is a chance to gain better performance by using caches or pre-compiler.

Don't get me wrong, you are awesome guys and I'm really happy about every MUI release :clap: But you need consider performance too, because not every user visit websites trough high performance desktop.

[kof]

if react is not performance bottleneck and you are sure about it, JSS won't be either. The only thing I could imagine which needs to be validated if there is some unnecessary work going on and css gets regenerated on update.

Pure component is something YOU should be using in your application level code, if you need an optimization. MUI doesn't have to do it, it is a generic library and shouldn't make any assumptions. PureComponent is not always good.

JSS has considered performance from the day one and I spent endless hours in micro optimizations.

[danielo515]

This is ridiculous. I can't stop material ui components be re-rendered on every single interaction. It does not matter what I do, eve something as simple as this:

class RowText extends Component {
  shouldComponentUpdate = (nextProps, nextState) => {
    return false;
  };

  render() {
    const { title, artist } = this.props;
    return <ListItemText primary={title} secondary={artist} />;
  }
}

Triggers re renders of the underlying Typography elements. How is this even possible ?

[danielo515]

Hey, this is only happens with list and list item. Why ? Is there something that I can do ?

[eps1lon]

@danielo515 Hard to tell without the full example. The List components are also using context so if you're changing that value too it will also trigger rerenders.

[danielo515]

I understand that @eps1lon. However, could you please explain how a change in context can happen? I am not passing anything to the list component, just rendering a lost of children inside it.

[eps1lon]

@danielo515 Basically every time the List rerenders. The new context API enabled some optimization strategies we could explore by memoizing the context value.

Currently reacts context API triggers a rerender on every consumer if the the value changes WRT to strict equality. Since we create a new object in every render call to List every consumer will also rerender.

So for now an easy perf boost would be to wrap the List so that doesn't rerender as frequently. I would recommend doing benchmarks first though or bail out of render earlier. Repeated render calls Typography shouldn't be that bad.

[prevostc]

Wrapping components with React.memo works well for anything without children. I have a form with 3 ExpansionPanel and some 14 FormControl and it's lagging on desktop.

Without a solution for these performance issues, I will not be able to keep using material-ui :s

[oliviertassinari]

@prevostc it's hard to tell what's wrong without an example.

[kof]

@prevostc without seeing a codesandbox reproduction, neither us nor you can know if this is even related to this issue

[dantman]

@prevostc Create your own components that do not require children which you can then memoize.

i.e. Create your own pure/memoed MyExpansionPanel component that takes data/event props but not children and is responsible for rendering a single expansion panel. Then use that <MyExpansionPanel ... /> to render each of your expansion panels. Then re-renders will be limited to a single expansion panel (or two when transitioning between two).

[prevostc]

@oliviertassinari @kof @dantman
Here is a codesandbox reproduction of my performance issue: https://codesandbox.io/s/yvv2y2zxxx

It is a form with ~20 fields (not uncommon) you will experience some lag on user input. On slower devices, this form is just not usable.

The performance issue comes from the massive re-render on user input, but wrapping MUI components in a pure component (React.memo) does nothing as everything here has children and children this will force re-rendering AFAIK (source: https://reactjs.org/docs/react-api.html#reactpurecomponent)

Below are some screenshots of my manual benchmark, one without any optimisation, one with everything memoized and one using a custom input component with a local state to avoid setting the state on whole form too often.
On each configuration, a user input reconciliation takes around 60ms (way above the 16ms I need to render at 60fps.

Please note that I would gladly learn that I did something wrong because I love MUI and I would be sad if there was not an easy fix <3

@dantman How can you write an ExpansionPanel that do not take any React.ReactNode as input (children or props)? If you mean that I should write a specific component for every panel in my app, this is not possible unfortunately, I have too many of them.

[dantman]

@dantman How can you write an ExpansionPanel that do not take any React.ReactNode as input (children or props)? If you mean that I should write a specific component for every panel in my app, this is not possible unfortunately, I have too many of them.

Yes, instead of making a single massive component with a deeply nested tree of panels separate the expansion panel pieces out into components. It is impossible to optimize massive trees like that.

It's not impossible. React components are very lightweight. Copy and paste a block of code from a massive component into a function and you're almost done, after that you just have to hookup the props. And as long as you React.memo that function and avoid passing things that break the pure optimization of the props, then things are optimized pretty easily.

Remember, if you are creating a small component to break a chunk out of a huge component. It does not have to be something complex with its own file and own prop validation. It can be a simple functional component in the same file as the component it is used in with no propTypes. i.e. You can create small components that are only available to the component in the same file as them.

Yes, Material UI is a bit slower than low level dom elements. But even if a MUI Input were 10x slower than a raw input and things became too slow after 100. Then you still have a problem even without MUI because even if it's 10x faster the raw input would make the site equally slow if you had 1000 of them. You cannot use single monolithic components in React even when you are aren't using MUI. You need to break your app up into reasonably sized chunks so that React has defined boundaries it can optimize.

[dantman]

@prevostc Here's your demo optimized by splitting the expansion panel out into a tiny same-file component. As you can see only one expansion panel is re-rendered when an input is updated. Time is not wasted re-rendering unrelated expansion panels. If I wanted I could even do something similar to inputs that share a similar pattern, resulting in unrelated inputs also not re-rendering.

I will note that this is not just a good optimization pattern, it's a good coding pattern. In real code, where you are not making an array of inputs but explicitly writing them out with defined names, labels, and purposes. Finding boundaries and repeating patterns not only optimizes things but also reduces boilerplate making code more DRY and readable. i.e. Instead of InputWrapper I would probably break that FormControl+InputLabel+FormHelperText+Input combo into a small local SimpleTextInput component. Which would not just optimize it (resulting in unrelated inputs not re-rendering) but would also mean that the code does not need to repeat the extra boilerplate.

https://codesandbox.io/s/0o7vw76wzp

[danielo515]

Reading this I came to the conclusion that in order to optimize mui you need to create smaller specific components. That is something I already realized and tried with success. However, I also did understand that there is no way to optimize list components because the context api changes all the input props.

Regards

[prevostc]

Ok here is an updated stress test https://codesandbox.io/s/wz7yy1kvqk

I agree with @dantman on this generic point https://github.com/mui-org/material-ui/issues/10778#issuecomment-449153635 BUT I was not expecting such performance issue with this low amount of components, but bear with me as I found the source of my performance issue.

Is JSS slow ? (spoiler alert: no)

In reference to some of the earlier comments in this thread, I added a checkbox in the stress test to remove all calls to withStyles and I came to the conclusion that JSS is fast and it's not the source of the perf issue (as @kof pointed it out in https://github.com/mui-org/material-ui/issues/10778#issuecomment-396609276).

The fix

For my specific use case, I was able pinpoint the issue being that each form input was re-rendered on form update, even though only one input actually changed.
In the screenshot below, I wrapped both the FormControl and the Input in a memoized component, avoiding render if the value did not change. @dantman actually suggested that I create a specific component for each ExpansionPanel but this is a way less generic solution. As a side note, each panel is still re-rendered and performance is far from optimal but it's sufficient for now.

So? What's next?

I think that there is no way to avoid this kind of issues with a change to the material-ui code without a massive change to the current API heavily relying on composition of React.ReactNode.
But as @dantman mentioned in https://github.com/mui-org/material-ui/issues/10778#issuecomment-449153635, MUI is a bit slower that what it expected from it. Not addressing this at all is a mistake IMHO.

Being aware of this issue, we may need to create a doc page related to performance issues and how to address them. Even if his page will mainly redirect to the official doc (https://reactjs.org/docs/optimizing-performance.html) and list components that might cause perf issues, it's a start.
It would be better to console.warn the user when such issue occur but I can't figure out a way to detect issues at the material-ui level.

[kof]

@prevostc this message made my day, this is the kind of community I love. Do you have ideas what mui could change to make performance better and avoid the need for user land optimizations? An API change might be doable.

[prevostc]

I do not :s

I do not know MUI's internal enough to have any idea how to improve it's raw performance (without api change) right now. I'm working on some ideas but nothing conclusive as of today: I have an app where a radio group is re-renderer when it's direct parent is not, cannot re-produce this locally yet.

Any API change would be to consider removing any React.ReactNode props from the API (children and other props like icon, etc) but I could not find a good way to keep the same configurability.
Here is what I tried: https://codesandbox.io/s/jpw36jw65 (warning: not finished).

Also it is something to note that MUI is especially slow while on development mode due to react being especially slow in development mode. I don't know if there is a way to improve on this.

[mkermani144]

Is there any progress on adding capabilities (like the one @Bessonov suggested) in order to optimize the performance issues Material-UI currently faces?
When we started using this great library for our project, I didn't know such performance issues may happen when the project gets larger and larger; in addition, I didn't see any section in Material-UI docs informing me about the cases that may cause Material-UI to become slow and harm UX.
Lots of performance problems - directly or indirectly related to Material-UI - are reported in this issue. I think it will be a good idea to list them in another issue so that everyone can track the progress. If you think it's OK, I can open a new issue.

[oliviertassinari]

@mkermani144 We have yet to see a performance report directly correlated to something Material-UI is doing wrong. So far this issue has been used as a help forum for people struggling. Do you confirm that nothing actionable was reported? I'm going to state the obvious, React abstraction has a cost. Each component wrapping a host will add weight to your render tree, it slows it down. While you can render more than 100 list items with native host, it starts to be an issue when you wrap them with a class component. It's not specific to Material-UI.

Table

Dev mode

Let's take the example of a table. It's a component people find slow. We have documented the virtualization, it helps, a lot.

In the following test case we render 100 items in dev mode. We can consider the following cases:

1. Table Raw: https://codesandbox.io/s/v066y5q7z3: 63ms in the render
2. Table Material-UI Master: https://codesandbox.io/s/m4kwmvj9ly: 250ms in the render
3. Table Material-UI Next: https://codesandbox.io/s/2o35yny1jn: 262ms in the render

So the overhead of using Material-UI in dev mode over host element is around x4 (the difference is smaller in production!), simply because we create intermediary components. It's why virtualization starts to be important after render a list of ~100 table items. Now, we can dive a bit into Why and what can we do about it?

1. Table Raw + function component. Why a functional component? We want to abstract the class names used. We don't want the component users to repeat them.
   https://codesandbox.io/s/1zl75mwlpj: 105ms in the render
2. Table Raw + function component + forwardRef. Why forwardRef? We want the component to be "transparent", being able to access the host element with a ref.
   https://codesandbox.io/s/32o2y0o9op: 120ms in the render
3. Table Raw + function component + forwardRef + withStyles. Why withStyles? Because we want to style our component:
   https://codesandbox.io/s/j2n6pv768y: 200ms in the render
4. Table Raw + function component + forwardRef + makeStyles. makeStyles might be faster than withStyles, let's try:
   https://codesandbox.io/s/yw52n07l3z: 130ms in the render.

So, we have one leverage available: migrate all the component from withStyles to makeStyles. We can win about +30% of performance (262 / (262 - 70)) in dev mode.

Production mode

I have run the same test cases in production mode:

• n°1 30ms in the hydrate
• n°3 106ms in the hydrate
• n°4 40ms in the hydrate
• n°5 41ms in the hydrate
• n°6 80ms in the hydrate
• n°7 57ms in the hydrate

So the withStyles to makeStyles migratation is still a +30% speedup in theory in production mode.

[oliviertassinari]

If you think it's OK, I can open a new issue.

@mkermani144 If you have a specific case were Material-UI is doing it wrong, sure.

[mkermani144]

I read about all of the comments below this issue. My problem doesn't fit in any of the other ones mentioned earlier.

I have a List component containing some ListItems, having one of them selected and highlighted. When I click another ListItem to select and highlight it, the whole list (containing its children) gets re-rendered again.

I know that this problem may seem exactly the same as a previous comment, but it's not; at least I think so.

Let's look at the results from React profiler:

As you can see, I have a MyList component at the top level of the image. This component is only a wrapper around MUI List and just makes it pure, that is:

class MyList extends React.PureComponent {
  render() {
    return (
      <List>
        {this.props.children}
      </List>
    );
  }
}

I added this wrapper because @eps1lon in one of his comments mentioned that re-rendering List causes context to update, and this context update makes all consumers (including ListItems) to re-render too.

I also tried making all my ListItems pure and profiled the app again. The results were the same, except that my custom component (i.e. MyListItem) didn't re-render _itself_, but all the children below it __did__.

I know the problem occurs because the context MUI uses for styling re-renders _somehow_. But I don't know why this re-render happen and how I can avoid it.

Or, am I doing something wrong?

Note: I use MUI new (alpha) styling solution, that is, @material-ui/styles. I don't know if this matters.

[oliviertassinari]

@mkermani144 Remove Material-UI with native elements, observe that the re-rendering is still present. The pure logic won't help like this. React.createElement creates new references at each render, it invalidates your PureComponent.

[mkermani144]

Yes, I know elements are objects, and objects are not strictly equal in Javascript, so sCU fails.

But I don't understand what you mean when you say React.createElement is called again. Which call to createElement? If you mean the calls inside List, it only calls createElement for its children (ListItems) only if it is re-rendered. If it's not re-rendered, no createElement will be called and no re-render should happen. The problem is that the List itself gets re-rendered.

[oliviertassinari]

@mkermani144 If you can create a minimal reproduction example, we can have a look at it.

[Pajn]

Your MyList (and thus List) gets rerendered because of the component that renders MyList (lets call it MyComponent) gets rerendered. PureComponent on MyList does not help as MyComponent have been rerendered and created new children for MyList so that MyLists check fails.

Your MyComponent probably gets rerendered because there is where you store the state of which item is selected.

I think MUIs implementation of List should change to not recreate the List context value each render
here: https://github.com/mui-org/material-ui/blob/fb4889f42613b05220c49f8fc361451066989328/packages/material-ui/src/List/List.js#L57

So instead have List look something like this:

const List = React.forwardRef(function List(props, ref) {
  const {
    children,
    classes,
    className,
    component: Component,
    dense,
    disablePadding,
    subheader,
    ...other
  } = props;
  const context = React.useMemo(() => ({ dense }), [dense]);

  return (
    <Component
      className={clsx(
        classes.root,
        {
          [classes.dense]: dense && !disablePadding,
          [classes.padding]: !disablePadding,
          [classes.subheader]: subheader,
        },
        className,
      )}
      ref={ref}
      {...other}
    >
      <ListContext.Provider value={context}>
        {subheader}
        {children}
      </ListContext.Provider>
    </Component>
  );
});

That would simplify creating sCU versions of the ListItems

[mkermani144]

Your MyList (and thus List) gets rerendered because of the component that renders MyList (lets call it MyComponent) gets rerendered. PureComponent on MyList does not help as MyComponent have been rerendered and created new children for MyList so that MyLists check fails.

@Pajn No, look at my React profiler results. MyList didn't re-render (it's grey), but List did (it's blue). I don't persist on PureComponent for MyList. Even though I implement sCU for MyList so that it doesn't re-render, List __does re-render__.

[mkermani144]

@oliviertassinari
I created a minimal reproduction example:

import React, { Component } from 'react';

import StylesProvider from '@material-ui/styles/StylesProvider';
import ThemeProvider from '@material-ui/styles/ThemeProvider';

import { createMuiTheme } from '@material-ui/core';

import List from '@material-ui/core/List';
import ListItem from '@material-ui/core/ListItem';

const theme = createMuiTheme({});

const MyListItem = React.memo(ListItem, (prev, next) => prev.selected === next.selected);

class App extends Component {
  state = {
    selected: null,
  }
  render() {
    return (
      <StylesProvider>
        <ThemeProvider theme={theme}>
          <List>
            {[0, 1, 2, 3, 4].map(el => (
              <MyListItem
                button
                selected={el === this.state.selected}
                onClick={() => this.setState({ selected: el })}
              >
                {el}
              </MyListItem>
            ))}
          </List>
        </ThemeProvider>
      </StylesProvider>
    );
  }
}

export default App;

React profiler results (after clicking on the 4th list item):

As you can see, it works as expected, i.e. there is no extra re-renders (except for ButtonBase components inside ListItems). The problem is that, this reproduction is _too minimal_; there are lots of stuff i skip in it.

I know you cannot tell me what is wrong with my code causing extra re-renders. But I ask you a question: What can cause a re-render in the WithStylesInner components which wrap MUI components?

[oliviertassinari]

@mkermani144 What do you think of this fix?

--- a/packages/material-ui/src/List/List.js
+++ b/packages/material-ui/src/List/List.js
@@ -40,6 +40,13 @@ const List = React.forwardRef(function List(props, ref) {
     ...other
   } = props;

+  const context = React.useMemo(
+    () => ({
+      dense,
+    }),
+    [dense],
+  );
+
   return (
     <Component
       className={clsx(
@@ -54,7 +61,7 @@ const List = React.forwardRef(function List(props, ref) {
       ref={ref}
       {...other}
     >
-      <ListContext.Provider value={{ dense }}>
+      <ListContext.Provider value={context}>
         {subheader}
         {children}
       </ListContext.Provider>

Do you want to submit a pull request? :) We are using the same strategy with the Table component. It works great. Thank you for reporting the problem!

[mkermani144]

@oliviertassinari Sure. This is exactly what @Pajn suggested earlier. I submitted a PR.

[mkermani144]

14934 is merged; the List component, however, may become a performance bottleneck if it's large enough, no matter how much it's optimized. Shouldn't we provide an example showing the usage of react-window or react-virtualized with List component, like the one we have in Table docs?

[eps1lon]

Shouldn't we provide an example showing the usage of react-window or react-virtualized with List component, like the one we have in Table docs?

That'd be great :+1:

[henrylearn2rock]

Fwiw I built a chat app and the app needs to render a large list of contacts. I ran into the same issue
@mkermani144 had.

https://stackoverflow.com/questions/55969987/why-do-the-children-nodes-rerender-when-the-parent-node-is-not-even-being-update/55971559

[oliviertassinari]

@henrylearn2rock Have you considered using virtualization? We have added a demo for the list: https://next.material-ui.com/demos/lists/#virtualized-list.

[pytyl]

This also really tripped me up. I think most people (including me) assumed everything under a pure component was safe from a rerender, which evidently isn't the case for this library. I am going to try virtualization as you most recently suggested. Thanks!

[eps1lon]

I think most people (including me) assumed everything under a pure component was safe from a rerender, which evidently isn't the case for this library.

This is not how React.PureComponent or React.memo works. It only affects the the component itself. Children might still have to re-render if context changes.

@pytyl Can you share the code where you used a PureComponent and expected it to prevent any re-render in its sub-tree?

[pytyl]

@eps1lon the following documentation makes it seem like returning false from shouldComponentUpdate automatically skips re-render in children components.
https://reactjs.org/docs/optimizing-performance.html#shouldcomponentupdate-in-action

Since shouldComponentUpdate returned false for the subtree rooted at C2, React did not attempt to render C2, and thus didn’t even have to invoke shouldComponentUpdate on C4 and C5.

Maybe I am mistaken on this? The following is a snapshot of my profiler. Just for the sake of testing, I explicitly return false for shouldComponentUpdate in my Menu component:

This made all of my children components (Categories, Category, CategoryItems, CategoryItem) not re-render. Many MUI related things appear to be re-rendering at the bottom which seems to be causing a lot of delay. Stuff like withStyles, Typography, ButtonBase. Still a bit new to React so please excuse my ignorance. Below is my code for Menu component (where I am returning false for shouldComponentUpdate):

import React, { Component } from "react";
import Categories from "./Categories";
import { withStyles, Paper } from "@material-ui/core";

const styles = theme => ({
  root: {
    paddingTop: 0,
    marginLeft: theme.spacing.unit * 2,
    marginRight: theme.spacing.unit * 2,
    marginTop: theme.spacing.unit * 1
  }
});

class Menu extends Component {
  shouldComponentUpdate(nextProps, nextState) {
    if (nextProps.categories.length == this.props.categories.length) {
      return false;
    }
    return true;
  }

  render() {
    const { classes, categories } = this.props;

    return (
      <Paper className={classes.root}>
        <Categories categories={categories} />
      </Paper>
    );
  }
}

export default withStyles(styles)(Menu);

[eps1lon]

I would need a full codesandbox to understand the issue.

[pytyl]

@eps1lon I will try to produce one for tomorrow. Thank you.

[pytyl]

@eps1lon here is the codepen:
https://codesandbox.io/s/348kwwymj5

Short description
It's a basic menu app for restaurants (which often have upwards of 100 menu items). When the user clicks on a menu item, it opens an "add to order" dialog. I will attach a few situations where the profiler is showing poor performance (these statistics are not on a production build).

System
MacBook Pro (Retina, 13-inch, Early 2015)
3.1 GHz Intel Core i7
Firefox 66.0.3

Case 1 (user clicks on a menu item)
Render duration: 218ms

Case 2 (user clicks add to order button in dialog)
Render duration: 356ms

I'm sure I'm making some novice mistake here so any guidance is greatly appreciated.

[Pajn]

As WithStyles(ButtonBase) is rerendered I assume that WithStyles uses a context that is recreated even though it doesn't need to.

I managed to find this https://github.com/mui-org/material-ui/blob/048c9ced0258f38aa38d95d9f1cfa4c7b993a6a5/packages/material-ui-styles/src/StylesProvider/StylesProvider.js#L38 but can't find a place where StylesProvider is used in actual code (GitHubs search isn't very good) but it might be the reason.

Does @eps1lon know if this might be the cause? If it is, a useMemo on the context object would probably fix this. Though I don't know if the localOptions are stable or if useMemo needs to be propagated even further.

[eps1lon]

Maybe. But you should first investigate why your component using a StylesProvider rerenders. This is either something at the top of your tree or should be at some UI boundary. In either case those should rarely rerender. Remember that react context isn't optimized for frequent updates anyway.

We shouldn't prematurely optimize these things just because some object is re-created during render. Memoization is not a silver bullet. So without a concrete example I can't do much. Yes things re-render. Sometimes more often then they need to. But a wasted re-render doesn't mean it's the cause of a performance bottleneck.

[oliviertassinari]

@pytyl I have looked at your codesandbox, there is a problem with the rendering architecture. You rerender everything when the menu item is clicked. Your GlobalContext jumps the pure logic.

@eps1lon I think that we should close this issue. It would be better to focus on specifically identified issues.

[eps1lon]

TL;DR: Create context slices, memoize context values, no issue with material-ui specifically: https://codesandbox.io/s/8lx6vk2978

Did some digging and the issue is that you have this big global context that is re-created during render. You re-render your App when you click at which point the global context is re-created. Your CategoryItem is listening to it which appears 100 times in your App. Since you have 100 material-ui MenuItems you encounter the classic death by a thousand cuts.

So ironically part of the solution is memoizing a context value but the important part is identifyng separate context slices. It seems like a state and dispatch context is appropriate. This is recommended when using useContext with useReducer and seems to fit here to.

This can create quite a big tree and render props hell the more contexts you have. I encourage you to have a look at useContext. It will help alot if you start facing these issues.

@oliviertassinari It's good issue to collect common pitfalls with solutions. We can decide if we want to create separate issues from it.

[pytyl]

@oliviertassinari @eps1lon thanks for revising! Performance seems great.

[mankittens]

I just had a problem with slow rendering performance. I solved it entirely by replacing all instances of the <Box> component with <div>s. I debugged using the react devtools flamegraph, and I went from around 420ms to 20ms.

With <Box>es;

Without <Box>es:

[oliviertassinari]

@mankittens You could keep the Box component, using styled-components as the style engine. The performance would be much better. It should get better with JSS in the near future https://github.com/mui-org/material-ui/pull/16858.

I'm closing this issue. We need a dedicated performance report for each potential area of improvement, not an umbrella thread.