Skip to content

[No QA] PoC: measure personalDetailsList as a single key vs an Onyx collection - #98857

Draft
TMisiukiewicz wants to merge 3 commits into
Expensify:mainfrom
callstack-internal:personal-details-collection-poc
Draft

[No QA] PoC: measure personalDetailsList as a single key vs an Onyx collection#98857
TMisiukiewicz wants to merge 3 commits into
Expensify:mainfrom
callstack-internal:personal-details-collection-poc

Conversation

@TMisiukiewicz

Copy link
Copy Markdown
Contributor

Explanation of Change

Measurement-only PoC — not for merge. This quantifies what personalDetailsList costs as a single Onyx key versus as a collection, so the migration can be scoped from numbers instead of intuition. Two instruments: src/libs/telemetry/instrumentPersonalDetailsMerge.ts patches Onyx.merge/Onyx.update at startup, mirrors every personal-details write into a write-only personalDetailsShadow_ collection, and times both shapes on identical data; tests/perf-test/PersonalDetailsListShape.perf-test.ts is a controlled Reassure A/B at 1k/5k/20k members with an equal subscriber fleet on both shapes.

Result — web, real IndexedDB, 11,173 members, 3 cold OpenApp runs: the initial full-list write costs ~1,471 ms as a single key vs 488 ms as a collection (3.1×); a 79-member follow-up costs ~393 ms vs ~17 ms (~24×). Personal details alone account for ~1.86 s of blocking Onyx work per OpenApp today, against ~505 ms for the collection shape. The Jest A/B shows why: the single key is O(N) in members (179 µs → 706 µs → 3,805 µs at 1k → 5k → 20k) while a collection member write is flat at ~25 µs, because every append re-allocates the whole object, deep-equals it in OnyxCache.hasValueChanged, rewrites it to storage, and broadcasts it to ~300 subscribers.

Two findings that matter for scoping. The collection's ~490 ms full-list cost is almost entirely fixed mergeCollection overhead for 11k keys, not per-value work — it was identical across runs where 11,093 members changed and runs where none did — so collection cost tracks key count, not change count. And the win only materialises if read sites move to per-member subscriptions: ~60 sites currently do useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST) with no selector, and a back-compat derived value re-materialising the O(N) object would be a regression in the interim.

Fixed Issues

$
PROPOSAL:

Tests

This PR adds no user-facing behaviour. The mirror collection is write-only — nothing in the app subscribes to it — so app behaviour should be unchanged.

  1. Run TEST_RUNNER_PATH=./node_modules/jest/bin/jest.js npm run perf-test -- --testMatch '**/PersonalDetailsListShape.perf-test.ts' and verify 6 passing cases; the .reassure/current.perf means should show the single key scaling with member count while the collection stays flat.
  2. To reproduce the device numbers: clear site data (required — see below), load the app on an account with many personal details, and read the [PersonalDetailsListPerf] write console lines. Pair a single-key line with the collection line that has the same incomingKeys, updatesInBatch and changedMembers, and discard any line with comparable: false.
  3. Verify the app behaves normally with the instrumentation active — chat, profile pages and mentions all still resolve personal details.
  • Verify that no errors appear in the JS console

Cold start is mandatory between runs. The shadow collection persists in IndexedDB; on a warm run every mirror write finds the member byte-identical, Onyx short-circuits it, and the collection posts near-zero durations against real single-key writes. changedMembers exposes this rather than letting it pass silently.

Offline tests

No offline behaviour changes. The instrumentation only wraps write timing and mirrors into an unsubscribed key, so queued and replayed writes are timed the same as any other.

QA Steps

None — this is instrumentation behind no user-facing surface and is not intended to ship. [No QA]

  • Verify that no errors appear in the JS console

Known limitations

  • Adds ~11k extra Onyx keys and doubles personal-details storage while active, plus 300 synthetic subscribers. Fine for a measurement build, unacceptable in production.
  • Patching Onyx.merge/Onyx.update globally at startup is a measurement hack, not a pattern to copy.
  • One unresolved anomaly: the 1-member write's mirror logs before the full-list write's mirror yet already sees a fully-populated mirror, which the serial mirrorChain should prevent. It does not affect the numbers above (all concurrentWrites: 0), but the serialisation is not behaving as designed.
  • The 79-member pair rests on one inference: both shapes receive the identical changes object and the single key did real work, so those members genuinely differ. Re-running now reports changedMembers and will settle it outright.

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 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 (beyond the intentional [PersonalDetailsListPerf] logging)
  • I followed proper code patterns (see Reviewing the code)
  • I verified that comments were added to code that is not self explanatory
  • I added unit tests for any new feature or bug fix in this PR

@github-actions

Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant