Page MenuHomePhabricator
Differential D9103

[native] Memoize `swipeableThread` in `ChatThreadListItem`
ClosedPublic
Actions

Authored by atul on Sep 7 2023, 11:22 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 25, 10:55 PM
Unknown Object (File)
Fri, Nov 22, 7:29 AM
Unknown Object (File)
Thu, Nov 21, 6:51 AM
Unknown Object (File)
Thu, Nov 21, 6:51 AM
Unknown Object (File)
Thu, Nov 21, 6:51 AM
Unknown Object (File)
Thu, Nov 21, 6:50 AM
Unknown Object (File)
Thu, Nov 21, 6:45 AM
Unknown Object (File)
Fri, Nov 8, 6:23 AM
View All 81 Files
Subscribers
ashoat

Details

Reviewers
ginsu
tomek
rohan
Commits
rCOMM37df956ea00c: [native] Memoize `swipeableThread` in `ChatThreadListItem`
Summary

This prevents re-renders of SwipeableThreads. As of this diff we've reduced the time of ChatThreadList re-renders from ~600ms to ~70ms.

ChatThreadListItem before D9102:

014d54.png (1×2 px, 648 KB)

ChatThreadListItem before this diff:

8cb34d.png (1×2 px, 892 KB)

ChatThreadListItem after this diff:

3a946b.png (1×2 px, 920 KB)

(This is the chatThreadListItem for Daily Updates)

Depends on D9102

Test Plan

Things continue to look/work as expected.

Diff Detail

Repository
rCOMM Comm
Branch
arcpatch-D9103 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul created this revision.Sep 7 2023, 11:22 AM
Herald added a subscriber: ashoat. · View Herald TranscriptSep 7 2023, 11:22 AM
atul published this revision for review.Sep 7 2023, 11:24 AM
atul edited the summary of this revision. (Show Details)
atul edited the summary of this revision. (Show Details)
atul added inline comments.
native/chat/chat-thread-list-item.react.js
133–140 ↗(On Diff #30852)

Might've gone too "high resolution" w/ the memoization, but personally find this more readable than the heavily nested JSX

Harbormaster completed remote builds in B22436: Diff 30852.Sep 7 2023, 11:40 AM
tomek accepted this revision.Sep 8 2023, 2:34 AM

While introducing a lot of memoization, we should also check how it affects memory usage

This revision is now accepted and ready to land.Sep 8 2023, 2:34 AM
atul added a comment.Sep 8 2023, 3:06 PM
In D9103#268200, @tomek wrote:

While introducing a lot of memoization, we should also check how it affects memory usage

Good point. When I previously used Xcode Instruments our app had pretty low memory usage, so will make sure that's still the case.

atul updated this revision to Diff 30883.Sep 8 2023, 3:08 PM

rebase and land

This revision was landed with ongoing or failed builds.Sep 8 2023, 3:11 PM
Closed by commit rCOMM37df956ea00c: [native] Memoize `swipeableThread` in `ChatThreadListItem` (authored by atul). · Explain Why
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMM37df956ea00c: [native] Memoize `swipeableThread` in `ChatThreadListItem`.
Harbormaster completed remote builds in B22463: Diff 30883.Sep 8 2023, 3:25 PM

Revision Contents
Changeset List

PathSize
native/
chat/
127 lines

Diff 30883

View Options

native/chat/chat-thread-list-item.react.js

Loading...