perf(replay): avoid repeated shadow connectivity walks - #4816
Open
marandaneto wants to merge 1 commit into
Open
Conversation
Contributor
|
Contributor
Replay incident risk checkThis diff touches code involved in past incidents. This is a heads-up, not a verdict: read the matched sections of INCIDENTS.md and answer their review questions before merging. For a judgment on whether this diff has the same failure mode, run the |
marandaneto
marked this pull request as ready for review
September 6, 2026 19:20
marandaneto
requested review from
TueHaulund,
arnohillen,
fasyy612 and
ksvat
and removed request for
a team
September 6, 2026 19:20
Contributor
Contributor
|
Size Change: +33.2 kB (+0.16%) Total Size: 20.5 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
|
Reviews (1): Last reviewed commit: "perf(replay): avoid repeated shadow conn..." | Re-trigger Greptile |
Contributor
|
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
Session recording checks whether queued nodes are still connected before serializing them. For shadow descendants, the existing fallback repeatedly walks shadow hosts and checks document containment.
This is stacked directly on #4812 (
perf/replay-remaining-hotspots). #4815 is an independent privacy fix and is not included in this branch or either timed build. This does not resolve #4217.Changes
Keep the existing owner-document guard and light-DOM containment fast path. If containment is false, read live connectivity using a native
Node.isConnectedgetter. Missing or non-native implementations retain the old algorithm. Validate the getter at first use and cache only its function, never a node's connectivity value.Tests cover moves, adoption, detached iframe documents, open/closed shadow roots, missing getters and prototype patching before and after recording starts. A focused work-count regression reduces 400 containment calls / 700 root lookups to 300 / zero for 300 mixed light/shadow queries.
Measurements against #4812 alone
Five complete alternating samples per build and shape, approximately 50k nodes, 11 moves, compression on, Chromium 136 on an Apple M4 Pro. The same unprofiled harness was used sequentially, without concurrent builds or tests.
These are descriptive medians, not statistical significance or customer guarantees. The shadow improvement is about 3–4%, not the roughly 8% seen in earlier samples. Every paired repeated-shadow sample improved. Four mixed-shadow pairs improved and one tied. Light-DOM results are mixed, with the flat mixed-move median about 5% slower. This draft is scoped to the shadow benefit, not a general performance win. The timed-out final candidate arm was excluded entirely and rerun, rather than selecting its completed shapes.
Recorder size changes by +295 raw / +86 gzip bytes. Detailed methodology and caveats are in
packages/browser/scripts/benchmark-replay-connectivity.md.Validation
337 recording/accessor tests passed, with 2 existing skips. All 18 Chromium/Firefox/WebKit connectivity and existing masking cases passed. All 40 complete comparison arms passed intermediate replay, ordering, privacy, duplicate-ID and drop/recovery checks. Compression on/off and 4x CPU-throttled 10k smoke matrices passed, as did build/type checks, targeted lint, ES5/ES6 checks and committed-branch autoreview.
For compatibility testing only, #4815 commit
089f90663was temporarily applied to both builds: 54 baseline blocking cases and 72 candidate blocking/connectivity/masking cases passed. The compressed/uncompressed cancellation matrix also passed with that overlay. The overlay was removed afterward. The bare performance stack retains the known held-shadow privacy bug; it is not claimed to pass that extended cancellation matrix without the independent fix.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file (equivalent patch entry authored directly)🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Implemented with Pi, bash, Vitest and Playwright under marandaneto's direction, with Pi autoreview of the committed branch. No session link is published. Human review is required.
A broader connectivity fast path had mixed timing results and was rejected. An unvalidated getter also dropped recordings under prototype patching, which was caught by a failing real-browser test. The final change preserves the light-DOM path and falls back when the optional getter is not native. No asynchronous recording, cached privacy eligibility or mutation coalescing is included.