Skip to content

Commit

Permalink
[lib] Use ChatThreadItemLoaderCache for loading sidebar
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Ashoat committed Dec 13, 2024
1 parent 5817605 commit 4b33216
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 10 deletions.
20 changes: 11 additions & 9 deletions lib/selectors/chat-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -111,29 +111,31 @@ 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;
})();

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,
};
Expand Down
81 changes: 80 additions & 1 deletion lib/shared/sidebar-item-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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<SidebarInfo>,
): Promise<$ReadOnlyArray<SidebarThreadItem>> {
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<SidebarItemForCache>(
loaders,
);
const unreadCache = new ChatThreadItemLoaderCache<SidebarItemForCache>(
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,
};

0 comments on commit 4b33216

Please sign in to comment.