Skip to content

Simplify ArrayIterator to a cached length and a position counter - #21598

Open
NullVoxPopuli wants to merge 1 commit into
emberjs:mainfrom
NullVoxPopuli:nvp/perf/array-iterator
Open

Simplify ArrayIterator to a cached length and a position counter#21598
NullVoxPopuli wants to merge 1 commit into
emberjs:mainfrom
NullVoxPopuli:nvp/perf/array-iterator

Conversation

@NullVoxPopuli

@NullVoxPopuli NullVoxPopuli commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Drops the current state object that ArrayIterator allocated per list and tracks progress with a position counter and a cached length instead.

The length is still read in the constructor so a tracked array's tag is consumed inside the iterator reference's tracking frame.

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

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@NullVoxPopuli NullVoxPopuli changed the title [PERF] Simplify ArrayIterator to a cached length and a position counter Simplify ArrayIterator to a cached length and a position counter Sep 9, 2026
@NullVoxPopuli

Copy link
Copy Markdown
Contributor Author

Benchmark: main (bdb2580) vs nvp/perf/array-iterator (a5db40a)

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,831ms 9,669ms -128ms (-1.3%) -239ms .. -2ms
render 151ms 147ms -4ms (-2.5%) -7ms .. -1ms ✅
render1000Items1 356ms 352ms -4ms (-1.1%) -7ms .. 0ms ✅
clearItems1 39ms 38ms -1ms (-1.5%) -1ms .. 0ms
render1000Items2 302ms 297ms -4ms (-1.2%) -6ms .. 1ms
clearItems2 137ms 136ms -1ms (-0.9%) -3ms .. 0ms ✅
render10000Items1 2,819ms 2,751ms -48ms (-1.7%) -92ms .. -22ms ✅
clearManyItems1 378ms 372ms -5ms (-1.3%) -8ms .. -2ms ✅
render10000Items2 2,803ms 2,775ms -22ms (-0.8%) -39ms .. 24ms
clearManyItems2 269ms 266ms -4ms (-1.4%) -7ms .. -1ms ✅
render1000Items3 267ms 265ms -2ms (-0.7%) -7ms .. 4ms
append1000Items1 352ms 346ms -4ms (-1.1%) -20ms .. 4ms
append1000Items2 332ms 331ms 1ms (+0.2%) -6ms .. 9ms
updateEvery10thItem1 296ms 301ms 5ms (+1.6%) -1ms .. 10ms
updateEvery10thItem2 315ms 313ms -1ms (-0.3%) -10ms .. 8ms
selectFirstRow1 93ms 92ms -1ms (-1.5%) -6ms .. 3ms
selectSecondRow1 63ms 63ms -1ms (-0.8%) -3ms .. 1ms
removeFirstRow1 200ms 198ms -1ms (-0.5%) -7ms .. 6ms
removeSecondRow1 193ms 192ms 1ms (+0.3%) -5ms .. 6ms
swapRows1 132ms 130ms -1ms (-1.1%) -5ms .. 1ms
swapRows2 130ms 128ms 0ms (-0.2%) -4ms .. 6ms
clearItems4 107ms 107ms 0ms (-0.0%) -2ms .. 2ms
paint 11ms 10ms 0ms (+0.5%) -3ms .. 2ms

let memo = ++this.pos;

let { keyFor } = this;
if (memo >= this.length) return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

will this also need to account for live array mutations during render?

-if (memo >= this.length) return null;
+if (memo >= this.iterator.length) return null;

@johanrd

johanrd commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

very cool work!

johanrd added a commit to johanrd/ember.js that referenced this pull request Sep 10, 2026
Logs how many blocks render when the backing array is truncated by user code
that runs during iteration. On emberjs#21598.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8ja8hgWMpTns6ezSXEMzr
johanrd added a commit to johanrd/ember.js that referenced this pull request Sep 10, 2026
Logs how many blocks render when the backing array is truncated by user code
that runs during iteration. On the emberjs#21598 base.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8ja8hgWMpTns6ezSXEMzr
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.

2 participants