Page MenuHomePhabricator

[native] Memoize `PanGestureHandler` props in `SwipeableMessage`
ClosedPublic

Authored by atul on Aug 31 2023, 9:25 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 27, 2:27 AM
Unknown Object (File)
Tue, Nov 26, 1:41 AM
Unknown Object (File)
Mon, Nov 25, 11:51 PM
Unknown Object (File)
Sat, Nov 9, 7:30 PM
Unknown Object (File)
Sun, Nov 3, 7:44 PM
Unknown Object (File)
Sun, Nov 3, 7:44 PM
Unknown Object (File)
Sun, Nov 3, 7:44 PM
Unknown Object (File)
Sun, Nov 3, 7:41 PM
Subscribers

Details

Summary

Based on flamegraph from profiler, it looks like A. creating SwipeSnakes and PanGestureHandler in SwipeableMessage is expensive B. because we're not memoizing things as carefully as we could be we're eating this cost multiple times.

In this diff I just memoize the PanGestureHandler props. In the subsequent diff I'll add some more memoization to this component specifically wrt the SwipeSnakes


Depends on D9052

Test Plan

"Before":

6b03c7.png (1×2 px, 530 KB)

Will do another round of profiling after the subsequent diff to show improvement.

Diff Detail

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

Event Timeline

atul published this revision for review.Aug 31 2023, 9:27 AM
atul edited the test plan for this revision. (Show Details)
tomek added inline comments.
native/chat/swipeable-message.react.js
242–243

It might be even better to define them outside the component

This revision is now accepted and ready to land.Sep 1 2023, 3:50 AM
native/chat/swipeable-message.react.js
242–243

That's a good point. There might be an argument for defining this "close to where this is being used," but I'll go ahead and pull it out of component.

This revision was landed with ongoing or failed builds.Sep 5 2023, 11:34 AM
This revision was automatically updated to reflect the committed changes.