From a5db40a9a3d57bc945145003f3723f3c9b138f72 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Wed, 9 Sep 2026 18:35:06 -0400 Subject: [PATCH 1/3] [PERF] Simplify ArrayIterator to a cached length and a position counter Split out of nvp/simplify-some-vm-hot-paths. Co-Authored-By: Claude Fable 5.1 --- packages/@glimmer/reference/lib/iterable.ts | 35 +++++++-------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/packages/@glimmer/reference/lib/iterable.ts b/packages/@glimmer/reference/lib/iterable.ts index 71134eb5c2b..a7d613d6351 100644 --- a/packages/@glimmer/reference/lib/iterable.ts +++ b/packages/@glimmer/reference/lib/iterable.ts @@ -225,42 +225,31 @@ class IteratorWrapper implements OpaqueIterator { } class ArrayIterator implements OpaqueIterator { - private current: { kind: 'empty' } | { kind: 'first'; value: unknown } | { kind: 'progress' }; - private pos = 0; + private pos = -1; + + // The constructor runs inside the iterator reference's tracking frame, so + // reading `length` here is what attributes a tracked collection's tag to that + // reference. Read it from `next()` instead and the list stops revalidating. + private length: number; constructor( private iterator: unknown[] | readonly unknown[], private keyFor: KeyFor ) { - if (iterator.length === 0) { - this.current = { kind: 'empty' }; - } else { - this.current = { kind: 'first', value: iterator[this.pos] }; - } + this.length = iterator.length; } isEmpty(): boolean { - return this.current.kind === 'empty'; + return this.length === 0; } next(): Nullable> { - let value: unknown; - - let current = this.current; - if (current.kind === 'first') { - this.current = { kind: 'progress' }; - value = current.value; - } else if (this.pos >= this.iterator.length - 1) { - return null; - } else { - value = this.iterator[++this.pos]; - } + let memo = ++this.pos; - let { keyFor } = this; + if (memo >= this.length) return null; - let key = keyFor(value, this.pos); - let memo = this.pos; + let value = this.iterator[memo]; - return { key, value, memo }; + return { key: this.keyFor(value, memo), value, memo }; } } From 980f7659c26b0795889d17fbcb72dea9688f512b Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Mon, 21 Sep 2026 18:46:07 -0400 Subject: [PATCH 2/3] Update iterable.ts --- packages/@glimmer/reference/lib/iterable.ts | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/@glimmer/reference/lib/iterable.ts b/packages/@glimmer/reference/lib/iterable.ts index a7d613d6351..96b056682c3 100644 --- a/packages/@glimmer/reference/lib/iterable.ts +++ b/packages/@glimmer/reference/lib/iterable.ts @@ -227,26 +227,19 @@ class IteratorWrapper implements OpaqueIterator { class ArrayIterator implements OpaqueIterator { private pos = -1; - // The constructor runs inside the iterator reference's tracking frame, so - // reading `length` here is what attributes a tracked collection's tag to that - // reference. Read it from `next()` instead and the list stops revalidating. - private length: number; - constructor( private iterator: unknown[] | readonly unknown[], private keyFor: KeyFor - ) { - this.length = iterator.length; - } + ) {} isEmpty(): boolean { - return this.length === 0; + return this.iterator.length === 0; } next(): Nullable> { let memo = ++this.pos; - if (memo >= this.length) return null; + if (memo >= this.iterator.length) return null; let value = this.iterator[memo]; From 1fcf3bd830871f6692a8d5af5e9729e4292ef879 Mon Sep 17 00:00:00 2001 From: "@NullVoxPopuli's reduced-access machine account for AI usage" <268630448+NullVoxPopuli-ai-agent@users.noreply.github.com> Date: Mon, 21 Sep 2026 20:50:24 -0400 Subject: [PATCH 3/3] Fix ArrayIterator tracking and keep the live length bound (#17) * Read the array length in the ArrayIterator constructor 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 * Compare the rendered item count in the {{each}} reactivity tests 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 * Read the live array length in ArrayIterator#next 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/ember.js#21598. Co-Authored-By: Claude Fable 5.1 --------- Co-authored-by: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 --- .../integration-tests/lib/render-test.ts | 16 ++++---- .../integration-tests/lib/suites/each.ts | 40 +++++++++++++++++++ packages/@glimmer/reference/lib/iterable.ts | 9 ++++- 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/packages/@glimmer-workspace/integration-tests/lib/render-test.ts b/packages/@glimmer-workspace/integration-tests/lib/render-test.ts index 3bb2198f2a0..fcb040348a2 100644 --- a/packages/@glimmer-workspace/integration-tests/lib/render-test.ts +++ b/packages/@glimmer-workspace/integration-tests/lib/render-test.ts @@ -448,14 +448,16 @@ export class RenderTest implements IRenderTest { private assertEachCompareResults( items: (number | string | [string | number, string | number])[] ) { - [...(this.element as unknown as HTMLElement).querySelectorAll('.test-item')].forEach( - (el, index) => { - let key = Array.isArray(items[index]) ? items[index][0] : index; - let value = Array.isArray(items[index]) ? items[index][1] : items[index]; + let rendered = (this.element as unknown as HTMLElement).querySelectorAll('.test-item'); - QUnit.assert.equal(el.textContent, `${key}.${value}`, `Comparing the rendered key.value`); - } - ); + QUnit.assert.strictEqual(rendered.length, items.length, `Comparing the rendered item count`); + + rendered.forEach((el, index) => { + let key = Array.isArray(items[index]) ? items[index][0] : index; + let value = Array.isArray(items[index]) ? items[index][1] : items[index]; + + QUnit.assert.equal(el.textContent, `${key}.${value}`, `Comparing the rendered key.value`); + }); } protected assertReactivity( diff --git a/packages/@glimmer-workspace/integration-tests/lib/suites/each.ts b/packages/@glimmer-workspace/integration-tests/lib/suites/each.ts index a8bc509b94c..576e8f81d94 100644 --- a/packages/@glimmer-workspace/integration-tests/lib/suites/each.ts +++ b/packages/@glimmer-workspace/integration-tests/lib/suites/each.ts @@ -46,6 +46,46 @@ export class EachSuite extends RenderTest { this.assertStableNodes(); } + @test + 'a plain array that shrinks during iteration stops the iteration'() { + let list: Item[] = []; + + // The getter is on the prototype so that `JSON.stringify` in `render` does not call it. + class Item { + constructor(private index: number) {} + + get name() { + if (this.index === 0) list.length = 1; + return `item-${this.index}`; + } + } + + for (let i = 0; i < 4; i++) list.push(new Item(i)); + + this.render('{{#each this.list key="@index" as |item|}}[{{item.name}}]{{/each}}', { list }); + this.assertHTML('[item-0]'); + } + + @test + 'a plain array that grows during iteration renders the new items'() { + let list: Item[] = []; + + // The getter is on the prototype so that `JSON.stringify` in `render` does not call it. + class Item { + constructor(private index: number) {} + + get name() { + if (this.index === 0) list.push(new Item(1)); + return `item-${this.index}`; + } + } + + list.push(new Item(0)); + + this.render('{{#each this.list key="@index" as |item|}}[{{item.name}}]{{/each}}', { list }); + this.assertHTML('[item-0][item-1]'); + } + @test 'autotracked custom iterable'() { if (typeof Symbol !== 'function') { diff --git a/packages/@glimmer/reference/lib/iterable.ts b/packages/@glimmer/reference/lib/iterable.ts index 96b056682c3..20366c736e1 100644 --- a/packages/@glimmer/reference/lib/iterable.ts +++ b/packages/@glimmer/reference/lib/iterable.ts @@ -226,19 +226,24 @@ class IteratorWrapper implements OpaqueIterator { class ArrayIterator implements OpaqueIterator { private pos = -1; + private empty: boolean; constructor( private iterator: unknown[] | readonly unknown[], private keyFor: KeyFor - ) {} + ) { + // Only this read runs in the tracking frame of the iterator reference. + this.empty = iterator.length === 0; + } isEmpty(): boolean { - return this.iterator.length === 0; + return this.empty; } next(): Nullable> { let memo = ++this.pos; + // The length is live because code in the block can change a plain array. if (memo >= this.iterator.length) return null; let value = this.iterator[memo];