Skip to content

Commit

Permalink
[lib] Avoid creating Promises for showInMessagePreview if not needed
Browse files Browse the repository at this point in the history
Summary:
This solves [ENG-10201](https://linear.app/comm/issue/ENG-10201/cold-start-is-significantly-slower-in-build-460), in a similar way to how D14136 (and associated stack) solved [ENG-9952](https://linear.app/comm/issue/ENG-9952/investigate-performance-regression-on-mobile-build-444). The basic story is that creating too many `Promise`s overwhelms Hermes, and in particular this occurs for many Farcaster-linked communities where all of the recent messages are membership robotext messages about users joining the community.

Luckily, we only need `Promise`s for `showInMessagePreview` in `reactionMessageSpec`. This diff simply updates the signature of `showInMessagePreview` to allow for returning immediately (instead of a `Promise`), which solves the performance regression. More context on Linear [here](https://linear.app/comm/issue/ENG-10201/cold-start-is-significantly-slower-in-build-460#comment-8746a762).

Test Plan: Deployed a Release build to my iOS device. Prior to this diff, it reproduced the cold start performance regression. Following this diff, it no longer reproduced.

Reviewers: tomek, angelika

Reviewed By: tomek

Differential Revision: https://phab.comm.dev/D14361
  • Loading branch information
Ashoat committed Feb 14, 2025
1 parent 8bc6216 commit 520c6e5
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 7 deletions.
5 changes: 4 additions & 1 deletion lib/hooks/message-hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,13 @@ function useGetMessageInfoForPreview(): (
shouldFetchOlderMessages: false,
};
}
const shouldShow = await showInMessagePreview(
let shouldShow = showInMessagePreview(
messageInfo,
showInMessagePreviewParams,
);
if (shouldShow instanceof Promise) {
shouldShow = await shouldShow;
}
if (shouldShow) {
return {
messageInfoForPreview: messageInfo,
Expand Down
2 changes: 1 addition & 1 deletion lib/shared/messages/add-members-message-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ export const addMembersMessageSpec: AddMembersMessageSpec = Object.freeze({
return { shouldMerge: true, item: mergedItem };
},

showInMessagePreview: async (
showInMessagePreview: (
messageInfo: AddMembersMessageInfo,
params: ShowInMessagePreviewParams,
) => threadTypeIsThick(params.threadInfo.type),
Expand Down
2 changes: 1 addition & 1 deletion lib/shared/messages/join-thread-message-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export const joinThreadMessageSpec: MessageSpec<
return { shouldMerge: true, item: mergedItem };
},

showInMessagePreview: async (
showInMessagePreview: (
messageInfo: JoinThreadMessageInfo,
params: ShowInMessagePreviewParams,
) => threadTypeIsThick(params.threadInfo.type),
Expand Down
2 changes: 1 addition & 1 deletion lib/shared/messages/leave-thread-message-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export const leaveThreadMessageSpec: MessageSpec<
return { shouldMerge: true, item: mergedItem };
},

showInMessagePreview: async (
showInMessagePreview: (
messageInfo: LeaveThreadMessageInfo,
params: ShowInMessagePreviewParams,
) => threadTypeIsThick(params.threadInfo.type),
Expand Down
2 changes: 1 addition & 1 deletion lib/shared/messages/message-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export type MessageSpec<Data, RawInfo, Info> = {
+showInMessagePreview?: (
messageInfo: Info,
params: ShowInMessagePreviewParams,
) => Promise<boolean>,
) => boolean | Promise<boolean>,
+getLastUpdatedTime?: (
messageInfoOrRawMessageInfo: Info | RawInfo,
params: ShowInMessagePreviewParams,
Expand Down
2 changes: 1 addition & 1 deletion lib/shared/messages/multimedia-message-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ export const multimediaMessageSpec: MultimediaMessageSpec = Object.freeze({
validator: rawMultimediaMessageInfoValidator,

showInMessagePreview: (messageInfo: MediaMessageInfo | ImagesMessageInfo) =>
Promise.resolve(messageInfo.media.length > 0),
messageInfo.media.length > 0,

getLastUpdatedTime: (
messageInfo:
Expand Down
2 changes: 1 addition & 1 deletion lib/shared/messages/remove-members-message-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ export const removeMembersMessageSpec: RemoveMembersMessageSpec = Object.freeze(
return { shouldMerge: true, item: mergedItem };
},

showInMessagePreview: async (
showInMessagePreview: (
messageInfo: RemoveMembersMessageInfo,
params: ShowInMessagePreviewParams,
) => threadTypeIsThick(params.threadInfo.type),
Expand Down

0 comments on commit 520c6e5

Please sign in to comment.