-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New room list: large account performance #14035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
For element-hq/element-web#14035 **This option is not recommended as it completely obliterates all chances of being able to support someone with a broken room list. It is intended for specific testing scenarios only.**
With the above PRs merged, performance is looking a lot better: At 37.77ms per operation, most of which is 1-2ms function calls, we're doing pretty good. Incremental updates are well down from 100ms to just 3.15ms on average (previous metrics: matrix-org/matrix-react-sdk#4253). The remaining ~35ms are spent preparing for a render of the update which just took place. We do still fire a boatload of updates though (as indicated by the flamegraph, which is only a tens of seconds of time). When we disable all the logging, incremental generation falls by nearly a full millisecond. Initial generation takes just 3.5ms on my account, without logging. With logging it's a bit longer but not by much. Most of the rendering time appears to be in the depths of React - we have some control over this but overall the benefit of going into it much further isn't totally worth it at this point. Although the numbers tell an amazing story, we're leaving this issue open until others with large accounts (Matthew) are happy with the overall performance. It's not expected to be perfect, but should be much closer to perfect than unusable. |
for comparison: the old room list achieved 394ms initial generation and 1.5ms incremental, with the list before that getting 0.13ms initial and 0.26ms incremental. The old old room list (~2018) got such great speeds because it wasn't doing anything terribly complicated, but with the "breadcrumb" (now called importance) algorithm there were a number of complex features introduced like sticky rooms and advanced sorting behaviours. When comparing the old room list versus this new one, the initial generation time is much better because we're tracking rooms differently - this is also what consequently increases the incremental time. We spend slightly more time making sure we're going to put a room in the right spot to make subsequent updates If we remove some safety code, we could probably shave a millisecond or two off the incremental time, however at a cost of a couple milliseconds I think it's worth keeping it in. |
This got the seal of approval I was looking for: "it feels better". |
It's really not feeling better from my side. Admittedly, I am not running the latest and greatest hardware but I don't believe we are targeting those users anyway. I'll gather logs. EDIT: Profiling information can be found at this permalink The symptoms I'm seeing are incredibly slow app performance when scrolling, or clicking on a room. When searching for a room, the performance is near enough to how the old Riot behaved (i.e. showing a subset of the rooms gives good perf). Showing all rooms is abysmal however. |
Please open new issues instead of reopening ones, it ruins our entire tracking process: #14539 |
Follow on from #14034 - make it not noticeable for large accounts.
The text was updated successfully, but these errors were encountered: