From 4427f536f3ef9204c404f265865a8aa3d682b45e Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Sat, 3 Oct 2026 04:16:13 -0400 Subject: [PATCH 1/2] Keep a tracked field's value and tag in one cell A read or a write of a tracked field went through three maps: the tag registry (a WeakMap, then a Map per object) and a WeakMap of values. Each field now has one WeakMap from the instance to a cell with the value and the tag, so a read is one lookup and consumeTag, and a write is one lookup and DIRTY_TAG. The cell takes its tag from the tag registry when it is made. Other code can ask the registry for the tag before the first read or write of the field (an observer, a computed chain, notifyPropertyChange), and that tag must stay the tag of the field. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../@glimmer/validator/lib/tracked-data.ts | 67 +++++++++++++++---- .../@glimmer/validator/test/tracking-test.ts | 46 +++++++++++++ 2 files changed, 100 insertions(+), 13 deletions(-) diff --git a/packages/@glimmer/validator/lib/tracked-data.ts b/packages/@glimmer/validator/lib/tracked-data.ts index 560f6d71a54..4c16abb1361 100644 --- a/packages/@glimmer/validator/lib/tracked-data.ts +++ b/packages/@glimmer/validator/lib/tracked-data.ts @@ -1,36 +1,77 @@ -import { dirtyTagFor, tagFor } from './meta'; +import { DEBUG } from '@glimmer/env'; +import type { UpdatableTag } from '@glimmer/interfaces'; + +import { debug } from './debug'; +import { tagFor } from './meta'; import { consumeTag } from './tracking'; +import { unwrap } from './utils'; +import { DIRTY_TAG } from './validators'; export type Getter = (self: T) => T[K] | undefined; export type Setter = (self: T, value: T[K]) => void; +/** + * The value and the tag of one field of one instance. A read or a write + * finds both with one WeakMap lookup. + * + * The tag is the one that the tag registry has for the field, so `tagFor` + * and `dirtyTagFor` work on the same tag as the field. + */ +interface TrackedCell { + value: V; + tag: UpdatableTag; + initialized: boolean; +} + export function trackedData( key: K, initializer?: (this: T) => T[K] ): { getter: Getter; setter: Setter } { - let values = new WeakMap(); + let cells = new WeakMap>(); let hasInitializer = typeof initializer === 'function'; + function cellFor(self: T): TrackedCell { + let cell = cells.get(self); + + if (cell === undefined) { + cell = { + value: undefined, + // Other code can ask the registry for this tag before the first read + // or write of the field, so the cell takes the tag from there. + tag: tagFor(self, key) as UpdatableTag, + initialized: !hasInitializer, + }; + cells.set(self, cell); + } + + return cell; + } + function getter(self: T) { - consumeTag(tagFor(self, key)); + const cell = cellFor(self); - let value; + consumeTag(cell.tag); // If the field has never been initialized, we should initialize it - if (hasInitializer && !values.has(self)) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- @fixme - value = initializer!.call(self); - values.set(self, value); - } else { - value = values.get(self); + if (!cell.initialized) { + cell.initialized = true; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- guarded by initialized + cell.value = initializer!.call(self); } - return value; + return cell.value; } function setter(self: T, value: T[K]): void { - dirtyTagFor(self, key); - values.set(self, value); + const cell = cellFor(self); + + if (DEBUG) { + unwrap(debug.assertTagNotConsumed)(cell.tag, self, key); + } + + DIRTY_TAG(cell.tag, true); + cell.initialized = true; + cell.value = value; } return { getter, setter }; diff --git a/packages/@glimmer/validator/test/tracking-test.ts b/packages/@glimmer/validator/test/tracking-test.ts index 1a0fc4c1dc0..ea01e91e361 100644 --- a/packages/@glimmer/validator/test/tracking-test.ts +++ b/packages/@glimmer/validator/test/tracking-test.ts @@ -6,10 +6,12 @@ import { createTag, debug, dirtyTag, + dirtyTagFor, endTrackFrame, getValue, isConst, isTracking, + tagFor, track, trackedData, untrack, @@ -485,6 +487,50 @@ module('@glimmer/validator: tracking', () => { assert.notOk(validateTag(tag, snapshot)); }); + test('a write dirties the tag that tagFor gave before the first read', (assert) => { + class Foo { + foo = 123; + } + + let { setter } = trackedData('foo'); + + let foo = new Foo(); + let tag = tagFor(foo, 'foo'); + let snapshot = valueForTag(tag); + + setter(foo, 456); + assert.notOk(validateTag(tag, snapshot)); + }); + + test('a read consumes the tag that tagFor gave before it', (assert) => { + class Foo { + foo = 123; + } + + let { getter } = trackedData('foo'); + + let foo = new Foo(); + let registered = tagFor(foo, 'foo'); + let tag = track(() => getter(foo)); + + assert.strictEqual(tag, registered); + }); + + test('dirtyTagFor dirties the tag that a read consumed', (assert) => { + class Foo { + foo = 123; + } + + let { getter } = trackedData('foo'); + + let foo = new Foo(); + let tag = track(() => getter(foo)); + let snapshot = valueForTag(tag); + + dirtyTagFor(foo, 'foo'); + assert.notOk(validateTag(tag, snapshot)); + }); + if (DEBUG) { test('it errors when attempting to update a value already consumed in the same transaction', (assert) => { class Foo { From fcfd26b0eac9a68dc254ec29229d1b1fd634db0c Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:30:33 -0400 Subject: [PATCH 2/2] Make the value field of a tracked cell general from the first cell All cells have one hidden class, and V8 records which kind of value a field held so far. If the tracked fields of an app hold no number at first, the first number makes V8 throw away the optimized code of each function that reads a cell. The benchmark app has one tracked field, `selected`, which is `undefined` until the first select of a row. `--trace-deopt` showed 3 deopts with the reason "dependent field representation changed" in that phase, and the phase was 7.3% slower than main. Each cell now starts with a number and gets `undefined` right after, so the field is general before that code is optimized. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../@glimmer/validator/lib/tracked-data.ts | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/packages/@glimmer/validator/lib/tracked-data.ts b/packages/@glimmer/validator/lib/tracked-data.ts index 4c16abb1361..afa2805e338 100644 --- a/packages/@glimmer/validator/lib/tracked-data.ts +++ b/packages/@glimmer/validator/lib/tracked-data.ts @@ -23,6 +23,22 @@ interface TrackedCell { initialized: boolean; } +/** + * All cells have one hidden class, and V8 records which kind of value the + * `value` field held so far. The first value of another kind makes V8 throw + * away the optimized code that reads the field. + * + * A number and then `undefined` make the field general from the first cell + * on, before that code is optimized. + */ +function createCell(tag: UpdatableTag, initialized: boolean): TrackedCell { + let cell: TrackedCell = { value: 0, tag, initialized }; + + cell.value = undefined; + + return cell as TrackedCell; +} + export function trackedData( key: K, initializer?: (this: T) => T[K] @@ -34,13 +50,9 @@ export function trackedData( let cell = cells.get(self); if (cell === undefined) { - cell = { - value: undefined, - // Other code can ask the registry for this tag before the first read - // or write of the field, so the cell takes the tag from there. - tag: tagFor(self, key) as UpdatableTag, - initialized: !hasInitializer, - }; + // Other code can ask the registry for this tag before the first read + // or write of the field, so the cell takes the tag from there. + cell = createCell(tagFor(self, key) as UpdatableTag, !hasInitializer); cells.set(self, cell); }