Skip to content

[PERF] Cut per-VM allocations in the append VM - #21606

Draft
NullVoxPopuli wants to merge 2 commits into
emberjs:mainfrom
NullVoxPopuli:nvp/perf/append-vm-stacks
Draft

[PERF] Cut per-VM allocations in the append VM#21606
NullVoxPopuli wants to merge 2 commits into
emberjs:mainfrom
NullVoxPopuli:nvp/perf/append-vm-stacks

Conversation

@NullVoxPopuli

Copy link
Copy Markdown
Contributor

The Stacks class uses plain arrays instead of six StackImpl wrappers, and execute() runs the opcode loop directly instead of allocating a { done, value } result per instruction. A VM is constructed for every independently re-rendering block, so this shows up on {{#each}}-heavy pages.

🤖 Generated with Claude Code

Split out of nvp/simplify-some-vm-hot-paths.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
};
};
this.debug = (): DebugStacks => ({
scope: [...this.scope],

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

boooo array spread

@NullVoxPopuli
NullVoxPopuli marked this pull request as draft September 9, 2026 22:53
@NullVoxPopuli

This comment was marked as outdated.

@NullVoxPopuli

This comment was marked as outdated.

@NullVoxPopuli

This comment was marked as outdated.

@NullVoxPopuli

This comment was marked as outdated.

@NullVoxPopuli

This comment was marked as outdated.

@NullVoxPopuli

This comment was marked as outdated.

@NullVoxPopuli

Copy link
Copy Markdown
Contributor Author

Order-swap check. Same two builds (main bdb2580, PR 8327f61), same settings, run back to back. In the normal run main is tracerbench's control and the PR is the experiment. In the swapped run the URLs are exchanged, so the PR is the control and main is the experiment. The swapped column below has its sign flipped so that in both columns a positive number means the PR is slower. If the effect were positional bias, the two columns would disagree in sign. ✅ marks rows tracerbench calls significant in that run.

Phase Normal order: PR vs main Swapped order: PR vs main (sign-flipped) Same direction?
Total duration +259ms (+2.5%) +376ms (+3.5%) ✅ n/s
render -1ms (-0.5%) +3ms (+1.9%) n/s
render1000Items1 -24ms (-6.3%) ✅ +3ms (+0.9%) n/s
clearItems1 -1ms (-3.6%) +1ms (+1.6%) n/s
render1000Items2 -39ms (-12.3%) ✅ -22ms (-7.7%) ✅ yes
clearItems2 +41ms (+29.6%) ✅ +44ms (+24.1%) ✅ yes
render10000Items1 +176ms (+6.0%) ✅ +203ms (+6.4%) ✅ yes
clearManyItems1 -1ms (-0.2%) +1ms (+0.4%) n/s
render10000Items2 +32ms (+1.1%) -42ms (-1.4%) n/s
clearManyItems2 +14ms (+5.2%) ✅ +11ms (+4.0%) ✅ yes
render1000Items3 -14ms (-4.7%) -11ms (-4.1%) ✅ n/s
append1000Items1 -22ms (-5.6%) -48ms (-13.2%) ✅ n/s
append1000Items2 -9ms (-2.5%) +5ms (+1.4%) n/s
updateEvery10thItem1 -11ms (-3.7%) -7ms (-2.1%) n/s
updateEvery10thItem2 +320ms (+97.7%) ✅ +330ms (+48.3%) ✅ yes
selectFirstRow1 -3ms (-4.1%) -7ms (-8.6%) n/s
selectSecondRow1 -6ms (-8.4%) +0ms (+0.5%) n/s
removeFirstRow1 -23ms (-10.9%) ✅ -25ms (-12.3%) ✅ yes
removeSecondRow1 -24ms (-11.7%) ✅ -20ms (-9.9%) ✅ yes
swapRows1 -23ms (-15.2%) ✅ -15ms (-11.0%) ✅ yes
swapRows2 -17ms (-12.3%) ✅ -15ms (-10.5%) ✅ yes
clearItems4 -11ms (-9.7%) ✅ -9ms (-8.4%) ✅ yes
paint -3ms (-20.8%) +0ms (+3.9%) n/s

n/s = not significant in at least one of the two runs.

Result: every row that is significant in both runs points the same way regardless of which build tracerbench treated as control, so this is not positional bias. The PR is slower on clearItems2, render10000Items1, clearManyItems2 and updateEvery10thItem2, and faster on render1000Items2 and the remove, swap and clearItems4 phases, in both orders.

@NullVoxPopuli

Copy link
Copy Markdown
Contributor Author

One correction to my earlier reading of run 2: I said updateEvery10thItem2 "did not reproduce" there because tracerbench did not mark it significant. Looking at the sample summaries, the PR median was 578ms against 286ms for main in that run too, the same doubling as every other run. It failed significance only because the PR samples are bimodal (min about 270ms, max about 660ms), which widens the confidence interval until it crosses zero.

Across all five runs the PR build's samples for this phase have a minimum matching main (about 250 to 290ms) but a median of 580 to 680ms, while main's median stays at 290 to 330ms with only occasional slow samples. So both builds have a fast mode and a slow mode for this phase. On main the slow mode is rare; on this PR it is the common case. That is also why this PR shows visibly more variance than main in every run including the swapped one: the bimodal phase inflates the spread of the whole-run total. It behaves like a major GC that now usually lands inside the second update pass.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@NullVoxPopuli

Copy link
Copy Markdown
Contributor Author

Revised branch (023b930): debug snapshots use Array.from instead of spread, and the top() helper is replaced with Array#at(-1) on the stack accessors. Benchmarked against the same main (bdb2580), 20 samples each, 8x CPU throttle. The previous column is run 4 of the earlier version of this branch (8327f61) for comparison. Positive means the PR is slower than main.

Phase main median revised PR median revised: PR vs main previous version: PR vs main
Total duration 10,506ms 10,813ms 322ms (+3.1%) ✅ 259ms (+2.5%)
render 159ms 159ms -1ms (-0.7%) -1ms (-0.5%)
render1000Items1 374ms 369ms -5ms (-1.5%) -24ms (-6.3%) ✅
clearItems1 40ms 40ms 0ms (+0.9%) -1ms (-3.6%)
render1000Items2 317ms 285ms -31ms (-9.8%) ✅ -39ms (-12.3%) ✅
clearItems2 137ms 184ms 46ms (+33.5%) ✅ 41ms (+29.6%) ✅
render10000Items1 3,007ms 3,158ms 157ms (+5.2%) ✅ 176ms (+6.0%) ✅
clearManyItems1 391ms 389ms -1ms (-0.1%) -1ms (-0.2%)
render10000Items2 2,989ms 2,968ms -15ms (-0.5%) 32ms (+1.1%)
clearManyItems2 282ms 286ms 6ms (+2.0%) 14ms (+5.2%) ✅
render1000Items3 292ms 277ms -15ms (-5.1%) ✅ -14ms (-4.7%)
append1000Items1 402ms 364ms -30ms (-7.6%) ✅ -22ms (-5.6%)
append1000Items2 360ms 368ms 9ms (+2.4%) -9ms (-2.5%)
updateEvery10thItem1 324ms 312ms -7ms (-2.0%) -11ms (-3.7%)
updateEvery10thItem2 360ms 689ms 329ms (+91.5%) ✅ 320ms (+97.7%) ✅
selectFirstRow1 86ms 72ms -13ms (-14.5%) ✅ -3ms (-4.1%)
selectSecondRow1 70ms 71ms 0ms (+0.3%) -6ms (-8.4%)
removeFirstRow1 225ms 198ms -26ms (-11.7%) ✅ -23ms (-10.9%) ✅
removeSecondRow1 224ms 191ms -32ms (-14.3%) ✅ -24ms (-11.7%) ✅
swapRows1 153ms 139ms -11ms (-7.0%) -23ms (-15.2%) ✅
swapRows2 154ms 138ms -15ms (-10.0%) ✅ -17ms (-12.3%) ✅
clearItems4 112ms 104ms -7ms (-6.6%) ✅ -11ms (-9.7%) ✅
paint 8ms 8ms 1ms (+6.6%) -3ms (-20.8%)

Reading: swapping top() for Array#at(-1) changes nothing measurable. Every row that was significant before is significant in the same direction now, with the same magnitudes: render10000Items1 +5 to 6%, clearItems2 +30 to 34%, updateEvery10thItem2 roughly doubled, and the remove, swap and clearItems4 phases 7 to 14% faster. The Array.from change only affects the LOCAL_DEBUG snapshot and was never on the measured path. So the regression is not in how the top of the stack is read. That leaves the allocation-profile change itself (six StackImpl instances per VM becoming six bare arrays) and the execute() loop rewrite as the remaining candidates, and the bimodal updateEvery10thItem2 still points at GC timing.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant