Page MenuHomePhabricator

[lib] Don't construct promises in useCreateChatThreadItem
ClosedPublic

Authored by ashoat on Wed, Dec 11, 3:43 PM.
Tags
None
Referenced Files
F3511027: D14136.id46436.diff
Sat, Dec 21, 1:01 PM
F3510094: D14136.id46438.diff
Sat, Dec 21, 11:47 AM
Unknown Object (File)
Thu, Dec 19, 9:18 PM
Unknown Object (File)
Thu, Dec 19, 8:44 PM
Unknown Object (File)
Thu, Dec 19, 5:22 PM
Unknown Object (File)
Wed, Dec 18, 3:18 PM
Unknown Object (File)
Wed, Dec 18, 8:11 AM
Unknown Object (File)
Tue, Dec 17, 11:27 AM
Subscribers
None

Details

Summary

This is the first diff in a stack that reduces the creation of Promises, which seems to overwhelm Hermes (see here).

This diff overhauls useCreateChatThreadItem:

  • It converts getLastUpdatedTime so that it returns a function that returns a Promise instead of just a Promise.
  • It replaces several of the returned properties with a single getFinalChatThreadItem() async function that returns all of them. I think this is cleaner.
  • It introduces lastUpdatedAtMostTimeIncludingSidebars. We need this for a later diff, which uses it to guess an order of which ChatThreadItems to check in order to get the top N ChatThreadItems by (resolved) lastUpdatedTimeIncludingSidebars. This way, we avoid creating the promises unless we need them.

Depends on D13984

Test Plan

I tested this task by playing around with the ChatThreadList on mobile while using a stopwatch to measure how long various operations took. I tested the updated code 3 times against both the most recent Testflight build as well as master. I found that performance was approximately the same as before. I tested scrolling down, selecting threads that were read, selecting threads that were unread, and search.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable