Repository navigation
perf(web): fix #574 by bounding the reconciler's suffix walk, not a per-row liveness check - #4133
Conversation
… liveness check The suffix walk is bounded to the open range [aStart, aEnd); inside that range this pass never detaches a node, so neither walk needs isLive. The bound alone closes #574, where the unbounded suffix walk reached back past aStart to a node a map-branch replace had taken out while it was still pending in b. Every anchor read and destructive branch keeps its ownership gate. Measured with the call profiler on the Octane js-framework fixture, swap of rows 1 and 998 in 1000: 29.5 -> 25.0 µs (Vapor 9.0). reconcileArrays inclusive 24.4 -> 13.8 µs; isLive (8.4 µs self) gone. uibench through the board runner, oracle identical to the previous run (96 cases, 32,543 shared identities): tree/[500] virtual_dom_worst_case -17%, snabbdom_worst_case -14%, moveFromEndToStart -11%, tree removeAll -7 to -11%, reverse flat. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: bfa6a89 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size (brotli, eager entry chunk)
|
Coverage Report for CI Build 39114342011Coverage remained the same at 77.564%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Merging this PR will improve performance by 26.64%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | head: remove first + insert first (1000 rows) |
3 ms | 2 ms | +51.9% |
| ⚡ | tail: remove last + insert last (1000 rows) |
2.5 ms | 1.9 ms | +29.58% |
| ⚡ | tail, keyed by fn: remove last + insert last (1000 rows) |
2.9 ms | 2.4 ms | +23.25% |
| ⚡ | memo + sync render effect only (reference) |
33 ms | 28.3 ms | +16.59% |
| ⚡ |
middle: remove at 500 + insert at 500 (1000 rows) |
5 ms | 4.3 ms | +15.17% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing perf/reconcile-walk-gate (2b5f46f) with next (5c8ec96)
Footnotes
-
5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Journal §5–§12 of the Vapor lane: the signal-favoring fixture form, the selection cost traced to the store key-count drift (#4132) and the rc.0/rc.1 comparison, swap's history back to rc.0 and the reconciler walk change (#4133), and the move-hint probe — built, measured against a back-to-back control, and parked (swap 25 -> 15.8 µs where it applies, a 3% tax where it does not, ~300 B brotli per app bundle, never for a keyed-by-function For). A pointer under the May metadata-channel deferral and the lane ledger updated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Generated by Claude Code |
…pped a migrated one Review on #4133: with the per-node check gone from the prefix and suffix walks, the node the append branch anchors on can be one user code moved away. `reconcileArrays(p, [x], [y, x])` with x detached threw NotFoundError from insertBefore, which halts reactivity; through insert, rendering [z, x], moving x into a modal and then rendering [y, x] left the slot empty. The suggested `isLive(b[bEnd]) ? b[bEnd] : after` stops the throw but lands the insert past the live suffix when the migrated node is not the suffix's last, and the bStart branch had the same fallback when the node before the insert is the migrated one. The anchor is now the last live prefix node's successor, else the first suffix node still ours, else `after`. The walks stay check-free; the loop runs only in the append branch and stops at its first live node, which is the first node in every case without migration. Tests: a migrated suffix node under reconcileArrays (no throw); a prepend over a migrated tail through insert (no throw, insert inside the slot, still reactive after); an insert after a migrated prefix node and before a migrated suffix head (both land before the live suffix). All four fail without the fix, and the first failure halts reactivity for the rest of the file. Web default 1383, hydrate 509, server 1719. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… pin the May migration rule Review on #4133. The reconciler keeps the slot-ownership design from May (78bb855c): identity-only prefix and suffix walks, every destructive op gated on ownership. A node moved away that sits in the unchanged prefix or suffix stays where it was moved; #574 stays fixed by the bounded suffix walk. Map branch: an old node that is no longer ours was "replaced" by inserting the new node at the region's end, and the sequence branch anchored on the region's end the same way — since May. [a, b] with a moved away, then [d, b, a], came out [b, d, a]. The dead node is now consumed up front and the new node lands when its turn comes. An oracle fuzz of the May rule (nothing throws; every unmoved node of the new list is in the slot, in list order; a moved node the list dropped stays put), 20k seeds with and without a slot marker: this PR before the change 7,510 out of order, next 9,714, now 0. The map branch is also smaller: list-bearing scenarios come out ~10 B minified under their recorded sizes. Tests pin the rule: a moved-away node in the unchanged prefix, a removed node in the unchanged suffix, a moved-away node the list drops, and the window placed in order around a moved-away node. On next six of the new tests fail; on this PR without the change, the ordering test. Web default 1387, hydrate 509, server 1719. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ryansolid
changed the title
…oncile-walk-gate
…ogether on next Each passed under the cap alone (#4129 +51 B minified, #4136 +4 B); together on next @ 5c8ec96 they measure 9,963 B brotli / 28,064 B minified, 3 B over the brotli cap and 39 B past the minified allowance, so the size gate failed on every PR against next. This PR touches no signals code; the merge of next brought the row in. Cap 9.96 -> 9.98 KB, recorded minified 28,005 -> 28,064, with a ledger line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oncile-walk-gate # Conflicts: # scripts/size/scenarios.js
…im, the May rule, final numbers; lane closed Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 participants
What
reconcileArrays(packages/web/src/reconcile.ts) goes back to the slot-ownership design from May (dom-expressions 78bb855c) and fixes #574 at its cause instead of with a per-row check:[aStart, aEnd).Every destructive operation keeps its ownership gate.
Why
#574, at its cause. The single-anchor swap (2fe6310f, May 7) could leave the suffix walk reaching back past
aStartinto entries the pass had already consumed, meeting a node a map-branch replace had taken out while it was still pending inb; it counted that node as a live common end and never reinserted it. The August patch (9052dcf5) fixed this by adding the ownership check to every row of both walks. Bounding the walk to the open range removes the case: inside it, this pass never detaches a node — replace, remove and swap all consume the node they take out. On clean arrays the result matchesnextexactly (review's differential fuzz: 0 divergences in 100k seeds).Cost of the per-row check. It was never measured when it went in. On a 1000-row list the walks run once per row per update, so it was ~9 µs of a 30 µs swap and a matching share of every long reorder. On js-framework-benchmark's millisecond, paint-dominated timings it is invisible.
Migration rule, as promised in May. Nothing throws; a node moved out of the slot is never destroyed by it; destructive operations touch only nodes the slot owns. A node moved away that sits in the unchanged prefix or suffix stays where it was moved. That was the behaviour from May 16 to Aug 21; the August per-row check had turned it into a reclaim. Framework-performed migration (#2030, #2357 and the rest of the May tests) passes unchanged.
Two bugs the review surfaced.
reconcileArrays(p, [x], [y, x])with x detached threwNotFoundError, which halts reactivity. The suggestedisLive(b[bEnd]) ? b[bEnd] : afterstops the throw but lands the insert past the live suffix when the moved node is not the suffix's last; the anchor is now the first node the slot still owns.[a, b]with a moved away, then[d, b, a], came out[b, d, a]. Consuming the dead node fixes it.Verified
An oracle fuzz of the May rule checked that nothing throws, that every unmoved node of the new list is in the slot in list order, and that a moved node the list dropped stays put. It ran 20,000 seeds with and without a slot marker:
nextNo variant threw or lost an owned node.
Measured
All on
next@ e1ced78 (with #4129, #4132, #4136 and #4138), unminified Octane fixtures, same machine, interleaved.Call profiler, js-framework swap of rows 1 and 998, µs per click:
nextOctane board, two rounds each, Solid ÷ Vapor (same-run pairing), this PR against
next:nextone_changeThe map-branch change costs nothing on swap and about 1–2% on rotations and displacements (+1.7% / +2.2% vs Vapor, near the ~1% noise there); it is a correctness fix for the order of the changed window. uibench's 96-case oracle was identical in all six runs (23,331 elements, 32,543 shared identities).
A variant that kept the August reclaim with the cheapest possible check (
parentNodeonly when there is no marker) kept none of the swap gain and only 15–30% of the rest: the cost is the per-row DOM read itself.CodSpeed:
single-item-ops1000 rows head +53%, tail +30%, tail keyed by fn +24%, middle +16%;reconcile-permuteunchanged.Size
List-bearing scenarios ~10 B minified under their recorded sizes; brotli moves are layout noise.
Tests
test/runtime/insert.spec.js, new:insert.On
nextsix of these fail; without the map-branch change, the ordering test does. The #574 test and the May migration tests pass unchanged. Web default 1387, hydrate 509, server 1719.🤖 Generated with Claude Code