fix(persistence): skip save() when serialized props are unchanged#3687
fix(persistence): skip save() when serialized props are unchanged#3687pauldambra wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Size Change: +1.34 kB (+0.01%) Total Size: 16.4 MB
ℹ️ View Unchanged
|
1e22336 to
ecfb184
Compare
ecfb184 to
2179a04
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2179a04f81
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Size Change: +2.25 kB (+0.01%) Total Size: 16.5 MB
ℹ️ View Unchanged
|
|
2179a04 to
615d2cf
Compare
|
Deployment failed with the following error: |
615d2cf to
5092516
Compare
5092516 to
ec37d27
Compare
ec37d27 to
b84aecc
Compare
Many internal code paths call `Persistence.save()` after touching a
property. When the resulting props serialize to the same string as the
previous successful write, we still wrote identical bytes to localStorage
(or the cookie). Each identical localStorage write fires a `storage`
event on every other same-origin tab — Chrome's mojo IPC allocates the
payload buffer for that event in `partition_alloc/buffer` regardless of
whether any listener reacts. With a multi-hundred-KB blob, the per-tab
off-heap pressure is significant.
Add a `_lastSavedSerialized` cache in `PostHogPersistence`. In `save()`,
serialize `props` and compare against the cache; skip the storage call
if they match. Update the cache on every successful write.
The cache is cleared in `remove()` because the storage entry no longer
exists — the next save must write through regardless of whether the
in-memory props look unchanged. Tests for the cookie/localStorage modes
had to be adjusted to call `mockClear()` after the setup writes because
`cookieStore` and `localStore` are shared singletons.
The existing 'should write to storage if opt_out_persistence_by_default
and opt_out_capturing_by_default is false' test was calling
`persistence.save()` with no state change and asserting that it wrote;
now correctly asserts via `persistence.register({...})`.
Generated-By: PostHog Code
Task-Id: 3946962f-7365-4b7b-8f5f-8f9309ed88c6
Generated-By: PostHog Code Task-Id: 3946962f-7365-4b7b-8f5f-8f9309ed88c6
…es and stale cookie options
Two regressions introduced by the no-op rejection in save():
1. JSON.stringify rejects BigInt and circular refs. Before, those
values were caught by the storage layer's try/catch and silently
dropped; the new preflight fingerprint threw out of save() into
the caller. Wrap the fingerprint in try/catch and fall through to
_storage._set so the storage layer keeps its existing handling.
2. The fingerprint only covered serialized props, but the call to
_storage._set also takes _expire_days, _cross_subdomain and
_secure. A `set_config({ cookie_expiration: 90 })` that didn't
touch any property would no-op the write, leaving the cookie on
the old Expires header. Include all four arguments in the
fingerprint so changes to cookie options trigger a write.
Generated-By: PostHog Code
Task-Id: 3946962f-7365-4b7b-8f5f-8f9309ed88c6
Generated-By: PostHog Code Task-Id: 3946962f-7365-4b7b-8f5f-8f9309ed88c6
639ae7d to
eb0a2e5
Compare
50d0da8 to
3619608
Compare

Problem
there is an open P2 bug in chromium that local storage writes cause a memory leak https://issues.chromium.org/issues/394823902
we write to local storage a lot
the PH memory leak in PostHog/posthog#32479 is fiendish to replicate in a test
Changes
let's write to local storage less since it can't hurt...
second, let's not write duplicates... it's a no-op
Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file