Page MenuHomePhabricator

[native] Add memoization to `SidebarItem`

Authored by atul on Thu, Sep 7, 9:31 AM.
Referenced Files
F774041: D9099.id30876.diff
Mon, Sep 25, 5:09 PM
Unknown Object (File)
Sun, Sep 24, 7:14 PM
Unknown Object (File)
Sun, Sep 17, 4:38 PM
Unknown Object (File)
Fri, Sep 15, 6:05 PM
F746618: 3798ef.png
Thu, Sep 7, 10:13 AM
F746620: 014d54.png
Thu, Sep 7, 10:13 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.Thu, Sep 7, 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.Thu, Sep 7, 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.