Perf-Onyx-optimize-get-all

[rinej]

Explanation of Change

Bumps react-native-onyx to pull in Expensify/react-native-onyx#805, which optimizes SQLiteProvider.getAll().

Previously getAll() fetched every row from keyvaluepairs and ran JSON.parse once per row (N parses) to hydrate the Onyx cache at startup. It now asks SQLite to aggregate the whole table into a single JSON string.

• Native-only - getAll() resolves to SQLiteProvider on iOS/Android. Web (IDBKeyValProvider) is unaffected.
• Runs on the cold-start cache hydration path
• Perf gain is modest on real accounts (~11%, within noise)

Fixed Issues

$ #89652 (comment)
PROPOSAL:

Tests

• Sign in on native, let the app fully sync
• Background the app and force-kill it (swipe away / kill process).
• Cold launch the app
• Verify:
  • LHN fully populates - same report list, order, unread/pinned states as before
  • Open several reports (chat, expense, workspace) - all messages, amounts, avatars render correctly
  • Open Settings - Workspaces - Profile - Wallet - all persisted data present
  • No missing, duplicated, or blank data

Offline tests

QA Steps

Same as Tests

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
• If new assets were added or existing ones were modified, I verified that:
  • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
  • The assets load correctly across all supported platforms.
• If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
  • I verified that all the inputs inside a form are aligned with each other.
  • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
• I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

Android-getAll-Perf.mp4

Android: mWeb Chrome

iOS: Native

iOS-getAll-Perf.mp4

iOS: mWeb Safari

MacOS: Chrome / Safari

[github-actions]

⚠️ This PR is possibly changing native code and/or updating libraries, it may cause problems with HybridApp. Please check if any patch updates are required in the HybridApp repo and run an AdHoc build to verify that HybridApp will not break. Ask Contributor Plus for help if you are not sure how to handle this. ⚠️

[melvin-bot]

@mkhutornyi Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

[mkhutornyi]

Reviewer Checklist

• I have verified the author checklist is complete (all boxes are checked off).
• I verified the correct issue is linked in the ### Fixed Issues section above
• I verified testing steps are clear and they cover the changes made in this PR
  • I verified the steps for local testing are in the Tests section
  • I verified the steps for Staging and/or Production testing are in the QA steps section
  • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
• I checked that screenshots or videos are included for tests on all platforms
• I included screenshots or videos for tests on all platforms
• I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
• I verified tests pass on all platforms & I tested again on:
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
• If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
• I verified proper code patterns were followed (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I verified that this PR follows the guidelines as stated in the Review Guidelines
• I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • The file is named correctly
  • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
  • The only data being stored in the state is data necessary for rendering and nothing else
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • All JSX used for rendering exists in the render method
  • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
• If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
  • I verified that all the inputs inside a form are aligned with each other.
  • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
• For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
• I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

android2.mov

Android: mWeb Chrome

iOS: HybridApp

ios2.mov

iOS: mWeb Safari

MacOS: Chrome / Safari

web2.mov

[mkhutornyi]

Onyx PR was merged so bump Onyx version

[roryabraham]

This will need to bump to 3.0.91, but there are some other versions in the queue that need to be done first. namely Expensify/react-native-onyx#799 and Expensify/react-native-onyx#801

[fabioh8010]

@rinej Could you put this PR into draft state? As stated by Rory we still have some PRs in the bump queue before we get into this one.

[rinej]

Ok, converting to draft. Will reopen after we bump the Onyx

[chrispader]

@rinej #93438 has just been merged, so this PR is next in the Onyx PR merge queue. 🙌🏼

[rinej]

sound great! I will resolve conflict and prepare proper PR

[fabioh8010]

@chrispader @rinej #93438 was merged but we need to wait until it at least hit Staging for one/two days so we can catch regressions

[rinej]

Ok, I merged main and resolved the conflicts. Now waiting for the green light once the previous release finishes testing

[mkhutornyi]

@rinej please merge main and mark as ready for review
Staging QA is done and no regressions found from #93438

[MelvinBot]

Review

This is a clean dependency bump. The App-side diff is only react-native-onyx 3.0.90 → 3.0.91 in package.json + package-lock.json, and the integrity/version pair matches the published release. All CI is green (Bun, unit tests, typecheck, ESLint, perf tests). Nothing to change in this repo.

The real change lives in the merged onyx PR (Expensify/react-native-onyx#805), which rewrites SQLiteProvider.getAll() to aggregate the whole keyvaluepairs table into one JSON string via json_group_array(...) and parse it once, instead of JSON.parse-ing every row. It's native-only (web IDBKeyValProvider untouched), runs on the cold-start cache-hydration path, has unit tests, and Staging QA on the prior bump found no regressions. I'd merge it, but a couple of things are worth a sanity check before/at QA since this path hydrates the entire Onyx cache:

Points worth confirming

1. Values now round-trip through SQLite's JSON serializer. Previously JSON.parse(row.valueJSON) parsed the stored text verbatim. Now json(valueJSON) re-serializes each value inside SQLite before JS parses the aggregate (SQLiteProvider.ts:201-217). SQLite's JSON functions minify/normalize, which can reformat number literals. Expensify amounts are integer cents (well within Number.MAX_SAFE_INTEGER) so this is almost certainly fine, but worth a quick check that no stored value carries a large integer or high-precision float that could shift on the round-trip.

2. Single aggregated string. The whole DB is now materialized as one JSON string in SQLite, then parsed — a transient memory spike (string + parsed structure held simultaneously) proportional to DB size, and subject to SQLITE_MAX_LENGTH (~1GB default). Unlikely to bite, but the failure mode for a very large high-traffic account would be a hard error hydrating the entire cache (app can't start) rather than the old incremental per-row behavior. The high-traffic-account cold-start test in the checklist covers this.

3. Perf benefit. The PR description itself notes the gain is "~11%, within noise." Given (1) and (2), it's worth confirming the team considers the win worth the added serialization path — not a blocker, just a judgment call.

Order isn't guaranteed by json_group_array (no ORDER BY), but the old query didn't guarantee it either and cache hydration doesn't depend on it, so that's a non-issue.

Since roryabraham's earlier CHANGES_REQUESTED was about the bump-queue timing (now cleared per mkhutornyi's latest comment) rather than the code, this looks ready once main is merged and it's marked ready for review.

[MelvinBot]

Regression scan

The App diff itself is regression-free — it's a pure react-native-onyx 3.0.90 → 3.0.91 version bump. I confirmed via the onyx 3.0.90...3.0.91 compare that the only functional change in that release is the getAll() rewrite from react-native-onyx#805 (SQLiteProvider.ts, native-only). So all regression risk lives in that one function, and there is one architectural risk plus a test gap worth flagging before merge — none are blockers, but they're on the cold-start critical path.

Top risk — this can fail closed on native cold start. getAll() now validates/parses every stored value through SQLite's json() inside a single aggregate query (SQLiteProvider.ts:199-214). If that query throws, the wrapper only degrades to memory-only for the web IDBKeyVal error string — a native SQLite error re-rejects (storage/index.ts:31-42) and propagates to the init catch, which boots the app with default key states only (OnyxUtils.ts:999-1057) — i.e. an empty, fresh-install-looking app for that launch until the next sync. One bad row fails the entire hydration.

Details & the three regression vectors

1. Fail-closed hydration (behavior change, verified). The old code parsed rows individually in JS (rows._array.map(row => [key, JSON.parse(row.valueJSON)])), which also rejected the whole batch on one bad row — so the "one bad row bricks hydration → boot with defaults" blast radius is not new. What changes is the set of inputs that trigger it: validation moved from V8's JSON.parse to SQLite's json(), and the two do not accept exactly the same inputs. Any value that json() rejects but JSON.parse accepts is a new way to fail cold-start hydration. Candidates worth verifying against the SQLite build bundled in react-native-nitro-sqlite@9.6.0 (I did not execute these — they need confirmation, not assumption):

• JSON nesting-depth limit (SQLite caps JSON depth; V8 effectively doesn't) — a deeply nested report/thread could trip it.
• Lone/unpaired UTF-16 surrogate escapes (\uD800 without a pair) in user-generated content — JSON.parse accepts these; some SQLite versions reject.

2. Silent value alteration (new invisible failure mode). Old path returned values parsed byte-for-byte by V8 from the stored text. New path round-trips each value through SQLite's json() serializer before V8 parses the aggregate, so SQLite renormalizes the JSON (number formatting, whitespace, duplicate-key handling, unicode escapes). Any divergence yields a cached value that silently differs from what was written — no error, just wrong data. Expensify amounts are integer cents (well within safe range) so likelihood is low, but unlike vector 1 this fails without any signal.

3. Memory / size ceiling on large accounts. The whole DB is now materialized as a single JSON string inside SQLite, then held in JS alongside the parsed result — a transient memory spike proportional to DB size, bounded by SQLITE_MAX_LENGTH (~1 GB default). High-traffic accounts are exactly the cold-start case this path runs on; the old row-streaming approach didn't build that intermediate mega-string.

Test gap (verified). The tests added in #805 cover only the happy path (5 rows) and an empty table — none exercise large payloads, deep nesting, unicode/surrogate edge cases, or numeric precision, so vectors 1–3 are currently unguarded.

Non-issues I checked: empty table returns '[]' → [] (handled); NULL valueJSON → [key, null], same as before; result ordering is unspecified but the consumer builds a keyed cache so order is irrelevant.

Suggestion: before merge, sanity-check the tester's high-traffic-account cold-start (the checklist item covers the memory/large-payload angle), and add unit tests for a lone-surrogate string and a deeply-nested object to lock down vector 1. Since this is native-only, mWeb/desktop are unaffected.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: b270be1068

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

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".

[rinej]

I merged main and it's ready for review. I also did some smoke testing, and it looks good

1-Onyx.mp4

[rinej]

@mkhutornyi please have a look, when you got a moment

[github-actions]

🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here.

[mkhutornyi]

I merged main and it's ready for review.

Looks like you didn't pull latest main.

1514 commits behind.

package com.margelo.nitro.nitrofetch does not exist

Android adhoc build failed because nitro fetch PR was not pulled here

[rinej]

I just merged the main again now, should be good now

[chrispader]

Is this ready to get merged now? @fabioh8010 👀

[fabioh8010]

I think we are just waiting on @roryabraham review

[mountiny]

C+ already approved I will move this one ahead to make the queue flowing

Addressed

[OSBotify]

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

[github-actions]

🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here.

[github-actions]

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #95511.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-b7ceed09108-2c5d336-9216baa9999aad3fe5c67483f4aaafb8fc5be802/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-b7ceed0-2c5d336-b15e513ed89588e1c674e76cebb67272ae6ce00b/index.html

Web 🕸️
https://95511.pr-testing.expensify.com

---

👀 View the workflow run that generated this build 👀

[OSBotify]

🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.49-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

[OSBotify]

🚀 Deployed to production by https://github.com/arosiclair in version: 9.4.49-3 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

Bundle Size Analysis (Sentry):

• Android