perf(replay): avoid per-node mutation traversal callbacks - #4811
Open
marandaneto wants to merge 1 commit into
Open
perf(replay): avoid per-node mutation traversal callbacks#4811marandaneto wants to merge 1 commit into
marandaneto wants to merge 1 commit into
Conversation
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 |
Contributor
|
Contributor
Contributor
|
Size Change: +5.33 kB (+0.03%) Total Size: 20.5 MB 📦 View Changed
ℹ️ View Unchanged
|
marandaneto
marked this pull request as ready for review
September 6, 2026 09:45
marandaneto
requested review from
TueHaulund,
arnohillen,
fasyy612 and
ksvat
and removed request for
a team
September 6, 2026 09:45
Contributor
|
Reviews (1): Last reviewed commit: "perf(replay): avoid per-node mutation tr..." | Re-trigger Greptile |
23 tasks
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
1 participant
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
Repeatedly moving a large subtree makes session recording walk its descendants many times. Each walk also creates per-node callbacks for
NodeList.forEach, adding main-thread overhead.This PR is stacked on #4808. The baseline already includes the getter-cache, serialization-options and mirror-removal improvements. This is a partial improvement for #4217, not a complete fix.
Changes
forEachcalls ingenAddsanddeepDeletewith indexed loops. Preserve the initial list length, read each child from the live list and skip slots removed during recursion.forEachcalls instead of 201. The remaining 201 calls belong to the unchangedprocessRemovestraversal.Performance
Three alternating baseline/candidate runs per shape, approximately 50k nodes, compression enabled, Apple M4 Pro and Chromium 136.0.7103.25. Both builds used the same harness source without profiling or counter instrumentation. No builds or tests ran concurrently with timings.
Mixed-move workloads also improved. Startup and child removal were effectively flat. These are descriptive desktop samples from a deliberate stress case, not statistical significance or typical customer impact. The candidate still blocks for roughly 300–400 ms.
Separate diagnostics confirmed identical node visits and set-operation counts before/after at 1, 3, 11 and 21 moves across all four shapes. Recorder size increased by 115 raw bytes and 80 gzip bytes.
Investigation, reproduction commands and full results.
Validation and risk
9e3a6f22553b55bda60422e0631bc672d2be9d99with no actionable findings, usingautoreview --mode branch --base origin/perf/replay-mirror-ordering.The incident checker flagged serializer correctness. Style serialization is unchanged, and real-browser checks verify intermediate ordering, masking, CSS and shadow state. There are no changes to lazy-load contracts, persisted fields, sampling, session rotation or flush policies. No recording-volume policy change is intended.
Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
posthog-jspatch changeset🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Implemented with Pi using repository inspection, Chromium profiling, Vitest, Playwright and isolated autoreview. The change only replaces child enumeration. Traversal deduplication was not attempted because intervening removals and parent bookkeeping have different semantics from mirror cleanup.
Diagnostics remain separate from production timing comparisons. No asynchronous recording architecture was introduced. Human review is required.