Skip to content
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

Optimize notifications rendering #1957

Merged
merged 1 commit into from
Nov 17, 2023
Merged

Optimize notifications rendering #1957

merged 1 commit into from
Nov 17, 2023

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Nov 17, 2023

Before

Scrolling notifications with 6x CPU throttling: 700ms stall.

Screenshot 2023-11-17 at 17 44 07

After

Scrolling notifications with 6x CPU throttling: 30ms stall.

Screenshot 2023-11-17 at 17 47 15

@gaearon gaearon requested a review from pfrazee November 17, 2023 17:49
@gaearon gaearon merged commit 7c51a39 into main Nov 17, 2023
4 checks passed
@gaearon gaearon deleted the perf-2 branch November 17, 2023 17:51
@@ -56,15 +56,15 @@ interface Author {
moderation: ProfileModeration
}

export function FeedItem({
let FeedItem = ({

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi, i'm just stranger
but can you explain this approach, please? interesting
why you need to store component in let variable and apply memo later, instead of just wrapping component in memo?

Copy link

@Hartaithan Hartaithan Nov 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow, i doesn't know that every stranger can comments on pr's like that
if i'm not allowed to, please excuse me

Copy link

@recursive-beast recursive-beast Nov 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the react docs, memo accepts a component to memoize, which is in this case a functional component. You can either store the component's function in a variable and then pass it to memo (what is done here) or just inline it in the memo() call (what is done in the docs).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't really matter, I just want to avoid an extra indentation level in the code.

@gaearon
Copy link
Collaborator Author

gaearon commented Nov 20, 2023

To people getting excited by this PR — this just returns us to the baseline perf we had before a bunch of refactors. So this won't by itself make anything faster, it just avoids us making things slow :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants