nileane / TangerineUI-for-Mastodon

A Tangerine redesign for Mastodon's Web UI. šŸŠšŸ˜
https://nileane.fr/@TangerineUI
MIT License
399 stars 27 forks source link

Use of has([attribute=""]) selector triggers serious performance issues in Blink engine #133

Open thomas-pike opened 3 days ago

thomas-pike commented 3 days ago

Hi

We've been investigating a very noticable performance issue on our instance that can only be seen when a user uses one of the TangerineUI themes.

The symptoms are that when viewing a post with a large number of replies (this is the case that caused us to investigate) in any Chromium-based browser, the page will initially load fine without the replies, but then the tab will freeze up and become entirely responsive for 30 seconds or more (using 100% CPU) while the replies are rendered.

Our team's investigation revealed this to be down to a combination of factors in TangerineUI, Mastodon, and Chromium. Here is the full summary from my colleague:

[TangerineUI] uses a CSS :has selector with an attribute selector in it. The :has selector causes layout to have to backtrack through the DOM, which is inherently slower than normal stepping through the DOM (like :last-child), and it seems this particular combination manages to prevent Chromium from optimising it.

Then the [Mastodon] JS code checks the .clientHeight of every comment that it adds, to see if that comment should be collapsed. That check forces a layout to take place, and while that would normally be cached by the engine, it seems the selector prevents that (and quite possibly also the reflow in between of adding the comment to the DOM, instead of doing it for all comments in a single action after all the checks are done on all of them). When you have 400 comments, this bottleneck gets hit 400 times, and that is about a 0.5 second delay per .clientHeight check.

([The Mastodon devs] knew about this, and thought they had fixed it by batching and caching the result. But it is definitely not good enough on initial load.)

The selector in question that triggers the performance issue is this one: .app-body .navigation-panel hr:has(+ .column-link[href="/settings/preferences"]) and my colleague also found that reducing the selector to simply p:has([href="/settings/preferences"]) is enough to trigger the performance issue.

I don't know if there's a better way to express this selector - obviously I know that you are working within the constraints of the HTML provided by Mastodon, but if it is possible to avoid it then it would be a large help. I hope we've managed to provide enough information to be helpful. I will also be filing a similar bug report to the Mastodon team.

Thank you for your continued work on this wonderful theme.

thomas-pike commented 2 days ago

The Mastodon issue report is here: https://github.com/mastodon/mastodon/issues/33031