MM-30407: improve slow typing

[lieut-data]

Summary

Improve the overall performance of the (new) sidebar by improving the ability for makeGetChannel to /actually/ memoize instead of constantly recomputing.

Phew, this one was a rabbit hole. First, let's look at a baseline performance profile of a user with lots of DMs open, and a "background user" that's posting to town square every second:

Keep in mind that those aren't solid blocks:

Essentially, every single entry in the sidebar is being forced through a React reconciliation. While the final DOM doesn't actually need to change, the cost for React to do the diff is significant. The reason this happens is because the channel prop passed to this component is spuriously changing. The corresponding makeGetChannel selector in this repository is memoized, but two things effectively force it be recomputed all the time:

• dependency on getAllChannels -- this means that any channel changing for any reason (i.e. the last post time changing from a new post) invalidates the caching here forcing the selector to run again
• dependency on state.entities.users -- similarly, this means that any property related to any user invalidates the cache (i.e. user status changes).

Now even if the memoization is failing, we might expect React to shallow compare the output of the selector and realize nothing has changed, right? Unfortunately, all DMs and GMs are piped through completeDirectChannelInfo, returning a new object every single time.

Fixing this /properly/ is non-trivial. We are almost certainly overusing reselect, memoizing things that recompute all the time anyway, and so only adding cost instead of saving it. Many of our selectors aren't "pure", often returning new objects (as noted above), but sometimes accidentally by returning an empty object {} (for which {} !== {}). There are still some reducers mutating the entire object (i.e. user statuses). And we could certainly look at reorganizing the actual structure of the state, leveraging reducers at action time instead of render time (but still keeping the data normalized).

This PR is an attempt to work around the immediate pain points while we build up momentum to double down on these kinds of performance issues. It was really hard to fix completeDirectChannelInfo returning a new object without touching waaaay too much code, so I focussed instead of narrowing the dependencies of the memoized makeGetChannel. The resulting changes (in this PR) improved things considerably:

Note that the CPU is no longer maxed out all the time, making space for important things like, you know, typing. But it was still costly. The next culprit proved to be the SidebarChannelMenu. It looks like this component is rendered all the time for every channel, even when not visible. Looking at its dependencies, I realized that getDisplayedChannels was constantly changing, but this led down a much deeper rabbit hole. Was there a way around this? See the linked WebApp PR, but I opted to just not render the menu until it opened -- and from what I can see, without any negative side effects. (Hoping Dev & QA review can help confirm this!)

The resulting performance graph:

This is far from optimal, but I think gets us over the hump.

Ticket Link

Fixes: https://mattermost.atlassian.net/browse/MM-30407

[enahum]

@lieut-data this is surely also affecting the mobile app even with the old sidebar as we use these selectors there, because of the fact that I'm currently without a computer I cannot really test this, perhaps @migbot can help with that. This seems to be a fix that if indeed helps with perf by correctly memoizing, the mobile app can benefit a lot from.

[migbot]

@lieut-data this is surely also affecting the mobile app even with the old sidebar as we use these selectors there, because of the fact that I'm currently without a computer I cannot really test this, perhaps @migbot can help with that. This seems to be a fix that if indeed helps with perf by correctly memoizing, the mobile app can benefit a lot from.

Yep, I'll apply these changes on the mobile repo tomorrow and test.

[hmhealey]

Honestly, makeGetChannel is a problematic function that I've wanted to remove. I get that attaching the status and teammate to the channel and having the proper display name for DMs/GMs is helpful, but seeing as that causes the object to be recreated when anything changes for any user (which is still the case even with these changes since profiles changes), I don't think it's worth it to do automatically. If we split those off into separate selectors and had most places use plain getChannel, I'd think we could improve things a lot

[devinbinnie]

Probably don't need QA review on this PR, will save it for the webapp one.

[lieut-data]

Honestly, makeGetChannel is a problematic function that I've wanted to remove.

Yes! This sounds awesome @hmhealey and fits exactly into what I'm hoping we can focus on in Q2 :)

[migbot]

@lieut-data this is surely also affecting the mobile app even with the old sidebar as we use these selectors there, because of the fact that I'm currently without a computer I cannot really test this, perhaps @migbot can help with that. This seems to be a fix that if indeed helps with perf by correctly memoizing, the mobile app can benefit a lot from.

Yep, I'll apply these changes on the mobile repo tomorrow and test.

No noticeable changes when testing on Android, however, I probably don't have enough channels in my test user's sidebar. Will wait until this is merged into mm-redux to apply the changes in the mobile repo then put up a PR for QA to test.

[lieut-data]

@migbot, keep in mind too that you'll need background user activity (like regular posts in a channel). I simulated this with a plugin posting every second.

[hmhealey]

Is that plugin available somewhere? A configurable "activity bot" would be great for performance testing the UI.

[lieut-data]

@hmhealey, the plugin isn't available anywhere yet -- it was really only about 10 minutes of effort -- but I agree this could be useful and will iterate.

[lieut-data]

Chatted with @jgilliam17 and we feel good about merging this to unblock mattermost-redux squashing efforts, with the remaining QA continuing on the corresponding webapp PR.

[lieut-data]

/cherry-pick release-5.33

[mattermod]

Cherry pick is scheduled.

[lieut-data]

/cherry-pick release-5.31

[mattermod]

Cherry pick is scheduled.

[lieut-data]

/cherry-pick cloud

[mattermod]

Cherry pick is scheduled.