Page MenuHomePhabricator

[native] Add memoization to `SidebarItem`

Authored by atul on Sep 7 2023, 9:31 AM.
Referenced Files
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
Unknown Object (File)
Feb 19 2024, 1:33 AM
Unknown Object (File)
Feb 19 2024, 12:50 AM



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

rCOMM Comm
Lint Not Applicable
Tests Not Applicable

Event Timeline

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

Please wait for CI to pass before landing

19–22 ↗(On Diff #30847)

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

19–22 ↗(On Diff #30847)

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.