Performance of hooks vs connect

[mikeyhogarth]

tl;dr; changing to hooks caused noticeable animation janking in my application

So this completely contravenes all of your guidelines about what to raise as an issue, however I got chatting to one of your maintainers on reddit and they asked me to raise it here. The short version is: Swapping to hooks caused a noticeable performance dip in my application.

I am working on a little pet project over here (caveat: I am not the greatest React developer in the world - if any of this is due to my poor practice then please feel free to point it out);

https://github.com/mikeyhogarth/cocktails/

It's basically just an interface that lets you browse/filter IBA cocktails. The spark that made me start working on this was to play around with React Hooks (there isnt a single class in the whole app) - so when I found out that react-redux had started officially supporting hooks, I jumped on and swapped all my HOC's to hooks that very evening.

Initially I just went through and swapped out all my connects to useSelectors/useDispatch hooks.

Upon building though it was pretty obvious there had been a quite noticeable dip in performance. You can particularly see it if you toggle the "only show things in my bar" switch at the top of the first page - pretty smooth on master, but very janky on the new branch. I swapped back/forth several times to make sure I wasn't imagining it and a quick check of the react devtools confirmed that many more re-renders were happening as a result of this change.

Other observations

• The problem becomes largely un-noticable when it's built for production
• The problem went away completely when I React.memod everything
• Your colleague over on reddit suggested that this might be because I might have lots "nested components that access the store" - this is absolutely true, I do. I always prefer connecting over props drilling unless there's a good reason.
• This might be a problem entirely unique to Material UI where animation janking becomes very noticeable - otherwise I'm sure you all would have noticed this during development.

Resources/Info

• The repository itself
• The branch that has hooks in it
• A PR showing the changes made in that branch
• A live deployment of master
• (sorry there is no live deployment of the janky branch but I can make that happen if it'll help)

package.json

    "@material-ui/core": "^4.0.0",
    "@material-ui/icons": "^4.0.0",
    "lodash": "^4.17.11",
    "react": "^16.8.6",
    "react-circular-progressbar": "^2.0.0",
    "react-dom": "^16.8.6",
    "react-redux": "^7.0.3",
    "react-router-dom": "^5.0.0",
    "redux": "^4.0.1",
    "redux-thunk": "^2.3.0"

Opening the discussion

+116 −167 code added/removed when I made this change, which I was really pleased with - the change also identified several store connections that I wasn't actually using (remnants of an earlier refactor that were previously invisible to the linter) - after promoting them to hooks I started getting warnings about un-used variables - again, another advantage of this syntax over the mapState pattern.

Ultimately though I am probably not going to make the switch as it introduces a performance concern that I have up to now not really needed to worry about - my app was just fast by default when I used connect and it's not after switching.

Thanks for all of your efforts in making this library such a vital tool for modern web development - I hope you don't take any of this as a moan or criticism - just trying to open the discussion as I think it's something that's going to be affecting quite a few developers.

I am up for helping any way I can.

[timdorr]

As we mentioned in the performance section for the Hooks API, you should be adding React.memo to most components. This is something connect did for you, but we are unable to do in Hooks.

You should also read up on memoizing your selectors, as doing so is another key part of getting performance out of Hooks (and not just ours, but many other Hooks, including the built-ins).

There is nothing inherently slow with Hooks or our implementation, just that there is more onus put on the user to make things fast. It's something we're all going to have to get better about.

[mikeyhogarth]

The docs don't say "you should be adding React.memo to most components", they say "If further performance optimizations are necessary, you may consider wrapping your function component in React.memo()" - IMO lots of people are going to get bitten by this so I wonder if maybe re-wording those docs to put a bit more emphasis might help.

The "more onus on the user to make things fast" is the reason I'm not going to use hooks for redux - at least not until someone figures out a pattern to use them without having to worry about how many re-renders you're going to be inviting in. I don't really want that responsibility if the HoC gives me it for free. I have no doubt that this is a hooks-wide problem so I think you're right - the implementation is probably fine but IMO the docs need a bit of refining.

Thanks for the tip about memoizing selectors - I will take a look.

[markerikson]

Yeah, at a minimum we're going to need to have a more meaningful section on hooks when we finally get around to adding a "Performance" page.

For this specific case, I think most of the functions in cocktail.utils.js should be turned into memoized selectors using Reselect or similar, and then you'd want to pass those to useSelector() so that the component only re-renders if the derived data has changed.

Please see my post Idiomatic Redux: Using Reselect Selectors for Encapsulation and Performance for explanations.

[mikeyhogarth]

Thank you :) the term "selector" is literally one I only started hearing two days ago despite having used redux for years so I guess I have some reading to do!

[mikeyhogarth]

@markerikson this was dynamite advice - I have just been through and added reselect to the whole app and am starting to wonder where this library has been all my life :) I'll probably now re-do the hooks example to see if it's made a difference to the performance.