Skip to content

Coalesce OnyxDerived recomputations via macrotask scheduling - #95287

Merged
roryabraham merged 21 commits into
Expensify:mainfrom
callstack-internal:perf/derived-value-macrotask-scheduling-2
Jul 8, 2026
Merged

roryabraham merged 21 commits into
Expensify:mainfrom
callstack-internal:perf/derived-value-macrotask-scheduling-2

Conversation

@fabioh8010

@fabioh8010 fabioh8010 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Explanation of Change

Slack proposal: https://expensify.slack.com/archives/C08CZDJFJ77/p1783094934084619?thread_ts=1782801937.464879&cid=C08CZDJFJ77

OnyxDerived values (e.g. reportAttributes) open one Onyx connection per dependency, and each connection callback recomputes the derived value synchronously. Because Onyx delivers a single logical write (a server response touching reports + transactions + violations + report actions, etc.) as several separate broadcasts, one logical update recomputes each derived value many times back-to-back, blocking the JS thread during startup and report opens. reportAttributes alone has 13 dependencies and is one of the heaviest computes in the app.

This PR coalesces the per-dependency recomputes for each derived value into a single compute per logical update.

How it works

  1. Coalescing. recomputeDerivedValue no longer computes inline — it records the triggering dependency in a pending set and, if no flush is already scheduled, schedules one on the next macrotask. Every dependency callback from the same logical update lands in that one pending flush, so N recomputes collapse to 1.

  2. Flush-time delta reconstruction. Instead of relying on Onyx's per-broadcast sourceValue (which doesn't survive coalescing — you'd be merging N partial payloads — and is being phased out), the flush reconstructs the changed-member delta itself: for each collection dependency that fired, it diffs the current snapshot against a baseline captured at the previous flush, via a new shared getCollectionDelta helper. Onyx's structural sharing keeps unchanged members reference-equal, so this is a cheap reference scan. The first flush has no baselines, so it computes from scratch and captures baselines for subsequent diffs.

  3. scheduleMacrotask (MessageChannel). The flush is scheduled with a small scheduleMacrotask helper that uses MessageChannel on web — the same technique React's scheduler uses — because setTimeout is throttled in background tabs (rules varies through different browsers) and clamped to 4ms after nested calls. It falls back to setTimeout on React Native (no MessageChannel; the JS thread is fully suspended in the background there anyway) and in tests.

  4. Incremental configs hardened to honor every coalesced trigger. Configs that update incrementally previously assumed a single triggering dependency per compute. Coalescing can now deliver several changed dependencies in one compute, so any config that branched on "which one changed" was updated to process all of them:

    • reportAttributes — its personal-details fast-path short-circuits only when personal details are the sole trigger.
    • reportTransactionsAndViolations — processes the union of transactions that changed directly and transactions whose violations changed (was an if/else that dropped the second trigger).
    • sortedReportActions — takes its report-actions incremental path only when report actions are the sole trigger; a batched REPORT change falls back to a full recompute.

With recomputes coalesced, one logical update produces exactly one compute of each derived value instead of one-per-dependency. Derived values become eventually consistent within one macrotask of the originating write; consumers re-render normally through useOnyx when the single updated value lands.

Fixed Issues

$ #95301
PROPOSAL:

Tests

  1. Pin/Unpin two reports in LHN, assert they were pinned/unpinned and LHN was re-ordered correctly.
  2. Draft a message to two reports in LHN, assert they were marked and LHN was re-ordered correctly.
  3. Mark two reports in LHN as read/unread, assert they were marked/unmarked, the Unread count indicator changed and LHN was re-ordered correctly.
  4. From another user send a expense to this one, mark it as paid, assert it appeared correctly and the To-dos count indicator changed.
  5. Send a message to a user you never sent before, assert it appeared in LHN.
  6. Delete that message, assert the chat disappeared from LHN.

MessageChannel testing (web-only)

I'm using Firefox as it's easier to demonstrate the differences in the latencies when comparing with setTimeout.

Before testing, apply these changes to src/libs/actions/OnyxDerived/index.ts:

diff --git a/src/libs/actions/OnyxDerived/index.ts b/src/libs/actions/OnyxDerived/index.ts
index 0413646e141..2e933211b32 100644
--- a/src/libs/actions/OnyxDerived/index.ts
+++ b/src/libs/actions/OnyxDerived/index.ts
@@ -71,6 +71,8 @@ function init() {
 
             // Coalesce per-dependency recomputes from one logical change into a single compute on the next macrotask.
             let flushScheduled = false;
+            // TEMP [perf] timestamp captured when a flush is scheduled, to measure schedule -> flush latency.
+            let scheduledAt = 0;
             // Dependency indexes that fired since the last flush; their deltas are reconstructed at flush time.
             const pendingDependencyIndexes = new Set<number>();
             // Snapshot of each collection dependency captured at the last flush. We diff the current snapshot
@@ -109,6 +111,13 @@ function init() {
             const flushRecompute = () => {
                 flushScheduled = false;
 
+                // TEMP [perf] measure how long the scheduled macrotask actually took to run (schedule -> flush).
+                // In a backgrounded web tab, setTimeout is throttled (~1s+) while MessageChannel is not.
+                const flushLatency = Math.round(performance.now() - scheduledAt);
+                const tabHidden = typeof document !== 'undefined' && document.visibilityState === 'hidden';
+                // eslint-disable-next-line no-console
+                console.log(`[perf][OnyxDerived] ${key} flush latency ${flushLatency}ms${tabHidden ? ' (tab hidden)' : ''}`);
+
                 // Reconstruct the source values at flush time by diffing each dependency that fired since the
                 // last flush against its last-flushed snapshot. On the very first flush we have no baselines, so
                 // we compute from scratch (undefined sourceValues) and capture snapshots for future diffs.
@@ -168,6 +177,8 @@ function init() {
                     return;
                 }
                 flushScheduled = true;
+                // TEMP [perf] stamp the schedule time so flushRecompute can log schedule -> flush latency.
+                scheduledAt = performance.now();
                 scheduleMacrotask(flushRecompute);
             };
 
  1. With an logged in account, duplicate the current Tab and wait for them to stop processing.
  2. In Tab A, execute the following command in the console: await Onyx.merge('session', {email: 'test@email.com'});.
  3. Go to Tab B and check [perf][OnyxDerived] logs. Assert the flush latency is lesser than 1000 ms.

If you change scheduleMacrotask.ts to always use the setTimeout fallback, the flush latency will be greater than 1000 ms.

  • Verify that no errors appear in the JS console

Offline tests

N/A

QA Steps

Same as Tests, except MessageChannel testing (web-only).

  • Verify that no errors appear in the JS console

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: mWeb Chrome
iOS: Native
Screen.Recording.2026-07-06.at.10.38.55.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-07-06.at.10.32.24.mov

MessageChannel testing (web-only)

Screen.Recording.2026-07-06.at.10.21.14.mov

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is needed because of getCollectionDelta, it was the same change I applied in 076914f in #93438

@fabioh8010

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0ebbee3bd5

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

Comment thread src/libs/actions/OnyxDerived/index.ts Outdated
}

pendingDependencyIndexes.clear();
runCompute(sourceValues);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve secondary triggers in coalesced recomputes

When this flush passes all pending dependency changes to a config at once, several existing incremental configs still assume only one source key is present. For example, reportTransactionsAndViolations.compute uses if (transactionsUpdates) ... else if (transactionViolationsUpdates) ..., so a single Onyx.update that changes transaction A and violations for transaction B now only processes A; before this change the callbacks ran separately and B's violation map was updated. The same pattern exists in sortedReportActions when REPORT_ACTIONS is batched with REPORT/NETWORK, so coalescing needs to either force a full recompute for mixed triggers or update those configs to handle all triggers together.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in 0842b78.

However I'm not implementing the general suggested fix (full recompute on any mixed trigger) because that would defeat the goal of this PR, so I'm instead fixing reportTransactionsAndViolations and sortedReportActions to account for this scenario. The other derived files were checked and they are already safe.

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

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.

Files with missing lines Coverage Δ
...bs/actions/OnyxDerived/configs/reportAttributes.ts 95.67% <100.00%> (-2.16%) ⬇️
...Derived/configs/reportTransactionsAndViolations.ts 100.00% <100.00%> (ø)
...actions/OnyxDerived/configs/sortedReportActions.ts 94.73% <100.00%> (+0.09%) ⬆️
src/libs/actions/OnyxDerived/index.ts 98.21% <100.00%> (+1.19%) ⬆️
src/libs/actions/OnyxDerived/utils.ts 100.00% <100.00%> (ø)
src/libs/getCollectionDelta.ts 100.00% <100.00%> (ø)
src/libs/scheduleMacrotask.ts 100.00% <100.00%> (ø)
...ctions/OnyxDerived/configs/visibleReportActions.ts 96.42% <94.44%> (+1.60%) ⬆️
... and 21 files with indirect coverage changes

@fabioh8010
fabioh8010 marked this pull request as ready for review July 6, 2026 10:00
@fabioh8010
fabioh8010 requested review from a team as code owners July 6, 2026 10:00
@melvin-bot
melvin-bot Bot requested review from heyjennahay and shubham1206agra and removed request for a team July 6, 2026 10:00
@melvin-bot

melvin-bot Bot commented Jul 6, 2026

Copy link
Copy Markdown

@shubham1206agra 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]

Comment thread src/hooks/useCollectionDelta.ts Outdated
Comment thread src/libs/scheduleMacrotask.ts Outdated
@roryabraham
roryabraham removed the request for review from heyjennahay July 8, 2026 16:48
@roryabraham
roryabraham merged commit 790fafb into Expensify:main Jul 8, 2026
36 checks passed
@OSBotify

OSBotify commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

✋ 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

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

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

@OSBotify

OSBotify commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/roryabraham in version: 9.4.32-0 🚀

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

@MelvinBot

Copy link
Copy Markdown
Contributor

🤖 I reviewed the changes in this PR to determine whether any Expensify help site articles under docs/articles need updating.

No help site changes are required.

This PR is a purely internal performance optimization — it coalesces per-dependency OnyxDerived recomputations into a single compute per logical update via macrotask scheduling. Every changed file is engineering-internal:

  • src/libs/actions/OnyxDerived/* — derived-value computation logic (reportAttributes, reportTransactionsAndViolations, sortedReportActions, visibleReportActions, index, types, utils)
  • src/libs/getCollectionDelta.ts, src/libs/scheduleMacrotask.ts — new internal helpers
  • cspell.json — spell-check dictionary entry (macrotask)
  • tests/** — unit/UI tests

There is no change to user-facing behavior: no new features, UI, tabs, settings, buttons, or workflows. Derived values remain eventually consistent within one macrotask of the originating write, and consumers re-render as before through useOnyx. The help site documents product features and behavior, none of which is affected here.

Because no changes are needed, I did not create a draft docs PR.

@fabioh8010, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR Ready for review

@mitarachim

Copy link
Copy Markdown

Deploy Blocker #95696 was identified to be related to this PR.

@mitarachim

Copy link
Copy Markdown

Deploy Blocker #95698 was identified to be related to this PR.

@mitarachim

Copy link
Copy Markdown

Deploy Blocker #95700 was identified to be related to this PR.

inimaga pushed a commit that referenced this pull request Jul 9, 2026
Reverts the coalescing engine that caused deploy blockers, scoped to just the
OnyxDerived changes on top of current main. Restores the synchronous, per-
dependency recompute engine (Onyx sourceValue based) and keeps
waitForCollectionCallback, matching main's current Onyx 3.0.86 (the onyx-store
work that removed it, #93436, was already reverted separately). Removes
getCollectionDelta, scheduleMacrotask, and the triggeredKeys/flush changes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mitarachim

Copy link
Copy Markdown

Deploy Blocker #95721 was identified to be related to this PR.

@jponikarchuk

Copy link
Copy Markdown

Deploy Blocker #95753 was identified to be related to this PR.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/grgia in version: 9.4.32-3 🚀

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

Bundle Size Analysis (Sentry):

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.

10 participants