[HOLD for payment 2024-01-03] [HOLD for payment 2023-12-28] [Performance] Search input performance optimization

[mountiny]

Coming from this performance proposal:

Problem:

Currently, when users type in the search input in Money Request Participant List search, each keystroke triggers re-rendering of Option List, which is preceded by heavy calculations in the getOptions function, leading to significant performance overhead. Comparing the execution time of this function on Web:

• Account with 66 reports and 4200 personal details → 45ms
• Account with 15330 reports and 12 personal details → 650ms

With keystrokes calling such expensive function, this results in a laggy user experience, as the app processes each input individually without any delay or optimization. The continuous processing of these inputs strains the app’s resources, causing slowdowns and delays in displaying search results.

Additionally, each keystroke is causing unnecessary re-renders in the list items that could be avoided to reduce the consumption of the CPU.

### Solution:
To address this problem, we could do a few steps that should positively impact the usage of search:

1. Debouncing the search value update - this will allow to call expensive calculations only after the user stopped typing for a specified duration (e.g. 200 ms). So, when typing “Andrew”, the getOptions would run only once instead of 6 times (unless there will be a pause while typing)
2. Memoizing components to avoid unnecessary re-renders - currently, when typing in the input, a lot of components are getting unnecessarily re-rendered - e.g. each list item and BaseOptionSelector. Memoizing BaseOptionsSelector and fixing wrong memo condition !_.isEqual(prevProps.option.icons, nextProps.option.icons) causing re-renders on each keystroke.
3. We could stop using debounce on search in server, since with a debounced search value update it won’t be necessary anymore.

Comparing the results on the account with 15330 reports on Web:
Action: write adg in the search
Metric: getOptions call count Before: 4 After: 1
Metric: max JS heap Before: 595mb After: 306mb
Metric: MoneyRequestParticipantSelector max render time Before: ~110ms After: ~75ms
Metric: Single list item render count Before: 4 After: 1

Comparing account with 4200 personal details on Android - you can see attached screenshot of CPU usage from Flashlight. The blue line indicates the CPU usage while writing my email address “tomasz.misiukiewicz@callstack.com” and having getOptions method called on each keystroke. CPU usage for JS thread is around 100% all the time. The yellow line indicates CPU usage when calling getOptions function is debounced - the usage jumps only when there was a pause in typing longer than the debounce time.

This changes will also benefit other lists using OptionsSelector component:

• CategoryPicker
• TagPicker
• NewChatPage
• SearchPage
• TaskAssigneeSelectorModal
• TaskShareDestinationSelectorModal

Few of them already have debouncing implemented, but they have different amount of milliseconds set, so we can unify them. Additionally each one of them will benefit from reduced amount of re-renders of list items.

[melvin-bot]

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[TMisiukiewicz]

Hi, I'm Tomasz from Callstack and I'd like to work on that issue

[kadiealexander]

Not overdue!

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.14-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• (perf) Debounce search in OptionList #32962

If no regressions arise, payment will be issued on 2023-12-28. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @TMisiukiewicz does not require payment (Contractor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@mountiny] The PR that introduced the bug has been identified. Link to the PR:
• [@mountiny] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@mountiny] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@TMisiukiewicz] Determine if we should create a regression test for this bug.
• [@TMisiukiewicz] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.17-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• (perf) Implement debounce in OptionList search #33347

If no regressions arise, payment will be issued on 2024-01-03. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @TMisiukiewicz does not require payment (Contractor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@mountiny] The PR that introduced the bug has been identified. Link to the PR:
• [@mountiny] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@mountiny] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@TMisiukiewicz] Determine if we should create a regression test for this bug.
• [@TMisiukiewicz] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[mountiny]

This is new feature and performance related I think no regression tests are needed we are working on new automated perf tests