Skip to content

[PERF] Reuse cursor slots in NewTreeBuilder - #21609

Draft
NullVoxPopuli wants to merge 1 commit into
emberjs:mainfrom
NullVoxPopuli:nvp/perf/tree-builder-cursors
Draft

[PERF] Reuse cursor slots in NewTreeBuilder#21609
NullVoxPopuli wants to merge 1 commit into
emberjs:mainfrom
NullVoxPopuli:nvp/perf/tree-builder-cursors

Conversation

@NullVoxPopuli

@NullVoxPopuli NullVoxPopuli commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

NewTreeBuilder keeps its cursors in an array indexed by cursorDepth and reuses CursorImpl slots on pushElement, since opening an element is one of the most frequent VM operations. The modifier and block stacks become plain arrays too, and RehydrateTree pushes its richer cursors through a new pushCursor.

🤖 Generated with Claude Code

@NullVoxPopuli @claude
[PERF] Reuse cursor slots in NewTreeBuilder
31fb845 
Split out of nvp/simplify-some-vm-hot-paths.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@NullVoxPopuli
NullVoxPopuli marked this pull request as draft September 9, 2026 23:13
@NullVoxPopuli

This comment was marked as outdated.

Sign in to view
@NullVoxPopuli

This comment was marked as outdated.

Sign in to view
@NullVoxPopuli

This comment was marked as outdated.

Sign in to view
@NullVoxPopuli

NullVoxPopuli commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Re-test after moving to draft. Control rebuilt from scratch off main (same commit as before, bdb2580), PR branch rebuilt from scratch (unchanged, 31fb845), machine load around 1.7 at start. This is run 3 for this PR.

Benchmark: main (bdb2580) vs nvp/perf/tree-builder-cursors (31fb845)

Ran via pnpm bench (the krausest js-framework-benchmark scenario in smoke-tests/benchmark-app, tracerbench, 20 samples each, 8x CPU throttle). Negative delta means the PR is faster. Only rows tracerbench marks significant (p < 0.05) are meaningful.

Phase main median PR median Δ (estimator) 95% CI Significant
Total duration 10,666ms 10,938ms 212ms (+2.0%) 40ms .. 349ms ✅
render 168ms 165ms -3ms (-2.1%) -8ms .. 0ms
render1000Items1 387ms 385ms -3ms (-0.8%) -7ms .. 3ms
clearItems1 42ms 43ms 0ms (+0.6%) 0ms .. 1ms
render1000Items2 327ms 330ms 2ms (+0.6%) -5ms .. 8ms
clearItems2 140ms 139ms -1ms (-0.6%) -2ms .. 1ms
render10000Items1 3,057ms 3,172ms 95ms (+3.1%) 26ms .. 143ms ✅
clearManyItems1 393ms 410ms 16ms (+4.0%) 2ms .. 22ms ✅
render10000Items2 3,011ms 3,052ms 38ms (+1.3%) -43ms .. 93ms
clearManyItems2 283ms 277ms -6ms (-2.0%) -10ms .. -1ms ✅
render1000Items3 296ms 334ms 37ms (+12.4%) 23ms .. 52ms ✅
append1000Items1 399ms 399ms -3ms (-0.6%) -48ms .. 14ms
append1000Items2 364ms 373ms 3ms (+0.9%) -15ms .. 16ms
updateEvery10thItem1 322ms 341ms 25ms (+7.6%) 13ms .. 42ms ✅
updateEvery10thItem2 361ms 341ms -14ms (-3.9%) -29ms .. 11ms
selectFirstRow1 91ms 88ms -4ms (-4.2%) -9ms .. 1ms
selectSecondRow1 74ms 72ms -3ms (-3.4%) -6ms .. 0ms
removeFirstRow1 230ms 227ms -1ms (-0.4%) -8ms .. 8ms
removeSecondRow1 222ms 219ms 0ms (+0.1%) -8ms .. 14ms
swapRows1 161ms 172ms 11ms (+6.8%) 4ms .. 18ms ✅
swapRows2 158ms 157ms -1ms (-0.3%) -9ms .. 6ms
clearItems4 112ms 116ms 5ms (+4.4%) 2ms .. 9ms ✅
paint 13ms 11ms -1ms (-4.0%) -4ms .. 2ms

@NullVoxPopuli

NullVoxPopuli commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Across three runs the consistent rows are clearManyItems1 +4 to 7% and clearItems4 +3 to 5%, slower every time. render1000Items3 was +11 to 12% slower in two of three runs, and total duration reached significance only in this run (+2%). Nothing is consistently faster. That still points at the clear path, so the retained cursor slots holding stale element/nextSibling references through teardown remains the first thing I would test, by nulling the slot in popElement and re-measuring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@NullVoxPopuli