Skip to content

LowLevelRegisters tuple order and drop unused register helpers - #21605

Open
NullVoxPopuli wants to merge 1 commit into
emberjs:mainfrom
NullVoxPopuli:nvp/cleanup/low-level-registers
Open

LowLevelRegisters tuple order and drop unused register helpers#21605
NullVoxPopuli wants to merge 1 commit into
emberjs:mainfrom
NullVoxPopuli:nvp/cleanup/low-level-registers

Conversation

@NullVoxPopuli

@NullVoxPopuli NullVoxPopuli commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

The tuple type listed $sp before $fp, but the register constants are $fp=2, $sp=3, so the initializeRegisters* helpers wrote to the wrong slots and their only caller immediately overwrote them.

EvaluationStackImpl.restore now builds the registers directly, and the duplicate snapshot static block is removed since the constructor already installs it under LOCAL_DEBUG.

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

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@NullVoxPopuli NullVoxPopuli changed the title Fix LowLevelRegisters tuple order and drop unused register helpers LowLevelRegisters tuple order and drop unused register helpers Sep 9, 2026
@NullVoxPopuli

Copy link
Copy Markdown
Contributor Author

Benchmark: main (bdb2580) vs nvp/cleanup/low-level-registers (2718522)

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 9,053ms 9,092ms -18ms (-0.2%) -205ms .. 180ms
render 137ms 139ms 1ms (+0.7%) -5ms .. 5ms
render1000Items1 329ms 337ms 2ms (+0.6%) -6ms .. 13ms
clearItems1 36ms 37ms 0ms (+0.2%) -2ms .. 2ms
render1000Items2 277ms 277ms 2ms (+0.9%) -8ms .. 15ms
clearItems2 135ms 133ms -2ms (-1.2%) -4ms .. 1ms
render10000Items1 2,609ms 2,654ms -4ms (-0.1%) -84ms .. 63ms
clearManyItems1 361ms 358ms -1ms (-0.2%) -6ms .. 3ms
render10000Items2 2,563ms 2,597ms 7ms (+0.3%) -43ms .. 114ms
clearManyItems2 259ms 252ms -2ms (-0.7%) -10ms .. 5ms
render1000Items3 234ms 236ms 1ms (+0.6%) -14ms .. 12ms
append1000Items1 341ms 363ms 8ms (+2.2%) -25ms .. 37ms
append1000Items2 308ms 309ms 0ms (-0.1%) -12ms .. 14ms
updateEvery10thItem1 271ms 278ms 6ms (+2.4%) -4ms .. 18ms
updateEvery10thItem2 300ms 288ms -1ms (-0.5%) -21ms .. 12ms
selectFirstRow1 74ms 71ms -2ms (-2.4%) -8ms .. 4ms
selectSecondRow1 56ms 56ms -1ms (-2.0%) -6ms .. 4ms
removeFirstRow1 181ms 182ms 0ms (+0.2%) -6ms .. 10ms
removeSecondRow1 172ms 174ms 0ms (+0.0%) -8ms .. 9ms
swapRows1 120ms 116ms -2ms (-1.9%) -11ms .. 4ms
swapRows2 115ms 115ms -1ms (-0.8%) -7ms .. 5ms
clearItems4 102ms 101ms 0ms (-0.2%) -2ms .. 3ms
paint 11ms 14ms 2ms (+18.5%) -2ms .. 6ms

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