Page MenuHomePhabricator

[lib] Don't construct promises in useCreateChatThreadItem
ClosedPublic

Authored by ashoat on Dec 11 2024, 3:43 PM.
Tags
None
Referenced Files
F6085548: D14136.id.diff
Sun, Apr 20, 11:42 PM
Unknown Object (File)
Sun, Apr 20, 1:10 AM
Unknown Object (File)
Thu, Apr 3, 12:09 PM
Unknown Object (File)
Mar 11 2025, 4:10 PM
Unknown Object (File)
Mar 11 2025, 4:06 PM
Unknown Object (File)
Feb 22 2025, 9:04 PM
Unknown Object (File)
Feb 22 2025, 9:04 PM
Unknown Object (File)
Feb 22 2025, 9:03 PM
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