Fix ArrayIterator tracking and keep the live length bound - #17
Merged
NullVoxPopuli merged 3 commits intoSep 22, 2026
Conversation
The iterator reference computes its value in a tracking frame.
The constructor is the only ArrayIterator code that runs in that frame.
`isEmpty()` and `next()` run later, in the frame of the VM.
Without a read in the constructor, the reference never consumes the tag of a tracked array.
The reference then returns the same iterator after a change, and the list block skips its sync.
This broke `{{each}} works when updating old items` for `trackedArray`.
It also stopped the list updates that the benchmark app waits for.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The assertion looped over the rendered elements only.
A list that did not render a new item passed the test.
`{{each}} works with new items` passed for `trackedArray` while the list did not update after `push`.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
|
Code in the block body or in a `key` getter runs between two `next()` calls. That code can change the length of a plain array, and a plain array has no tag. On main, `next()` compares against the live length: - a shorter array stops the iteration - a longer array renders the new items A stored length rendered blocks for `undefined` after a truncation, and skipped added items. The constructor still reads `length`, because only that read runs in the tracking frame of the iterator reference. It now stores the empty state only, as main does. Reported by johanrd in emberjs#21598. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
NullVoxPopuli-ai-agent
changed the title
NullVoxPopuli
merged commit Sep 22, 2026
1fcf3bd
into
NullVoxPopuli:nvp/perf/array-iterator
48 of 49 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the two CI failures on emberjs#21598, and keeps the behavior that johanrd reported as lost.
ArrayIteratorneeds two reads oflength, for two different reasons.1. A read in the constructor, for tracking
createIteratorRefbuilds the iterator in a tracking frame. The constructor is the onlyArrayIteratorcode that runs in that frame.isEmpty()andnext()run later, in the frame of the VM.980f7659c2removed the constructor read. The iterator reference then never consumed the tag of a tracked array. After a change, the reference returned the same iterator, andListBlockOpcodeskipped its sync. That caused the failures inBasic TestandPerf script still works.The constructor now reads
lengthand stores the empty state only, asmaindoes.2. A live read in
next(), for plain arraysCode in the block body or in a
keygetter runs between twonext()calls. That code can change the length of a plain array, and a plain array has no tag.{{#each}}over a plain arraymaina5db40a9a3)[item-0][item-0][][][][item-0][item-0][item-1][item-0][item-0][item-1]Two new tests in the
#eachsuite cover these rows.Test helper
assertEachCompareResultsnow compares the rendered item count. Before, it looped over the rendered elements only, so{{each}} works with new itemspassed whilepushdid not update the list.Verification
tsc, ESLint, and Prettier pass.a5db40a9a3, which had the constructor read.🤖 Generated with Claude Code