[native] Add memoization to `SidebarItem`
ClosedPublic
Actions

Authored by atul on Sep 7 2023, 9:31 AM.

Details

Reviewers

ginsu
tomek
rohan

Commits

rCOMM3eae57836282: [native] Add memoization to `SidebarItem`

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 created this revision.Sep 7 2023, 9:31 AM

Herald added a subscriber: ashoat. · View Herald TranscriptSep 7 2023, 9:31 AM

atul edited the summary of this revision. (Show Details)Sep 7 2023, 9:35 AM

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

Harbormaster completed remote builds in B22431: Diff 30847.Sep 7 2023, 9:48 AM

atul added a child revision: D9100: [native] Memoize `SwipeableThread`.Sep 7 2023, 10:05 AM

Disregard, pasted flame graph for incorrect render

atul added inline comments.Sep 8 2023, 11:02 AM

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.