Page MenuHomePhabricator

[native] Add memoization to `SidebarItem`
ClosedPublic

Authored by atul on Sep 7 2023, 9:31 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jun 15, 10:34 PM
Unknown Object (File)
Mon, Jun 10, 7:25 PM
Unknown Object (File)
Wed, May 22, 9:30 AM
Unknown Object (File)
Apr 9 2024, 2:00 AM
Unknown Object (File)
Apr 9 2024, 2:00 AM
Unknown Object (File)
Apr 3 2024, 7:23 PM
Unknown Object (File)
Feb 26 2024, 3:15 AM
Unknown Object (File)
Feb 19 2024, 4:15 AM
Subscribers

Details

Summary

Call to shortAbsoluteDate in particular takes surprisingly long (like 5ms each time and it's re-rendering like 5 times and there are 5 instances so 5*5*5ms = 125ms total when doing the "navigate to thread w/ varun scenario").

Test Plan

Continues to work/appear as before.

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul published this revision for review.Sep 7 2023, 9:37 AM

Please wait for CI to pass before landing

native/chat/sidebar-item.react.js
19–22

Doing a global search on this I've noticed that this pattern is repeated a few times. As a follow up diff, should we make shortAbsoluteDate into a hook (maybe like useShortAbsoluteDate) so that we can use useMemo in the hook and simplify this call here and in other places?

This revision is now accepted and ready to land.Sep 7 2023, 9:46 AM

Disregard, pasted flame graph for incorrect render

native/chat/sidebar-item.react.js
19–22

Yeah I'll jot down that we should look at all shortAbsoluteDate callsites. There's other low-hanging fruit so I'll probably prioritize that first.

This revision was automatically updated to reflect the committed changes.