feat: Cache status height to avoid expensive renders#4439
feat: Cache status height to avoid expensive renders#4439Gargron merged 4 commits intomastodon:masterfrom sorin-davidoi:cache-status-height
Conversation
|
Would be nice if this could be deployed on a test instance to make sure nothing breaks. |
|
@sorin-davidoi aye aye |
|
Has this now been confirmed to not break stuff? |
|
Not yet. If this will receive extra testing I've also moved the formatting logic into the reducers (no need to call |
| overflow-y: scroll; | ||
| overflow-x: hidden; | ||
| flex: 1 1 auto; | ||
| backface-visibility: hidden; |
There was a problem hiding this comment.
Creates redundant layers that the browser has to keep in the GPU memory.
| import { Map as ImmutableMap, fromJS } from 'immutable'; | ||
| import escapeTextContentForBrowser from 'escape-html'; | ||
|
|
||
| const domParser = new DOMParser(); |
|
Think this is enough for this PR. @beatrix-bitrot maybe you can take this for a test pretty please? |
|
What's the status of the PR? Can we merge it or do we still want to test it first? |
|
I'd still want some testing first. |
|
@sorin-davidoi I deployed this on my staging server and does some test. |
* Revert "Adjust tags and accounts page (#4534)" This reverts commit a3e53bd. * Revert "feat: Cache status height to avoid expensive renders (#4439)" This reverts commit 8eb6d17. * Revert "Refactor Avatar and AvatarOverlay to have 'account' as prop instead of src and staticSrc (#4526)" This reverts commit 5942347. * Revert "Update dependencies for Ruby (#4543)" This reverts commit 22db947. * Revert "[Docker] Add multicore support to "make" and "bundler" (#4544)" This reverts commit 5d408fd. * Revert "It makes no sense to try using invalid or expired link again (#4521)" This reverts commit 47579ec. * Revert "i18n: Update Polish translation (#4545)" This reverts commit 3363a05. * Revert "Set false to animated options for thumbnail processor (#4547)" This reverts commit 87f10d4.
…set of status updates in order to limit the maximum size of a status in a list view (e.g. the home timeline), so as to avoid having to scroll all the way through an abnormally large status update (see mastodon#8205), the following steps are taken: •the element containing the status is rendered in the browser •its height is calculated, to determine if it exceeds the maximum height threshold. Unfortunately for performance, these steps are carried out in the componentDidMount(/Update) method, which also performs style modifications on the element. The combination of height request and style modification during javascript evaluation in the browser leads to layout-thrashing, where the elements are repeatedly re-laid-out (see https://developers.google.com/web/fundamentals/performance/rendering/avoid-large-complex-layouts-and-layout-thrashing & https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Performance_best_practices_for_Firefox_fe_engineers). The solution implemented here is to memoize the collapsed state in Redux the first time the status is seen (e.g. when fetched as part of a small batch, to populate the home timeline) , so that on subsequent re-renders, the value can be queried, rather than recalculated. This strategy is derived from mastodon#4439 & mastodon#4909, and should resolve mastodon#12455. Andrew Lin (https://github.com/onethreeseven) is thanked for his assistance in root cause analysis and solution brainstorming
#12661) * Summary: fix slowness due to layout thrashing when reloading a large set of status updates in order to limit the maximum size of a status in a list view (e.g. the home timeline), so as to avoid having to scroll all the way through an abnormally large status update (see #8205), the following steps are taken: •the element containing the status is rendered in the browser •its height is calculated, to determine if it exceeds the maximum height threshold. Unfortunately for performance, these steps are carried out in the componentDidMount(/Update) method, which also performs style modifications on the element. The combination of height request and style modification during javascript evaluation in the browser leads to layout-thrashing, where the elements are repeatedly re-laid-out (see https://developers.google.com/web/fundamentals/performance/rendering/avoid-large-complex-layouts-and-layout-thrashing & https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Performance_best_practices_for_Firefox_fe_engineers). The solution implemented here is to memoize the collapsed state in Redux the first time the status is seen (e.g. when fetched as part of a small batch, to populate the home timeline) , so that on subsequent re-renders, the value can be queried, rather than recalculated. This strategy is derived from #4439 & #4909, and should resolve #12455. Andrew Lin (https://github.com/onethreeseven) is thanked for his assistance in root cause analysis and solution brainstorming * remove getSnapshotBeforeUpdate from status * remove componentWillUnmount from status * persist last-intersected status update and restore when ScrollableList is restored e.g. when navigating from home-timeline to a status conversational thread and <Back again * cache currently-viewing status id to avoid calling redux with identical value * refactor collapse toggle to pass explicit boolean

Scenario:
The reason that we render all 200 toots is that their rendered height was lost when they were unmounted (we have to fully render them again to save the original height).
This pull request saves the height of the toots in the store, so that we only fully render the 30 toots. This "cached height" is invalidated if the window is resized.