Reuse unchanged derived values and skip no-op derived writes

src/libs/actions/OnyxDerived/configs/guideAccountIDs.ts

@@ -5,6 +5,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
 import type {GuideAccountIDsDerivedValue} from '@src/types/onyx';
 
 import {Str} from 'expensify-common';
+import {shallowEqual} from 'fast-equals';
 
 /**
  * Derives the accountIDs of every Expensify Guide (identified by login domain) in the personal details list.
@@ -13,7 +14,7 @@ import {Str} from 'expensify-common';
 export default createOnyxDerivedValueConfig({
     key: ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS,
     dependencies: [ONYXKEYS.PERSONAL_DETAILS_LIST],
-    compute: ([personalDetailsList]) => {
+    compute: ([personalDetailsList], {currentValue}) => {
         if (!personalDetailsList) {
             return [];
         }
@@ -25,6 +26,7 @@ export default createOnyxDerivedValueConfig({
             }
             guideAccountIDs.push(personalDetails.accountID);
         }
-        return guideAccountIDs.sort((a, b) => a - b);
+        guideAccountIDs.sort((a, b) => a - b);
+        return currentValue && shallowEqual(currentValue, guideAccountIDs) ? currentValue : guideAccountIDs;
     },
 });

src/libs/actions/OnyxDerived/configs/loginToAccountIDMap.ts

@@ -3,6 +3,8 @@ import createOnyxDerivedValueConfig from '@userActions/OnyxDerived/createOnyxDer
 import ONYXKEYS from '@src/ONYXKEYS';
 import type {LoginToAccountIDMapDerivedValue} from '@src/types/onyx';
 
+import {shallowEqual} from 'fast-equals';
+
 /**
  * Derives a map of login (lowercased) -> accountID from the personal details list.
  *
@@ -13,7 +15,7 @@ import type {LoginToAccountIDMapDerivedValue} from '@src/types/onyx';
 export default createOnyxDerivedValueConfig({
     key: ONYXKEYS.DERIVED.LOGIN_TO_ACCOUNT_ID_MAP,
     dependencies: [ONYXKEYS.PERSONAL_DETAILS_LIST],
-    compute: ([personalDetailsList]) => {
+    compute: ([personalDetailsList], {currentValue}) => {
         if (!personalDetailsList) {
             return {};
         }
@@ -30,6 +32,6 @@ export default createOnyxDerivedValueConfig({
                 loginToAccountIDMap[login] = personalDetails.accountID;
             }
         }
-        return loginToAccountIDMap;
+        return currentValue && shallowEqual(currentValue, loginToAccountIDMap) ? currentValue : loginToAccountIDMap;
     },
 });

src/libs/actions/OnyxDerived/configs/reportTransactionsAndViolations.ts

@@ -5,6 +5,8 @@ import type {TransactionViolation} from '@src/types/onyx';
 
 import type {OnyxCollection} from 'react-native-onyx';
 
+import {shallowEqual} from 'fast-equals';
+
 let previousViolations: OnyxCollection<TransactionViolation[]> = {};
 let transactionReportIDMapping: Record<string, string> = {};
 
@@ -117,7 +119,7 @@ export default createOnyxDerivedValueConfig({
 
         previousViolations = violations;
 
-        return reportTransactionsAndViolations;
+        return currentValue && shallowEqual(currentValue, reportTransactionsAndViolations) ? currentValue : reportTransactionsAndViolations;
     },
     // On cache clear, drop the cross-compute state so the map is rebuilt from scratch (see the engine's resetForClear).
     onReset: () => {

src/libs/actions/OnyxDerived/index.ts

@@ -146,6 +146,9 @@ function init() {
                 try {
                     // @ts-expect-error TypeScript can't confirm the shape of dependencyValues matches the compute function's parameters
                     const newDerivedValue = compute(dependencyValues, context);
+                    if (newDerivedValue === derivedValue) {
+                        return;
+                    }
                     Log.info(`[OnyxDerived] updating value for ${key} in Onyx`);
                     derivedValue = newDerivedValue;
                     setDerivedValue(key, derivedValue);

tests/perf-test/AttendeesCellPersonalDetailsWrites.perf-test.tsx

@@ -0,0 +1,93 @@
+import {screen} from '@testing-library/react-native';
+
+import ComposeProviders from '@components/ComposeProviders';
+import {LocaleContextProvider} from '@components/LocaleContextProvider';
+import OnyxListItemProvider from '@components/OnyxListItemProvider';
+import AttendeesCell from '@components/Search/SearchList/ListItem/AttendeesCell';
+
+import initOnyxDerivedValues from '@userActions/OnyxDerived';
+
+import ONYXKEYS from '@src/ONYXKEYS';
+import type {PersonalDetailsList} from '@src/types/onyx';
+import type {Attendee} from '@src/types/onyx/IOU';
+
+import React from 'react';
+import {View} from 'react-native';
+import Onyx from 'react-native-onyx';
+import {measureRenders} from 'reassure';
+
+import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
+
+const ROW_COUNT = 50;
+const ATTENDEES_PER_ROW = 3;
+const WRITES_PER_SCENARIO = 10;
+const UNRELATED_ACCOUNT_ID = 1000;
+
+const personalDetails: PersonalDetailsList = {
+    [UNRELATED_ACCOUNT_ID]: {accountID: UNRELATED_ACCOUNT_ID, login: 'unrelated@test.com', displayName: 'Unrelated'},
+};
+const rows: Attendee[][] = [];
+for (let row = 0; row < ROW_COUNT; row++) {
+    const attendees: Attendee[] = [];
+    for (let index = 0; index < ATTENDEES_PER_ROW; index++) {
+        const accountID = row * ATTENDEES_PER_ROW + index + 1;
+        const email = `attendee${accountID}@test.com`;
+        personalDetails[accountID] = {accountID, login: email, displayName: `Attendee ${accountID}`};
+        attendees.push({email, displayName: `Attendee ${accountID}`, avatarUrl: ''});
+    }
+    rows.push(attendees);
+}
+
+function SearchRowsWithAttendees() {
+    return (
+        <ComposeProviders components={[OnyxListItemProvider, LocaleContextProvider]}>
+            <View>
+                {rows.map((attendees, index) => (
+                    <AttendeesCell
+                        // eslint-disable-next-line react/no-array-index-key
+                        key={index}
+                        attendees={attendees}
+                        isHovered={false}
+                        isPressed={false}
+                    />
+                ))}
+            </View>
+        </ComposeProviders>
+    );
+}
+
+describe('AttendeesCell on personal details writes', () => {
+    beforeAll(() => {
+        Onyx.init({keys: ONYXKEYS});
+        initOnyxDerivedValues();
+    });
+
+    beforeEach(async () => {
+        await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, personalDetails);
+        await waitForBatchedUpdates();
+    });
+
+    afterEach(async () => {
+        await Onyx.clear();
+        await waitForBatchedUpdates();
+    });
+
+    test('[AttendeesCell] should re-render 50 Search rows when a user who attends nothing is renamed', async () => {
+        // Reassure runs the scenario several times against one seeded Onyx, so every run writes a name that doesn't exist yet
+        let run = 0;
+        const scenario = async () => {
+            // Given 50 Search rows with attendees have rendered
+            await screen.findAllByTestId('AttendeesCell-Row');
+
+            // When a user who attends none of them is renamed, which makes the engine recompute the login map to the same content
+            for (let index = 0; index < WRITES_PER_SCENARIO; index++) {
+                await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[UNRELATED_ACCOUNT_ID]: {displayName: `Unrelated ${run}-${index}`}});
+                await waitForBatchedUpdates();
+            }
+            run++;
+        };
+
+        // Then reassure measures how many row re-renders the unrelated writes cause
+        await measureRenders(<SearchRowsWithAttendees />, {scenario});
+    });
+});

tests/perf-test/SidebarLinksPersonalDetailsWrites.perf-test.tsx

@@ -0,0 +1,110 @@
+import {screen} from '@testing-library/react-native';
+
+import {setHasRadio} from '@libs/NetworkState';
+
+import CONST from '@src/CONST';
+import ONYXKEYS from '@src/ONYXKEYS';
+import type {PersonalDetailsList} from '@src/types/onyx';
+
+import Onyx from 'react-native-onyx';
+import {measureRenders} from 'reassure';
+
+import * as LHNTestUtils from '../utils/LHNTestUtils';
+import * as TestHelper from '../utils/TestHelper';
+import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
+import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatchedUpdates';
+
+jest.mock('@libs/Permissions');
+jest.mock('../../src/libs/Navigation/Navigation', () => ({
+    navigate: jest.fn(),
+    isActiveRoute: jest.fn(),
+    getTopmostReportId: jest.fn(),
+    getActiveRoute: jest.fn(),
+    getTopmostReportActionId: jest.fn(),
+    isNavigationReady: jest.fn(() => Promise.resolve()),
+    isDisplayedInModal: jest.fn(() => false),
+    getActiveRouteWithoutParams: jest.fn(() => ''),
+}));
+jest.mock('../../src/libs/Navigation/navigationRef', () => ({
+    getState: () => ({
+        routes: [{name: 'Report'}],
+    }),
+    getRootState: () => ({
+        routes: [],
+    }),
+    addListener: () => () => {},
+    isReady: () => true,
+}));
+
+jest.mock('@react-navigation/native');
+
+const REPORT_COUNT = 500;
+const WRITES_PER_SCENARIO = 10;
+const GUIDE_ACCOUNT_ID = 50;
+const UNRELATED_ACCOUNT_ID = 60;
+
+const reports = Object.fromEntries(
+    Array.from({length: REPORT_COUNT}, (value, index) => {
+        const report = {...LHNTestUtils.getFakeReport([1, 2], 1, true), lastMessageText: 'hey'};
+        return [`${ONYXKEYS.COLLECTION.REPORT}${index + 1}`, report];
+    }),
+);
+
+const personalDetails: PersonalDetailsList = {
+    ...LHNTestUtils.fakePersonalDetails,
+    [GUIDE_ACCOUNT_ID]: {accountID: GUIDE_ACCOUNT_ID, login: `guide@${CONST.EMAIL.GUIDES_DOMAIN}`, displayName: 'Guide'},
+    [UNRELATED_ACCOUNT_ID]: {accountID: UNRELATED_ACCOUNT_ID, login: 'unrelated@test.com', displayName: 'Unrelated'},
+};
+
+describe('SidebarLinks on personal details writes', () => {
+    beforeAll(() => {
+        Onyx.init({
+            keys: ONYXKEYS,
+            evictableKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS],
+        });
+        // Required lazily so LHNTestUtils registers its @react-navigation/native mock before the engine's imports load it
+        // eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
+        require('@userActions/OnyxDerived').default();
+    });
+
+    beforeEach(async () => {
+        global.fetch = TestHelper.getGlobalFetchMock();
+        wrapOnyxWithWaitForBatchedUpdates(Onyx);
+        setHasRadio(true);
+        await TestHelper.signInWithTestUser(1, 'email1@test.com', undefined, undefined, 'One');
+        await waitForBatchedUpdates();
+    });
+
+    afterEach(async () => {
+        await Onyx.clear();
+        await waitForBatchedUpdates();
+    });
+
+    test('[SidebarLinks] should re-render when a user outside the LHN is renamed', async () => {
+        // Given 500 reports in the LHN, a guide, and a user who is in none of the reports
+        await Onyx.multiSet({
+            [ONYXKEYS.PERSONAL_DETAILS_LIST]: personalDetails,
+            [ONYXKEYS.BETAS]: [CONST.BETAS.DEFAULT_ROOMS],
+            [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.GSD,
+            [ONYXKEYS.IS_LOADING_REPORT_DATA]: false,
+            ...reports,
+        });
+        await waitForBatchedUpdates();
+
+        // Reassure runs the scenario several times against one seeded Onyx, so every run writes a name that doesn't exist yet
+        let run = 0;
+        const scenario = async () => {
+            await screen.findByTestId('lhn-options-list');
+
+            // When that user is renamed repeatedly, which makes the derived engine recompute the guide accountIDs to the same content
+            for (let index = 0; index < WRITES_PER_SCENARIO; index++) {
+                await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[UNRELATED_ACCOUNT_ID]: {displayName: `Unrelated ${run}-${index}`}});
+                await waitForBatchedUpdates();
+            }
+            run++;
+        };
+
+        // Then reassure measures what the unrelated writes cost the LHN
+        await measureRenders(<LHNTestUtils.MockedSidebarLinks />, {scenario});
+    });
+});

tests/unit/OnyxDerived/guideAccountIDsTest.ts

@@ -47,4 +47,36 @@ describe('guideAccountIDs', () => {
         expect(guideAccountIDsConfig.compute([guideListedLast], {})).toEqual([otherGuideAccountID, guideAccountID]);
         expect(guideAccountIDsConfig.compute([sameGuidesPlusAnAvatarChange], {})).toEqual([otherGuideAccountID, guideAccountID]);
     });
+
+    it('should return the current value when the guides are unchanged', () => {
+        // Given the guide accountIDs computed from the current personal details
+        const personalDetailsList: PersonalDetailsList = {
+            [guideAccountID]: {accountID: guideAccountID, login: guideLogin},
+            [memberAccountID]: {accountID: memberAccountID, login: 'member@example.com'},
+        };
+        const currentValue = guideAccountIDsConfig.compute([personalDetailsList], {});
+
+        // When a member changes their name, which leaves the set of guides as it was
+        const renamedMember: PersonalDetailsList = {...personalDetailsList, [memberAccountID]: {accountID: memberAccountID, login: 'member@example.com', displayName: 'Renamed'}};
+
+        // Then the same array comes back, so consumers comparing by reference don't re-render
+        expect(guideAccountIDsConfig.compute([renamedMember], {currentValue})).toBe(currentValue);
+    });
+
+    it('should return a new array when a guide is removed', () => {
+        // Given two guides
+        const twoGuides: PersonalDetailsList = {
+            [guideAccountID]: {accountID: guideAccountID, login: guideLogin},
+            [otherGuideAccountID]: {accountID: otherGuideAccountID, login: otherGuideLogin},
+        };
+        const currentValue = guideAccountIDsConfig.compute([twoGuides], {});
+
+        // When one of them leaves the personal details
+        const oneGuide: PersonalDetailsList = {[guideAccountID]: {accountID: guideAccountID, login: guideLogin}};
+        const result = guideAccountIDsConfig.compute([oneGuide], {currentValue});
+
+        // Then a shorter list must not be mistaken for the current one
+        expect(result).not.toBe(currentValue);
+        expect(result).toEqual([guideAccountID]);
+    });
 });

tests/unit/OnyxDerived/loginToAccountIDMapTest.ts

@@ -38,4 +38,30 @@ describe('loginToAccountIDMap', () => {
 
         expect(loginToAccountIDMapConfig.compute([optimisticHasLowerAccountID], {})).toEqual({[login]: accountID2});
     });
+
+    it('should return the current value when the logins are unchanged', () => {
+        // Given the map computed from the current personal details
+        const personalDetailsList: PersonalDetailsList = {[accountID1]: {accountID: accountID1, login}};
+        const currentValue = loginToAccountIDMapConfig.compute([personalDetailsList], {});
+
+        // When the user changes their name, which keeps every login on the same accountID
+        const renamed: PersonalDetailsList = {[accountID1]: {accountID: accountID1, login, displayName: 'Renamed'}};
+
+        // Then the same map comes back, so consumers comparing by reference don't re-render
+        expect(loginToAccountIDMapConfig.compute([renamed], {currentValue})).toBe(currentValue);
+    });
+
+    it('should return a new map when a login moves to another accountID', () => {
+        // Given a login that only has an optimistic personal detail so far
+        const optimistic: PersonalDetailsList = {[accountID1]: {accountID: accountID1, login, isOptimisticPersonalDetail: true}};
+        const currentValue = loginToAccountIDMapConfig.compute([optimistic], {});
+
+        // When the server replaces it with the real account, so the key stays and only its value changes
+        const real: PersonalDetailsList = {[accountID2]: {accountID: accountID2, login}};
+        const result = loginToAccountIDMapConfig.compute([real], {currentValue});
+
+        // Then the change must be detected, or attendees would keep pointing at the optimistic account
+        expect(result).not.toBe(currentValue);
+        expect(result).toEqual({[login]: accountID2});
+    });
 });

tests/unit/OnyxDerived/reportTransactionsAndViolationsTest.ts

@@ -112,4 +112,37 @@ describe('reportTransactionsAndViolations compute', () => {
         expect(result.reportF?.transactions?.[txKey('8')]).toBeUndefined();
         expect(result.reportF?.transactions?.[txKey('9')]).toBeTruthy();
     });
+
+    it('should return the current value when the delta touches no stored transaction', () => {
+        // Given a derived value holding one transaction
+        const transactions: OnyxCollection<Transaction> = {[txKey('10')]: makeTx('10', 'reportG')};
+        const currentValue: ReportTransactionsAndViolationsDerivedValue = {reportG: {transactions: {[txKey('10')]: makeTx('10', 'reportG')}, violations: {}}};
+
+        // When a violation lands for a transaction that isn't in the collection
+        const violations: OnyxCollection<TransactionViolation[]> = {[violationsKey('unknown')]: [dupViolation]};
+        const result = reportTransactionsAndViolationsConfig.compute([transactions, violations], {
+            sourceValues: {[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS]: violations},
+            currentValue,
+        });
+
+        // Then the same object comes back, so consumers comparing by reference don't re-render
+        expect(result).toBe(currentValue);
+    });
+
+    it('should return a new object when the delta changes a stored transaction', () => {
+        // Given a derived value holding one transaction
+        const transactions: OnyxCollection<Transaction> = {[txKey('11')]: makeTx('11', 'reportH')};
+        const currentValue: ReportTransactionsAndViolationsDerivedValue = {reportH: {transactions: {[txKey('11')]: makeTx('11', 'reportH')}, violations: {}}};
+
+        // When that transaction gets a violation
+        const violations: OnyxCollection<TransactionViolation[]> = {[violationsKey('11')]: [dupViolation]};
+        const result = reportTransactionsAndViolationsConfig.compute([transactions, violations], {
+            sourceValues: {[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS]: violations},
+            currentValue,
+        });
+
+        // Then a new object carries the violation instead of the current value being reused
+        expect(result).not.toBe(currentValue);
+        expect(result.reportH?.violations?.[violationsKey('11')]).toHaveLength(1);
+    });
 });