Page MenuHomePhabricator

[native] Add some memoization to `InnerTextMessage` component
ClosedPublic

Authored by atul on Aug 31 2023, 11:38 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 14, 11:39 PM
Unknown Object (File)
Wed, Nov 13, 3:55 AM
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
Unknown Object (File)
Sun, Nov 3, 7:23 PM
Unknown Object (File)
Mon, Oct 28, 1:45 PM
Subscribers

Details

Summary

Was able to reduce re-renders with some memoization.


Depends on D9054

Test Plan

Before:

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

After:

762d99.png (1×344 px, 201 KB)

You can observe that we don't render anything nested within InnerTextMessage

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul published this revision for review.Aug 31 2023, 11:39 AM
atul edited the test plan for this revision. (Show Details)

As of this diff we've brought the "second render" of MessageList from ~100ms to ~40ms. The ThreadList optimizations should be 4-5x as impactful.

tomek added inline comments.
native/chat/inner-text-message.react.js
130–133 ↗(On Diff #30669)

Probably can be defined outside the component

This revision is now accepted and ready to land.Sep 1 2023, 5:41 AM
native/chat/inner-text-message.react.js
130–133 ↗(On Diff #30669)

I think the cost of this is pretty low and would prefer to keep the styles "near" the JSX

This revision was landed with ongoing or failed builds.Sep 6 2023, 1:25 PM
This revision was automatically updated to reflect the committed changes.