PERF: speed up about page render time and limit category mods (PR #8112)

Context: About page hangs/slow to load - bug - Discourse Meta

I’ve tried 2 different approaches here to improve the render time, one using widgets and the other using strings concatenation to produce the same HTML as the user-info component. I’ve found the strings approach to be about 34% faster than the widgets approach. On my phone, rendering 300 users was taking on average ~520 ms using the strings approach, whereas the widgets were taking ~790 ms on average to render the same amount of users.

This also limits the number of category moderators that are displayed on the page to 300. If there are multiple categories with moderators, then the limit is divided between the categories. So if there are 3 categories, each one will display up to 100 moderators ordered by last_seen_at in descending order.

GitHub

You’ve signed the CLA, OsamaSayegh. Thank you! This pull request is ready for review.

would it make any change to store the result of userPath(user.username) as you use it two times ?

@eviltrout thoughts here?

I wonder if we should just do something simpler and only render up to a maximum N users on the about page instead of adding a new rendering pipeline for groups of users?

I am overall ok with this fix bug a big concerned we need a special rendering thing here.

I wonder if we should just do something simpler and only render up to a maximum N users on the about page instead of adding a new rendering pipeline for groups of users?

Note this PR does introduce a limit of 300 to the number of category mods that we render on the page. Right now it takes my phone 3 seconds to render 300 users (using the user-info component for each user). Did you mean to have a more aggressive limit? 3 seconds seems pretty bad to me, and it may be a lot slower on other phones.

Maybe a simpler limit of 100 users on the whole page, can we speed up the user info component?

Sure, I can bump the limit to 100. I looked up ways to improve render speed and couldn’t find anything that would make a meaningful difference, maybe @eviltrout and @jjaffeux have some ideas?

Maybe instead of using this new special rendering method, I could create a raw hbs template and use it here? I’ll benchmark it and see how it performs.

Can you try this patch please?

about-page-perf-joffrey.patch.zip

I’m unsure about how you do your benchmark, so can’t exactly compare, but this seems reasonably fast for a big list of users, and still uses regular hbs file.

FWIW, here the culprit is all the formatUsername, userPath, renderAvatar, escapeExpression calls… Not exactly the template rendering.

Screenshot 2019-10-02 at 16 30 11

We could maybe look at optimising these if that’s actually an issue.

I would vastly prefer if we didn’t make strings like this unless absolutely necessary.

(For the record I am not surprised widgets are slower - widgets are meant to be updated and re-rendered and that’s where they truly excel.)

I think @jjaffeux 's solution is excellent so if that is fast enough let’s go with that.

1 Like

I benchmarked @jjaffeux solution and sadly it turns out it’s slower :cry: It took ~1.6 seconds to render 300 users on my phone. However, since we’re going with a limit of 100 users on the page, this may allow us to avoid the strings approach (which I’m not married to just to be clear) in favor of cleaner code but slightly slower?

About is not a common path, so I think we prefer cleaner code. Is it ~ 0.5s for Joffrey’s with 100 records? We can live with that considering that visiting the page is rare.

what phone do you have btw?

If you don’t have a recent iphone, and this is 1.6s on 300 users with a “slow” phone, I think this is totally fine.

This is total edge case here: rare page, slow device and rare condition of a lot of admins

@eviltrout I’m getting 0.8-0.9s for 100 users, which doesn’t seem proportional because there are other elements on the page (I’m measuring how long the entire page takes) which take about 0.3s. If I subtract that from the render times the math starts to add up.

But like Joffrey said, it’s rare to have a 100 users on /about, so I think this is good enough.

To answer your question @jjaffeux I have a Huawei Mate 10 Lite.

1 Like

So yes, not a great phone. Think it will be fine for 99% of people at least.

I think we should merge the Joffrey approach then. We don’t need to spend forever on this page that I think is rarely visited anyway!

1 Like

Ok, I’ve applied Joffrey’s patch and made the limit 100. Will merge this shortly :rocket: Thanks everyone!

1 Like