Speed up SearchRouter open on large accounts#95378
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
| /** Whether search mode is active - when true, builds full report map for personal details (default: false) */ | ||
| isSearching?: boolean; | ||
| /** Beta features the user has access to */ | ||
| betas?: OnyxEntry<Beta[]>; |
There was a problem hiding this comment.
Removed betas because it was dead code: createFilteredOptionList never read it (only declared in the options type), and it was the one input excluded from the cache key — a trap if someone ever started using it. No behavior change: screens still pass betas to getSearchOptions/getValidOptions, where beta filtering actually happens.
| // Contacts are expensive to build on large accounts (one option per personal detail). When a screen | ||
| // opts into deferral and is not actively searching, skip building them entirely; the empty state | ||
| // does not display standalone contacts, and typing flips `isSearching` which rebuilds the full set. | ||
| const shouldBuildContacts = includeP2P && !(deferContactsUntilSearch && !isSearching); |
There was a problem hiding this comment.
Can we simplify this? maybe extract one more variable to write it more clearly
There was a problem hiding this comment.
Done 👍 Extracted an intermediate variable so the double negation is gone:
const areContactsDeferred = deferContactsUntilSearch && !isSearching;
const shouldBuildContacts = includeP2P && !areContactsDeferred;
| registerSessionCleanupCallback(() => { | ||
| cachedSource = undefined; | ||
| cachedMap = {}; | ||
| hasCached = false; |
There was a problem hiding this comment.
I wonder if we really need this boolean. If I understand it correctly after first caching, it's always true, isn't it?
There was a problem hiding this comment.
Good catch, you're right, it was redundant
| // does not display standalone contacts, and typing flips `isSearching` which rebuilds the full set. | ||
| const shouldBuildContacts = includeP2P && !(deferContactsUntilSearch && !isSearching); | ||
|
|
||
| const cacheEntryKey = `${maxRecentReports}_${includeP2P}_${isSearching}_${deferContactsUntilSearch}`; |
There was a problem hiding this comment.
Maybe let's add a function to generate this key :)
There was a problem hiding this comment.
And I wonder, should we take into account locale here?
| * empty state (contact pickers) must leave this false. | ||
| */ | ||
| deferContactsUntilSearch?: boolean; | ||
| /** Active locale; defaults to `IntlStore.getCurrentLocale()`. Pass it from a hook to recompute on language switch. */ |
There was a problem hiding this comment.
NAB: To me comment for locale is unnecessary
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR reduces SearchRouter open latency on large accounts by avoiding expensive, contact-heavy option building on empty query and by reusing createFilteredOptionList results between opens when relevant Onyx inputs are unchanged.
Changes:
- Add an LRU, module-level cache for
createFilteredOptionList(cache hits gated by referential equality of Onyx inputs + locale + report-actions version). - Introduce
deferContactsUntilSearchto skip building per-contact options until a non-empty query is entered (used for SearchRouter/autocomplete). - Add session-signout cleanup hooks for module-level caches, plus tests/perf-test updates to keep runs deterministic.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/OptionsListUtilsTest.tsx | Clears the new module cache per test; adds unit coverage for deferContactsUntilSearch. |
| tests/perf-test/OptionsListUtils.perf-test.ts | Clears cache between perf-test iterations to measure cold-build behavior. |
| src/pages/Share/ShareTab.tsx | Removes now-unneeded betas config from useFilteredOptions call. |
| src/pages/NewChatPage/index.tsx | Removes now-unneeded betas config from useFilteredOptions call. |
| src/libs/SessionCleanup.ts | Adds a dependency-light registry for “clear module cache on sign-out” callbacks. |
| src/libs/OptionsListUtils/index.ts | Implements cached createFilteredOptionList, stable defaults, contacts deferral option, and cache invalidation inputs. |
| src/libs/actions/Session/index.ts | Runs session cleanup callbacks during cleanupSession() on sign-out. |
| src/hooks/useSearchSelector/base.ts | Removes now-unneeded betas config from useFilteredOptions call. |
| src/hooks/usePrivateIsArchivedMap.ts | Adds a stable-reference cache for the derived archived map, with sign-out cleanup. |
| src/hooks/useFilteredOptions.ts | Adds deferContactsUntilSearch, removes betas, and forces recompute on locale changes. |
| src/components/Search/SearchAutocompleteList.tsx | Enables deferContactsUntilSearch for empty-query SearchRouter-like behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| import useOnyx from './useOnyx'; | ||
|
|
||
| type PrivateIsArchivedMap = Record<string, boolean>; |
ReviewSolid, carefully-documented perf PR. I traced the two mechanisms end-to-end against the code and they're correct: the empty-query SearchRouter state never renders standalone contacts, so ✅ Verified
🟡 Worth confirming
TestingI did not run the suite. Given the caching, the module-level cache being cleared per-test via the new How I reviewedChecked out the PR branch, compared |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
947b353 to
c38c6a5
Compare
Cache createFilteredOptionList results between opens, defer building contact options until the user types a query, and clear the caches on sign-out.
cb0e894 to
eaabef6
Compare
|
All review feedback addressed. I verified the fixes have no negative perf impact (A/B control run) — the gain vs |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Replace the deprecatedReportActionsVersion counter (bumped inside the deprecated Onyx.connect callback) with the reference of sortedActions from the RAM_ONLY_SORTED_REPORT_ACTIONS derived value, which produces a new object on every recompute. useFilteredOptions now passes sortedActions down through processReport to createOption, so this call path also reads sorted actions from the derived value instead of the deprecated module-level map.
# Conflicts: # tests/unit/OptionsListUtilsTest.tsx
| // Search-mode results contain an option for every report and contact, so caching them would retain | ||
| // full-account-sized arrays until sign-out — and any Onyx change invalidates them anyway. | ||
| const shouldUseCache = !isSearching; |
There was a problem hiding this comment.
@WojtekBoman Should we use the cache when building full personal details, shouldBuildContacts is true?
There was a problem hiding this comment.
Search-mode results are already reused by useMemo while typing (the query isn't a build input), and SearchForReports Onyx updates keep invalidating them anyway. Caching these builds would retain our largest results (no maxRecentReports cap) for hits that would almost never occur.
…st-render # Conflicts: # src/components/Search/SearchAutocompleteList.tsx
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-07-20.at.11.12.57.PM.mov |
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Explanation of Change
On the Sentry "P90 vital metrics - large accounts" dashboard,
ManualOpenSearchRouterroughly doubled to ~1s p90 in production — almost entirely inSearchRouter.ListRender, which builds the initial empty-query option list. Two structural issues, both scaling linearly with account size:createFilteredOptionListis rebuilt from scratch on every open (no caching between opens; two unstable input references blocked memoization).Fix
createFilteredOptionList— a small keyed map (one entry per screen config, bounded to 8 with LRU eviction) validated by reference equality of its Onyx inputs (plus reference-stabilizedusePrivateIsArchivedMap/visibleReportActionsData, with the active locale and a report-actions version in the key). Reopening reuses the previous result while Onyx is unchanged; each caller gets a shallow clone so in-place mutations don't leak across screens, andisSearchingresults aren't cached.deferContactsUntilSearch, enabled only for the SearchRouter — skips per-contact options while the query is empty; typing flipsisSearchingand builds the full set. OtheruseFilteredOptionsconsumers keep the default and are unaffected.Performance results
Measured on web dev, span
SearchRouter.ListRender(empty-query open — the Sentry-tracked metric). Each cell is the mean of 100+ opens driven by Playwright: a script clicks the search button, reads the span duration from the console, closes the router, and repeats.mainthere is no cache, so cold ≈ warm.Accounts are ordered by increasing gain; all bars share one scale (1 block ≈ 10 ms).
Account 1 —
expensify@swmansion.com(small real account, no seeding): 588–610 reports, 273–305 transactions, 265 transaction violations, 255 policies, ~2.8–2.9k Onyx keys; 140–161 real personal details (the account was syncing from staging during the run, so counts drifted slightly upward between the branch andmainpasses — figures shown are the observed ranges).main)██████████░░░░░░~103 ms██████████░░░░░░~103 ms██████████░░░░░░~96 ms█████████░░░░░░░~91 msAccount 2 —
applausetester@applause.expensifail.com(large account, all data real, no seeding): 2,457 reports, 2,777 transactions, 969 transaction violations, 129 policies, ~11.5k Onyx keys; 19,178 real personal details.main)███████████████░150 ms███████████████░151 ms████████████░░░░~125 ms███████████░░░░░~113 msAccount 3 —
expensify@swmansion.com(same account, seeded): ~600 reports, ~300 transactions, 255 policies, ~2.9k Onyx keys; 166 real personal details, seeded up to 19,161 synthetic contacts to reproduce large-account contact volume.main)████████████████160 ms████████████████160 ms███████████░░░░░~108 ms██████████░░░░░░~102 msConclusions. The gain comes entirely from cutting per-contact work at empty query, so it scales with contact count: −7/−12% at ~150 contacts up to −33/−36% at ~19k — exactly the large accounts the Sentry dashboard flagged, with no cost to small ones. Account 3 gains more than the larger Account 2 despite being smaller overall because the saving comes from contacts, and for the seeded account contacts are almost its entire cost — whereas Account 2's 2.4k reports / 2.8k transactions are fixed rebuild cost the PR doesn't remove.
Fixed Issues
$ #95585
PROPOSAL:
Tests
from:followed by a few letters of a contact's name — verify the autocomplete suggests matching contacts.Offline tests
Unnecessary — this PR only changes how the Search Router option list is built (caching + skipping unused work); it does not touch any network or persistence logic.
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari