From 4b33216d7218d9aff9d35b577d4c13adb8094906 Mon Sep 17 00:00:00 2001 From: Ashoat Tevosyan Date: Thu, 12 Dec 2024 22:37:45 -0500 Subject: [PATCH] [lib] Use ChatThreadItemLoaderCache for loading sidebar Summary: We have the same problem for sidebars that we have at the parent level, especially for the "Daily updates" channel, which has close to 1000 sidebars inside of it. We need to avoid creating a `Promise` for each sidebar. We can use `ChatThreadItemLoaderCache` to achieve this. Depends on D14144 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. Reviewers: tomek, angelika Reviewed By: tomek Differential Revision: https://phab.comm.dev/D14154 --- lib/selectors/chat-selectors.js | 20 ++++---- lib/shared/sidebar-item-utils.js | 81 +++++++++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 10 deletions(-) diff --git a/lib/selectors/chat-selectors.js b/lib/selectors/chat-selectors.js index bdd65cc57c..e17a0c4274 100644 --- a/lib/selectors/chat-selectors.js +++ b/lib/selectors/chat-selectors.js @@ -23,7 +23,7 @@ import { messageSpecs } from '../shared/messages/message-specs.js'; import { getSidebarItems, getAllInitialSidebarItems, - getAllFinalSidebarItems, + getCandidateSidebarItemsForThreadList, type SidebarItem, } from '../shared/sidebar-item-utils.js'; import { threadInChatList, threadIsPending } from '../shared/thread-utils.js'; @@ -111,12 +111,14 @@ function useGetChatThreadItemLoader(): ThreadInfo => ChatThreadItemLoader { const getFinalChatThreadItem = async () => { const lastUpdatedTimePromise = lastUpdatedTime(); + const candidateSidebarItemsPromise = + getCandidateSidebarItemsForThreadList(sidebars); + const lastUpdatedTimeIncludingSidebarsPromise = (async () => { - const lastUpdatedTimePromises = [ - lastUpdatedTimePromise, - ...sidebars.map(sidebar => sidebar.lastUpdatedTime()), - ]; - const lastUpdatedTimes = await Promise.all(lastUpdatedTimePromises); + const candidateSidebarItems = await candidateSidebarItemsPromise; + const lastUpdatedTimes = candidateSidebarItems.map( + sidebarItem => sidebarItem.lastUpdatedTime, + ); const max = lastUpdatedTimes.reduce((a, b) => Math.max(a, b), -1); return max; })(); @@ -124,16 +126,16 @@ function useGetChatThreadItemLoader(): ThreadInfo => ChatThreadItemLoader { const [ lastUpdatedTimeResult, lastUpdatedTimeIncludingSidebars, - allSidebarItems, + candidateSidebarItems, ] = await Promise.all([ lastUpdatedTimePromise, lastUpdatedTimeIncludingSidebarsPromise, - getAllFinalSidebarItems(sidebars), + candidateSidebarItemsPromise, ]); return { ...chatThreadItemBase, - sidebars: getSidebarItems(allSidebarItems), + sidebars: getSidebarItems(candidateSidebarItems), lastUpdatedTime: lastUpdatedTimeResult, lastUpdatedTimeIncludingSidebars: lastUpdatedTimeIncludingSidebars, }; diff --git a/lib/shared/sidebar-item-utils.js b/lib/shared/sidebar-item-utils.js index 2125910b9d..c78427e1ac 100644 --- a/lib/shared/sidebar-item-utils.js +++ b/lib/shared/sidebar-item-utils.js @@ -8,6 +8,7 @@ import { maxUnreadSidebars, type SidebarInfo, } from '../types/thread-types.js'; +import ChatThreadItemLoaderCache from '../utils/chat-thread-item-loader-cache.js'; import { threeDays } from '../utils/date-utils.js'; export type SidebarThreadItem = { @@ -113,4 +114,82 @@ async function getAllFinalSidebarItems( return await Promise.all(allSidebarItemPromises); } -export { getSidebarItems, getAllInitialSidebarItems, getAllFinalSidebarItems }; +type SidebarItemForCache = { + +threadInfo: ThreadInfo, + +mostRecentNonLocalMessage: ?string, + +lastUpdatedTimeIncludingSidebars: number, +}; + +// This async function returns a set of candidates that can be passed to +// getSidebarItems. It avoids passing all of the sidebarInfos so that we don't +// need to `await lastUpdatedTime()` for all of them, which we've found can be +// expensive on Hermes. Instead, we use ChatThreadItemLoaderCache to only +// consider the top N candidates, such that passing the results to +// getSidebarItems would yield the same set as if we had processed every single +// sidebar. +async function getCandidateSidebarItemsForThreadList( + sidebarInfos: $ReadOnlyArray, +): Promise<$ReadOnlyArray> { + const loaders = sidebarInfos.map(sidebar => ({ + threadInfo: sidebar.threadInfo, + lastUpdatedAtLeastTimeIncludingSidebars: sidebar.lastUpdatedAtLeastTime, + lastUpdatedAtMostTimeIncludingSidebars: sidebar.lastUpdatedAtMostTime, + initialChatThreadItem: { + threadInfo: sidebar.threadInfo, + mostRecentNonLocalMessage: sidebar.mostRecentNonLocalMessage, + lastUpdatedTimeIncludingSidebars: sidebar.lastUpdatedAtLeastTime, + }, + getFinalChatThreadItem: async () => { + const lastUpdatedTime = await sidebar.lastUpdatedTime(); + return { + threadInfo: sidebar.threadInfo, + mostRecentNonLocalMessage: sidebar.mostRecentNonLocalMessage, + lastUpdatedTimeIncludingSidebars: lastUpdatedTime, + }; + }, + })); + + // We want the top maxReadSidebars threads (either read or unread), + // and the top maxUnreadSidebars unread threads + const generalCache = new ChatThreadItemLoaderCache( + loaders, + ); + const unreadCache = new ChatThreadItemLoaderCache( + loaders.filter(loader => loader.threadInfo.currentUser.unread), + ); + + const topGeneralPromise = + generalCache.loadMostRecentChatThreadItems(maxReadSidebars); + const topUnreadPromise = + unreadCache.loadMostRecentChatThreadItems(maxUnreadSidebars); + const [topGeneralResults, topUnreadResults] = await Promise.all([ + topGeneralPromise, + topUnreadPromise, + ]); + + const topResults = [ + ...topGeneralResults.slice(0, maxReadSidebars), + ...topUnreadResults.slice(0, maxUnreadSidebars), + ]; + + return topResults.map(result => { + const { + threadInfo, + mostRecentNonLocalMessage, + lastUpdatedTimeIncludingSidebars, + } = result; + return { + type: 'sidebar', + threadInfo, + lastUpdatedTime: lastUpdatedTimeIncludingSidebars, + mostRecentNonLocalMessage, + }; + }); +} + +export { + getSidebarItems, + getAllInitialSidebarItems, + getAllFinalSidebarItems, + getCandidateSidebarItemsForThreadList, +};