perf(react-native): coalesce storage writes with a debounce window#3701
Merged
Conversation
RN persists the whole storage blob on every change, so a burst of captures re-serializes a growing blob N times (O(n^2)) and does N disk writes. Batch writes into a 100ms window while keeping the in-memory copy current; event flush, AppState background, shutdown, and login/logout still drain synchronously so the queue and identity stay durable. Adds TDD tests for coalescing and each synchronous drain, plus a Storage demo screen in example-expo-53. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Contributor
Contributor
|
Size Change: +1.43 kB (+0.01%) Total Size: 16.4 MB
ℹ️ View Unchanged
|
5 tasks
Member
|
|
Exceptions are crash-correlated: a fatal one can terminate the app within the 100ms debounce window, so the exception write is initiated synchronously (draining the scheduled write now) rather than waiting out the timer. Otherwise the exception that crashed the app would be lost. Events pipeline only, since the exception is an event. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Refine captureException durability. Drain both storage pipelines to disk only when the exception is fatal (tagged via $exception_level), rather than unconditionally on every captureException: console autocapture turns every console.error/warn into a captureException, so an unconditional drain would re-introduce write amplification. Handled exceptions persist via the normal debounce. Soften the durability comments to say writes are initiated (best-effort, async) rather than guaranteed to land before a crash. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
@marandaneto Added a sync drain on fatal exceptions in |
Member
|
PR is very chatty with very long comments, can we simplify? |
dustinbyrne
approved these changes
May 29, 2026
Contributor
dustinbyrne
left a comment
dustinbyrne
left a comment
There was a problem hiding this comment.
Makes sense to me. I think the decision to debounce persistence writes over 100ms windows is a good call. Definitely an improvement over batching once per tick of the event loop 👍
| @@ -16,6 +32,12 @@ export class PostHogRNStorage { | |||
| preloadPromise: Promise<void> | undefined | |||
| private _storageKey: string | |||
| private _pendingPromises: Set<Promise<void>> = new Set() | |||
Contributor
There was a problem hiding this comment.
I don't know that the concept of multiple pending promises makes sense anymore, considering we want to limit the maximum number of times we're writing to once per 100ms.
Allowing multiple in-flight writes seems like it could introduce ordering issues - though, the actual risk of this seems relatively low?
Contributor
Author
There was a problem hiding this comment.
Agreed the actual risk is small since memoryCache is the authoritative latest state. And we can keep it for ergonomics.
…omments - Drain on optIn() / optOut(). Consent must be durable: a hard-kill within the debounce window before any flush/background/shutdown would lose the OptedOut write, and the optedOut getter falls back to !defaultOptIn so capture silently resumes for a user who explicitly opted out. Same crash-window the PR closes for logout/identify. - New storage test: the debounced write fires on its own after the window. Every other functional test forced the write via waitForPersist, so a regression where the timer never armed would ship green. - New storage test: the timer is not reset by later mutations (arm-once, no starvation). Guards against a textbook clear-and-reschedule debounce. - Refined the fatal-exception assertion to check the parsed events queue for an item whose message.event is \$exception, rather than a substring match that could pass on only the level tag round-tripping. - Trimmed the verbose drain-site comments and centralized the waitForPersist contract on the method doc (addresses the PR feedback). - Updated the changeset to note custom-storage write-timing and the ~100ms loss window. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
React Native apps that fire many events quickly (e.g. a screen that captures several events on navigation) can stutter on low-end Android. A customer is experiencing performance implications on low end androids (#3493).
It comes from how the RN SDK stores data. Native iOS/Android write one file per event. The RN SDK instead keeps everything in a single in-memory object and, on every change, serializes that whole object and writes the entire blob to disk (AsyncStorage). So a burst of N captures re-serializes a growing blob N times (O(n²) bytes) and does N disk writes — all on the JS thread. Reads aren't affected; they come from the in-memory copy. Only the disk write is costly.
Changes
Batch the disk writes instead of writing on every change:
Two write triggers, whichever comes first wins:
flushAt) or every 10s, and a flush forces a write — so in a fast burst you get ~1 write per 20 events.Measured on a Pixel (100 captures in a tight loop): 203 writes / 8.7 MB → 1 write / 85 KB.
Why 100ms: a small simulation swept window sizes against realistic traffic. Beyond ~100ms there's no further reduction (the write count is already floored by
flushAt), while a larger window only widens how much could be lost if the app is killed before the timer fires. 100ms reaches that floor with the smallest loss window.Tests are TDD-style — each behavior has a test confirmed to fail without the change. 345 RN + 587 core pass. Also adds a "Storage" demo screen to
example-expo-53.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file