From 04b6297bc4e5a0dacb957a0e5519932b51d4dde0 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Tue, 18 Aug 2026 11:56:01 +0200 Subject: [PATCH 1/2] Add personalDetailsList merge timing instrumentation --- src/ONYXKEYS.ts | 6 + .../instrumentPersonalDetailsMerge.ts | 150 ++++++++++++++++++ src/setup/index.ts | 3 + 3 files changed, 159 insertions(+) create mode 100644 src/libs/telemetry/instrumentPersonalDetailsMerge.ts diff --git a/src/ONYXKEYS.ts b/src/ONYXKEYS.ts index a108395f26e9..464839e2cd36 100755 --- a/src/ONYXKEYS.ts +++ b/src/ONYXKEYS.ts @@ -856,6 +856,11 @@ const ONYXKEYS = { /** Collection Keys */ COLLECTION: { + /** + * Write-only mirror of PERSONAL_DETAILS_LIST used to benchmark what that key would cost as a collection. + * Nothing reads it — see instrumentPersonalDetailsMerge.ts. Delete once the measurement is done. + */ + PERSONAL_DETAILS_SHADOW: 'personalDetailsShadow_', ATTACHMENT: 'attachment_', DOMAIN: 'domain_', DOWNLOAD: 'download_', @@ -1450,6 +1455,7 @@ type OnyxFormDraftValuesMapping = { }; type OnyxCollectionValuesMapping = { + [ONYXKEYS.COLLECTION.PERSONAL_DETAILS_SHADOW]: OnyxTypes.PersonalDetails; [ONYXKEYS.COLLECTION.ATTACHMENT]: OnyxTypes.Attachment; [ONYXKEYS.COLLECTION.DOMAIN]: OnyxTypes.Domain; [ONYXKEYS.COLLECTION.DOWNLOAD]: OnyxTypes.Download; diff --git a/src/libs/telemetry/instrumentPersonalDetailsMerge.ts b/src/libs/telemetry/instrumentPersonalDetailsMerge.ts new file mode 100644 index 000000000000..643f540f9aab --- /dev/null +++ b/src/libs/telemetry/instrumentPersonalDetailsMerge.ts @@ -0,0 +1,150 @@ +/* eslint-disable no-console */ +// The Onyx write methods are wrapped for timing here, this module never writes data of its own. +/* eslint-disable rulesdir/prefer-actions-set-data */ +import ONYXKEYS from '@src/ONYXKEYS'; +import type {PersonalDetailsList} from '@src/types/onyx'; + +import type {OnyxMergeCollectionInput} from 'react-native-onyx'; + +import Onyx from 'react-native-onyx'; + +/** + * Times every Onyx write that appends to `personalDetailsList` so we can correlate the merge duration + * with how many keys the object already holds. Patches Onyx at startup instead of touching every + * call site, so both local merges and server-driven updates are covered. + * + * Every append is then mirrored into the `personalDetailsShadow_` collection and timed the same way, + * so the single-key and collection shapes can be compared on identical data. The mirror is + * write-only — nothing subscribes to it, so it cannot affect app behaviour. + */ + +const SHADOW_KEY = ONYXKEYS.COLLECTION.PERSONAL_DETAILS_SHADOW; + +let existingKeyCount = 0; + +/** Account IDs written to the shadow collection, so we can report its size without subscribing to it */ +const shadowAccountIDs = new Set(); + +let hasSeededShadowCollection = false; + +function countKeys(value: unknown): number { + return typeof value === 'object' && value !== null ? Object.keys(value).length : 0; +} + +/** Onyx writes are loosely typed at the patch boundary, so narrow to the shape we can mirror */ +function isPersonalDetailsChanges(value: unknown): value is PersonalDetailsList { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +// console.log instead of Log.info: Log's client callback uses console.debug, which is hidden +// behind the Verbose level in Chrome DevTools. +function measure(source: string, existingKeys: number, incomingKeys: number, extraParams: Record, promise: Promise): Promise { + const startTime = performance.now(); + + return promise.finally(() => { + console.log('[PersonalDetailsListPerf] append', { + source, + existingKeys, + incomingKeys, + durationMs: Math.round((performance.now() - startTime) * 100) / 100, + ...extraParams, + }); + }); +} + +function mergeShadowCollection(source: string, changes: PersonalDetailsList, extraParams: Record) { + const accountIDs = Object.keys(changes); + + if (accountIDs.length === 0) { + return; + } + + // Read before mutating, so it matches how the single-key path reports `existingKeys` + const existingKeys = shadowAccountIDs.size; + + const collection: OnyxMergeCollectionInput = {}; + for (const accountID of accountIDs) { + collection[`${SHADOW_KEY}${accountID}`] = changes[accountID]; + + if (changes[accountID] === null) { + shadowAccountIDs.delete(accountID); + } else { + shadowAccountIDs.add(accountID); + } + } + + measure(source, existingKeys, accountIDs.length, extraParams, Onyx.mergeCollection(SHADOW_KEY, collection)); +} + +/** + * Mirrors an append after the single-key write settles. Running them concurrently would make the two + * shapes fight over the same JS thread and storage, so neither measurement would mean anything. + */ +function mirrorAfter(promise: Promise, changes: PersonalDetailsList, extraParams: Record): Promise { + return promise.finally(() => { + mergeShadowCollection('collection', changes, extraParams); + }); +} + +Onyx.connectWithoutView({ + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + callback: (value) => { + existingKeyCount = value ? Object.keys(value).length : 0; + + // The shadow collection has to start from the same data as the single key, otherwise every + // measurement would compare an append to N keys against an append to an almost empty collection. + if (hasSeededShadowCollection || !value || existingKeyCount === 0) { + return; + } + hasSeededShadowCollection = true; + mergeShadowCollection('collection-seed', value, {}); + }, +}); + +let isInstrumented = false; + +export default function instrumentPersonalDetailsMerge() { + // Fast Refresh can re-run setup, and wrapping twice would log every write twice + if (isInstrumented) { + return; + } + isInstrumented = true; + console.log('[PersonalDetailsListPerf] instrumentation installed'); + + const originalMerge = Onyx.merge; + const originalUpdate = Onyx.update; + + Onyx.merge = ((key, changes) => { + const promise = originalMerge(key, changes); + + if (key !== ONYXKEYS.PERSONAL_DETAILS_LIST) { + return promise; + } + + const measuredPromise = measure('single-key', existingKeyCount, countKeys(changes), {}, promise); + + return isPersonalDetailsChanges(changes) ? mirrorAfter(measuredPromise, changes, {}) : measuredPromise; + }) as typeof Onyx.merge; + + Onyx.update = ((updates) => { + const promise = originalUpdate(updates); + const personalDetailsUpdates = updates.filter((update) => update.key === ONYXKEYS.PERSONAL_DETAILS_LIST); + + if (personalDetailsUpdates.length === 0) { + return promise; + } + + // ponytail: an Onyx.update batch resolves as a whole, so durationMs covers the sibling keys too. + // `updatesInBatch` is logged to spot the noisy samples; measure the isolated cost inside Onyx if that is not enough. + const extraParams = {updatesInBatch: updates.length}; + const changes: PersonalDetailsList = {}; + for (const update of personalDetailsUpdates) { + if (isPersonalDetailsChanges(update.value)) { + Object.assign(changes, update.value); + } + } + const measuredPromise = measure('single-key', existingKeyCount, countKeys(changes), extraParams, promise); + + return mirrorAfter(measuredPromise, changes, extraParams); + }) as typeof Onyx.update; +} diff --git a/src/setup/index.ts b/src/setup/index.ts index 2d0f25d0f3aa..ab23fece931f 100644 --- a/src/setup/index.ts +++ b/src/setup/index.ts @@ -1,4 +1,5 @@ import intlPolyfill from '@libs/IntlPolyfill'; +import instrumentPersonalDetailsMerge from '@libs/telemetry/instrumentPersonalDetailsMerge'; import {setDeviceID} from '@userActions/Device'; import initOnyxDerivedValues from '@userActions/OnyxDerived'; @@ -85,6 +86,8 @@ export default function () { ], }); + instrumentPersonalDetailsMerge(); + // Must be imported after Onyx.init() and outside the React lifecycle so that push notification // handlers are registered before any push arrives, including Android headless/background wake-ups. import('@libs/Notification/PushNotification/subscribeToPushNotifications'); From bac96670e7cf46da4411e5f03d85456b43f9d736 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Tue, 18 Aug 2026 12:57:37 +0200 Subject: [PATCH 2/2] Measure personalDetailsList as single key vs collection --- .../instrumentPersonalDetailsMerge.ts | 153 ++++++++++++++---- .../PersonalDetailsListShape.perf-test.ts | 111 +++++++++++++ 2 files changed, 234 insertions(+), 30 deletions(-) create mode 100644 tests/perf-test/PersonalDetailsListShape.perf-test.ts diff --git a/src/libs/telemetry/instrumentPersonalDetailsMerge.ts b/src/libs/telemetry/instrumentPersonalDetailsMerge.ts index 643f540f9aab..262f3395cb4b 100644 --- a/src/libs/telemetry/instrumentPersonalDetailsMerge.ts +++ b/src/libs/telemetry/instrumentPersonalDetailsMerge.ts @@ -13,19 +13,44 @@ import Onyx from 'react-native-onyx'; * with how many keys the object already holds. Patches Onyx at startup instead of touching every * call site, so both local merges and server-driven updates are covered. * - * Every append is then mirrored into the `personalDetailsShadow_` collection and timed the same way, - * so the single-key and collection shapes can be compared on identical data. The mirror is - * write-only — nothing subscribes to it, so it cannot affect app behaviour. + * Every write is then mirrored into the `personalDetailsShadow_` collection and timed the same way, so + * the single-key and collection shapes can be compared on identical data. Nothing in the app reads the + * mirror, so it cannot affect app behaviour. + * + * REQUIRES A COLD START: clear site data (or at least every `personalDetailsShadow_` key) before each + * run. The mirror is deliberately *not* pre-seeded from the existing list — it accumulates only from the + * writes it observes, so both shapes see the identical write sequence starting from empty. If stale + * mirror data survives from a previous run, every mirror write finds the member already byte-identical, + * `hasValueChanged` short-circuits it, and the collection posts near-zero durations against real + * single-key writes. That is a silent failure, so every collection sample logs `changedMembers`: pair a + * single-key line with a collection line only when their changed counts match. + * + * A synthetic subscriber fleet is attached to the mirror because the comparison is otherwise rigged: + * the single key broadcasts every write to its ~300 real subscribers, and a mirror with none would + * win on that alone. Each synthetic subscriber watches one member key, which is what the migration + * would produce. `shadowSubscribers` is logged on every line so a run is self-describing; set it to 0 + * to measure the write path in isolation. */ const SHADOW_KEY = ONYXKEYS.COLLECTION.PERSONAL_DETAILS_SHADOW; +/** + * Kept in the same order of magnitude as the real `personalDetailsList` subscriber count. Grep + * `ONYXKEYS.PERSONAL_DETAILS_LIST` under src/ to re-check it before trusting a run. + */ +const SHADOW_SUBSCRIBER_COUNT = 300; + let existingKeyCount = 0; -/** Account IDs written to the shadow collection, so we can report its size without subscribing to it */ -const shadowAccountIDs = new Set(); +/** + * Account ID -> serialised value last written to the mirror. A Set of IDs was not enough: knowing an ID + * was mirrored before says nothing about whether the incoming value differs, and Onyx short-circuits on + * value equality, not key presence. Keeping the value lets each sample report how many members were + * genuinely written (`changedMembers`) versus how many the collection path skipped for free. + */ +const mirroredMembers = new Map(); -let hasSeededShadowCollection = false; +const shadowConnections: Array> = []; function countKeys(value: unknown): number { return typeof value === 'object' && value !== null ? Object.keys(value).length : 0; @@ -36,68 +61,134 @@ function isPersonalDetailsChanges(value: unknown): value is PersonalDetailsList return typeof value === 'object' && value !== null && !Array.isArray(value); } +/** + * Measured writes currently in flight. Anything above zero means this sample is sharing the JS thread + * and the IndexedDB write queue with another sample — and if the other one is a merge to the same key, + * Onyx's `mergeQueue` hands both callers the *same* promise, so both "durations" end at one instant and + * neither is the cost of its own write. + */ +let inFlightWrites = 0; + // console.log instead of Log.info: Log's client callback uses console.debug, which is hidden // behind the Verbose level in Chrome DevTools. function measure(source: string, existingKeys: number, incomingKeys: number, extraParams: Record, promise: Promise): Promise { const startTime = performance.now(); + const concurrentWrites = inFlightWrites; + inFlightWrites++; return promise.finally(() => { - console.log('[PersonalDetailsListPerf] append', { + inFlightWrites--; + console.log('[PersonalDetailsListPerf] write', { source, existingKeys, incomingKeys, durationMs: Math.round((performance.now() - startTime) * 100) / 100, + // Filter on this. `false` means the sample overlapped another measured write, so its duration + // is contention plus possible `mergeQueue` promise-sharing, not the cost of the write it names. + // It does NOT judge whether the paired write did equivalent work — compare `changedMembers` + // between the two sources for that. + comparable: concurrentWrites === 0, + concurrentWrites, ...extraParams, }); }); } -function mergeShadowCollection(source: string, changes: PersonalDetailsList, extraParams: Record) { +function mergeShadowCollection(source: string, changes: PersonalDetailsList, extraParams: Record): Promise { const accountIDs = Object.keys(changes); if (accountIDs.length === 0) { - return; + return Promise.resolve(); } // Read before mutating, so it matches how the single-key path reports `existingKeys` - const existingKeys = shadowAccountIDs.size; + const existingKeys = mirroredMembers.size; + // `mergeCollection` cannot carry a null member, so removals go out as individual member merges. + // They are applied for mirror correctness but left untimed — appends are what's being measured. const collection: OnyxMergeCollectionInput = {}; + let upsertCount = 0; + let changedMembers = 0; for (const accountID of accountIDs) { - collection[`${SHADOW_KEY}${accountID}`] = changes[accountID]; + const member = changes[accountID]; - if (changes[accountID] === null) { - shadowAccountIDs.delete(accountID); - } else { - shadowAccountIDs.add(accountID); + if (member === null) { + mirroredMembers.delete(accountID); + Onyx.merge(`${SHADOW_KEY}${accountID}`, null); + continue; } + + // Onyx short-circuits a member whose value is unchanged, so only differing members cost anything. + // This is what makes a collection sample comparable to the single-key one: the single key does real + // work whenever *any* member differs, so the two are only equivalent if the changed counts match. + const serialised = JSON.stringify(member); + if (mirroredMembers.get(accountID) !== serialised) { + changedMembers++; + } + mirroredMembers.set(accountID, serialised); + + collection[`${SHADOW_KEY}${accountID}`] = member; + upsertCount++; + } + + if (upsertCount === 0) { + return Promise.resolve(); } - measure(source, existingKeys, accountIDs.length, extraParams, Onyx.mergeCollection(SHADOW_KEY, collection)); + return measure(source, existingKeys, upsertCount, {...extraParams, shadowSubscribers: shadowConnections.length, changedMembers}, Onyx.mergeCollection(SHADOW_KEY, collection)); } /** - * Mirrors an append after the single-key write settles. Running them concurrently would make the two + * Attached once the mirror first holds members, spread across the members written so far, so later + * writes land on a subscribed key as often as they would after a migration. + */ +function attachShadowSubscribers() { + const mirroredAccountIDs = [...mirroredMembers.keys()]; + + if (mirroredAccountIDs.length === 0 || shadowConnections.length > 0) { + return; + } + + for (let i = 0; i < SHADOW_SUBSCRIBER_COUNT; i++) { + const accountID = mirroredAccountIDs.at(i % mirroredAccountIDs.length); + shadowConnections.push( + Onyx.connectWithoutView({ + key: `${SHADOW_KEY}${accountID}` as const, + // reuseConnection: false, or identical key+config would collapse the fleet into one connection + reuseConnection: false, + // Reading the value is the point: it's what a real per-member subscriber costs + callback: (member) => member?.accountID, + }), + ); + } +} + +/** + * Every mirror write runs through this one chain. Two single-key merges to the same key inside one tick + * share a `mergeQueue` promise, so both `.finally` callbacks fire at the same instant — without the chain + * their mirrors would run concurrently and each would time the other's contention. + */ +let mirrorChain: Promise = Promise.resolve(); + +/** + * Mirrors a write after the single-key write settles. Running them concurrently would make the two * shapes fight over the same JS thread and storage, so neither measurement would mean anything. */ function mirrorAfter(promise: Promise, changes: PersonalDetailsList, extraParams: Record): Promise { return promise.finally(() => { - mergeShadowCollection('collection', changes, extraParams); + mirrorChain = mirrorChain + .then(() => mergeShadowCollection('collection', changes, extraParams)) + .then(attachShadowSubscribers) + .catch(() => undefined); }); } +// Tracks how many members the single key already holds, so each sample can be correlated with N. +// The mirror is intentionally not seeded from this value — see the cold-start note at the top. Onyx.connectWithoutView({ key: ONYXKEYS.PERSONAL_DETAILS_LIST, callback: (value) => { existingKeyCount = value ? Object.keys(value).length : 0; - - // The shadow collection has to start from the same data as the single key, otherwise every - // measurement would compare an append to N keys against an append to an almost empty collection. - if (hasSeededShadowCollection || !value || existingKeyCount === 0) { - return; - } - hasSeededShadowCollection = true; - mergeShadowCollection('collection-seed', value, {}); }, }); @@ -121,7 +212,8 @@ export default function instrumentPersonalDetailsMerge() { return promise; } - const measuredPromise = measure('single-key', existingKeyCount, countKeys(changes), {}, promise); + const existingKeys = existingKeyCount; + const measuredPromise = measure('single-key', existingKeys, countKeys(changes), {}, promise); return isPersonalDetailsChanges(changes) ? mirrorAfter(measuredPromise, changes, {}) : measuredPromise; }) as typeof Onyx.merge; @@ -134,8 +226,8 @@ export default function instrumentPersonalDetailsMerge() { return promise; } - // ponytail: an Onyx.update batch resolves as a whole, so durationMs covers the sibling keys too. - // `updatesInBatch` is logged to spot the noisy samples; measure the isolated cost inside Onyx if that is not enough. + // An Onyx.update batch resolves as a whole, so durationMs covers the sibling keys too. + // `updatesInBatch` is logged to spot the noisy samples; measure inside Onyx if that is not enough. const extraParams = {updatesInBatch: updates.length}; const changes: PersonalDetailsList = {}; for (const update of personalDetailsUpdates) { @@ -143,7 +235,8 @@ export default function instrumentPersonalDetailsMerge() { Object.assign(changes, update.value); } } - const measuredPromise = measure('single-key', existingKeyCount, countKeys(changes), extraParams, promise); + const existingKeys = existingKeyCount; + const measuredPromise = measure('single-key', existingKeys, countKeys(changes), extraParams, promise); return mirrorAfter(measuredPromise, changes, extraParams); }) as typeof Onyx.update; diff --git a/tests/perf-test/PersonalDetailsListShape.perf-test.ts b/tests/perf-test/PersonalDetailsListShape.perf-test.ts new file mode 100644 index 000000000000..c60f81783b3f --- /dev/null +++ b/tests/perf-test/PersonalDetailsListShape.perf-test.ts @@ -0,0 +1,111 @@ +import ONYXKEYS from '@src/ONYXKEYS'; +import type {PersonalDetails} from '@src/types/onyx'; + +import Onyx from 'react-native-onyx'; +import {measureAsyncFunction} from 'reassure'; + +import createPersonalDetails from '../utils/collections/personalDetails'; + +/** + * A/B for the two shapes `personalDetailsList` could have, on identical data: + * - single key — one object at `personalDetailsList` holding every member (today) + * - collection — one Onyx key per member under `personalDetailsShadow_` + * + * Jest resolves Onyx storage to `MemoryOnlyProvider`, so these numbers are the JS-side cost only: + * `mergeChanges` allocating a full copy, `cache.hasValueChanged` deep-equalling the result, and the + * subscriber broadcast. IndexedDB/SQLite write cost is not included — see + * `src/libs/telemetry/instrumentPersonalDetailsMerge.ts` for real-device numbers. + */ + +const COLLECTION_KEY = ONYXKEYS.COLLECTION.PERSONAL_DETAILS_SHADOW; + +/** Member counts to seed before appending, spanning a small account up to a large domain */ +const SIZES = [1000, 5000, 20000]; + +/** + * Both shapes carry this many subscribers, and every one of them watches the member being written, so + * both broadcast to the same 50 callbacks. That is deliberately conservative: it throws away the + * collection's real advantage — a member change wakes only that member's watchers, where the single key + * wakes all ~300 of its subscribers — so whatever gap remains is merge/deep-equal/storage cost alone. + */ +const SUBSCRIBER_COUNT = 50; + +/** The member every write targets, and every subscriber watches. Must exist before subscribing. */ +const TARGET_ACCOUNT_ID = 0; + +/** Non-nullable values, so the same data can seed both a `merge` and a `mergeCollection` without a cast */ +function buildMembers(size: number): Record { + const members: Record = {}; + for (let i = 0; i < size; i++) { + members[i] = createPersonalDetails(i); + } + return members; +} + +function toCollection(members: Record): Record<`${typeof COLLECTION_KEY}${number}`, PersonalDetails> { + const collection: Record<`${typeof COLLECTION_KEY}${number}`, PersonalDetails> = {}; + for (const [accountID, member] of Object.entries(members)) { + collection[`${COLLECTION_KEY}${Number(accountID)}`] = member; + } + return collection; +} + +/** + * `reuseConnection: false` matters: identical key + config would otherwise be deduped into one shared + * connection by OnyxConnectionManager, collapsing the fleet to a single subscriber. + */ +function subscribeAll(key: typeof ONYXKEYS.PERSONAL_DETAILS_LIST | `${typeof COLLECTION_KEY}${number}`): () => void { + const connections = Array.from({length: SUBSCRIBER_COUNT}, () => + Onyx.connectWithoutView({ + key, + reuseConnection: false, + callback: (value) => value, + }), + ); + + return () => { + for (const connection of connections) { + Onyx.disconnect(connection); + } + }; +} + +/** + * Each iteration has to write a value that differs from the last one, otherwise `hasValueChanged` + * short-circuits and both the storage write and the broadcast are skipped — measuring nothing. + */ +function makeWrite(accountID: number): (iteration: number) => Partial { + return (iteration) => ({accountID, displayName: `written-${iteration}`}); +} + +describe('personalDetailsList shape', () => { + afterEach(() => Onyx.clear()); + + describe.each(SIZES)('%i existing members', (size) => { + test('single key: write one member', async () => { + const members = buildMembers(size); + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, members); + const unsubscribe = subscribeAll(ONYXKEYS.PERSONAL_DETAILS_LIST); + + const write = makeWrite(TARGET_ACCOUNT_ID); + let iteration = 0; + + await measureAsyncFunction(() => Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[TARGET_ACCOUNT_ID]: write(iteration++)})); + + unsubscribe(); + }); + + test('collection: write one member', async () => { + const members = buildMembers(size); + await Onyx.mergeCollection(COLLECTION_KEY, toCollection(members)); + const unsubscribe = subscribeAll(`${COLLECTION_KEY}${TARGET_ACCOUNT_ID}`); + + const write = makeWrite(TARGET_ACCOUNT_ID); + let iteration = 0; + + await measureAsyncFunction(() => Onyx.merge(`${COLLECTION_KEY}${TARGET_ACCOUNT_ID}`, write(iteration++))); + + unsubscribe(); + }); + }); +});