From 1816d33bbdc3dbaf7f46346691889bbeb9fe9e6a Mon Sep 17 00:00:00 2001 From: David Eggertsen <3967142+deggertsen@users.noreply.github.com> Date: Thu, 10 Sep 2026 18:08:36 -0400 Subject: [PATCH 1/4] perf: avoid quadratic removed-key scan and skip scalar auto-activation Two micro-optimizations to the observable node update path: 1. updateNodes: the removed-key scan called keys.includes(key) for every previous key, which is O(n*m) when many keys change at once. Add a same-position fast path and only build a Set of the new keys when both key lists exceed 32 entries. On a 3000-key rotation this cuts a set() from ~12ms to ~0.9ms (p50, M4 Pro). Small objects take the identical includes() path as before. 2. recursivelyAutoActivateInner: skip non-object/non-function values before the isObservable check. Scalars can never be observables or linked functions, so this avoids boxing every string during auto activation of large plain objects. --- src/ObservableObject.ts | 9 ++++++++- tests/perf.test.ts | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/ObservableObject.ts b/src/ObservableObject.ts index b4421db8f..6646e73fb 100644 --- a/src/ObservableObject.ts +++ b/src/ObservableObject.ts @@ -230,9 +230,13 @@ function updateNodes(parent: NodeInfo, obj: Record | Array | unde } else if (prevValue && (!obj || isObject(obj))) { // For keys that have been removed from object, notify and update children recursively const lengthPrev = keysPrev.length; + let keySet: Set | undefined; for (let i = 0; i < lengthPrev; i++) { const key = keysPrev[i]; - if (!keys.includes(key)) { + // Fast path for same-position keys, then Set lookup for large key lists to avoid quadratic scans. + if (keys[i] === key) continue; + if (!keySet && keys.length > 32 && lengthPrev > 32) keySet = new Set(keys); + if (!(keySet ? keySet.has(key) : keys.includes(key))) { hasADiff = true; const child = getChildNode(parent, key); @@ -1479,6 +1483,9 @@ function recursivelyAutoActivateInner(obj: Record, pathStack: strin for (const key in obj) { if (hasOwnProperty.call(obj, key)) { const value = obj[key]; + // Scalars can never be observables or linked functions; skip them before the isObservable check. + const type = typeof value; + if (type !== 'object' && type !== 'function') continue; if (isObservable(value)) { const childNode = getNodeAtPath(); diff --git a/tests/perf.test.ts b/tests/perf.test.ts index 3b862155f..d1b080cce 100644 --- a/tests/perf.test.ts +++ b/tests/perf.test.ts @@ -181,4 +181,23 @@ describe('Perf', () => { expect(then2 - now2).toBeLessThan(process.env.CI === 'true' ? 5 : 1); }); + test('Object: replacing all keys of a large object notifies removed keys', () => { + const obs = observable({ records: {} as Record }); + const initial: Record = {}; + for (let i = 0; i < 2000; i++) initial['key' + i] = i; + obs.records.set(initial); + + // A listener on a removed key must still be notified when the removed-key + // scan uses the Set fast path for large key lists. + let removed = 0; + obs.records.key1999.onChange(() => removed++); + + const next: Record = {}; + for (let i = 0; i < 2000; i++) next['other' + i] = i; + + obs.records.set(next); + + expect(removed).toEqual(1); + expect(obs.records.peek()).toEqual(next); + }); }); From 893b6babd881e2f94ae955b1d4b9e962aa040ece Mon Sep 17 00:00:00 2001 From: David Eggertsen <3967142+deggertsen@users.noreply.github.com> Date: Thu, 10 Sep 2026 18:37:36 -0400 Subject: [PATCH 2/4] fix: preserve removed Map keys in update fast path --- src/ObservableObject.ts | 3 ++- tests/computed.test.ts | 36 +++++++++++++++++++++++++++++++ tests/mapset.test.ts | 47 ++++++++++++++++++++++++++++++++++++++++- tests/perf.test.ts | 19 ----------------- tests/tests.test.ts | 39 ++++++++++++++++++++++++++++++++++ 5 files changed, 123 insertions(+), 21 deletions(-) diff --git a/src/ObservableObject.ts b/src/ObservableObject.ts index 6646e73fb..873b13ac4 100644 --- a/src/ObservableObject.ts +++ b/src/ObservableObject.ts @@ -234,7 +234,8 @@ function updateNodes(parent: NodeInfo, obj: Record | Array | unde for (let i = 0; i < lengthPrev; i++) { const key = keysPrev[i]; // Fast path for same-position keys, then Set lookup for large key lists to avoid quadratic scans. - if (keys[i] === key) continue; + // Map keys can be undefined, so an out-of-bounds read must not count as a match. + if (i < length && keys[i] === key) continue; if (!keySet && keys.length > 32 && lengthPrev > 32) keySet = new Set(keys); if (!(keySet ? keySet.has(key) : keys.includes(key))) { hasADiff = true; diff --git a/tests/computed.test.ts b/tests/computed.test.ts index 36b631c2b..ca1dd4869 100644 --- a/tests/computed.test.ts +++ b/tests/computed.test.ts @@ -2441,6 +2441,42 @@ describe('Complex computeds', () => { }); }); describe('Activation', () => { + test('Recursive activation traverses mixed scalar values, objects, and arrays', () => { + const source = observable(1); + const lazyGet = jest.fn(() => 10); + const normalFunction = jest.fn(() => 20); + const scalars = { + text: 'hello', + number: 3, + boolean: true, + nil: null, + missing: undefined, + bigint: BigInt(1), + symbol: Symbol('value'), + }; + const obs = observable({ + ...scalars, + nested: { + ...scalars, + value: source, + auto: linked({ get: () => source.get() + 1, activate: 'auto' }), + lazy: linked({ get: lazyGet, activate: 'lazy' }), + normalFunction, + }, + array: [null, 'hello', { value: source }], + }); + + const value = obs.get(); + + expect(value).toMatchObject(scalars); + expect(value.nested).toMatchObject({ ...scalars, value: 1, auto: 2 }); + expect(value.array).toEqual([null, 'hello', { value: 1 }]); + expect(lazyGet).not.toHaveBeenCalled(); + expect(normalFunction).not.toHaveBeenCalled(); + source.set(2); + expect(obs.get().nested).toMatchObject({ value: 2, auto: 3 }); + expect(obs.get().array).toEqual([null, 'hello', { value: 2 }]); + }); test('linked with function', () => { const obs = observable({ test: 10, test2: 20 }); const comp = observable(linked(() => obs.test.get() + obs.test2.get())); diff --git a/tests/mapset.test.ts b/tests/mapset.test.ts index df50b4282..0cf6f79d8 100644 --- a/tests/mapset.test.ts +++ b/tests/mapset.test.ts @@ -1,7 +1,8 @@ import { observe } from '../src/observe'; -import { isObservable, optimized } from '../src/globals'; +import { getChildNode, getNode, isObservable, optimized } from '../src/globals'; import { mergeIntoObservable } from '../src/helpers'; import { observable } from '../src/observable'; +import { getProxy } from '../src/ObservableObject'; import { expectChangeHandler } from './testglobals'; describe('Map default behavior', () => { @@ -24,6 +25,50 @@ describe('Map default behavior', () => { expect(obs.test.get('key3').get()).toEqual('value3'); expect(obs.test.has('key1')).toEqual(false); }); + test.each([32, 33])('Reordering %i Map keys preserves key identity and retained listeners', (size) => { + const objectKey = { id: 1 }; + const replacementKey = { id: 1 }; + const initial = new Map([ + [objectKey, 1], + [NaN, 2], + [0, 3], + ...Array.from({ length: size - 3 }, (_, i): [unknown, number] => ['key' + i, i]), + ]); + const obs = observable(initial); + const onRemoved = expectChangeHandler(obs.get(objectKey)); + const onRetained = jest.fn(); + obs.get(NaN).onChange(onRetained); + obs.get(0).onChange(onRetained); + const next = new Map([...initial].reverse()); + next.delete(objectKey); + next.set(replacementKey, 4); + + obs.set(next); + + expect(onRemoved).toHaveBeenCalledTimes(1); + expect(onRemoved).toHaveBeenCalledWith(undefined, 1, [ + { path: [], pathTypes: [], prevAtPath: 1, valueAtPath: undefined }, + ]); + expect(onRetained).not.toHaveBeenCalled(); + expect(obs.peek()).toEqual(next); + obs.get(NaN).set(5); + obs.get(-0).set(6); + expect(onRetained).toHaveBeenCalledTimes(2); + }); + test.each([0, 1, 33])('Map replacement removes an undefined key after %i retained keys', (size) => { + const retained = Array.from({ length: size }, (_, i): [unknown, number] => ['key' + i, i]); + const obs = observable(new Map([...retained, [undefined, 42]])); + // Access the node directly because get(undefined) currently returns the Map's own proxy. + const child = getProxy(getChildNode(getNode(obs), undefined as any)); + const onRemoved = jest.fn(); + child.onChange(({ value }) => onRemoved(value)); + + obs.set(new Map(retained)); + + expect(onRemoved).toHaveBeenCalledTimes(1); + expect(onRemoved).toHaveBeenCalledWith(undefined); + expect(obs.peek()).toEqual(new Map(retained)); + }); test('Map delete', () => { const obs = observable({ test: new Map([ diff --git a/tests/perf.test.ts b/tests/perf.test.ts index d1b080cce..3b862155f 100644 --- a/tests/perf.test.ts +++ b/tests/perf.test.ts @@ -181,23 +181,4 @@ describe('Perf', () => { expect(then2 - now2).toBeLessThan(process.env.CI === 'true' ? 5 : 1); }); - test('Object: replacing all keys of a large object notifies removed keys', () => { - const obs = observable({ records: {} as Record }); - const initial: Record = {}; - for (let i = 0; i < 2000; i++) initial['key' + i] = i; - obs.records.set(initial); - - // A listener on a removed key must still be notified when the removed-key - // scan uses the Set fast path for large key lists. - let removed = 0; - obs.records.key1999.onChange(() => removed++); - - const next: Record = {}; - for (let i = 0; i < 2000; i++) next['other' + i] = i; - - obs.records.set(next); - - expect(removed).toEqual(1); - expect(obs.records.peek()).toEqual(next); - }); }); diff --git a/tests/tests.test.ts b/tests/tests.test.ts index 0eb3194f2..b0cd42a60 100644 --- a/tests/tests.test.ts +++ b/tests/tests.test.ts @@ -35,6 +35,45 @@ describe('Set', () => { expect(obs.test.get()).toEqual({ text: 't2' }); expect(obs.get()).toEqual({ test: { text: 't2' } }); }); + test.each([32, 33, 2000])('Replacing all %i object keys notifies removed descendants', (size) => { + const initial = Object.fromEntries(Array.from({ length: size }, (_, i) => ['key' + i, { value: i }])); + const obs = observable>(initial); + const removed = obs['key' + (size - 1)].value; + const onRemoved = expectChangeHandler(removed); + const next = Object.fromEntries(Array.from({ length: size }, (_, i) => ['other' + i, { value: i }])); + + obs.set(next); + + expect(onRemoved).toHaveBeenCalledTimes(1); + expect(onRemoved).toHaveBeenCalledWith(undefined, size - 1, [ + { path: [], pathTypes: [], prevAtPath: size - 1, valueAtPath: undefined }, + ]); + expect(obs.peek()).toEqual(next); + }); + test.each([32, 33])('Reordering %i object keys preserves retained listeners and deletion order', (size) => { + const initial = Object.fromEntries(Array.from({ length: size }, (_, i) => ['key' + i, i])); + const obs = observable>(initial); + const retained = obs.key1; + const onRetained = jest.fn(); + retained.onChange(onRetained); + const removed: string[] = []; + obs.key0.onChange(() => removed.push('key0')); + obs['key' + (size - 1)].onChange(() => removed.push('key' + (size - 1))); + const next = Object.fromEntries([ + ['added0', -1], + ...Object.entries(initial).slice(1, -1).reverse(), + ['added1', -2], + ]); + + obs.set(next); + + expect(removed).toEqual(['key0', 'key' + (size - 1)]); + expect(onRetained).not.toHaveBeenCalled(); + expect(obs.key1).toBe(retained); + expect(obs.peek()).toEqual(next); + obs.key1.set(100); + expect(onRetained).toHaveBeenCalledTimes(1); + }); test('Set primitive', () => { const obs = observable({ test: { text: 't' } }); obs.test.text.set('t2'); From 4a5179a79fed9e91331e40ccb868c35a9a50d8d4 Mon Sep 17 00:00:00 2001 From: David Eggertsen <3967142+deggertsen@users.noreply.github.com> Date: Thu, 17 Sep 2026 15:38:25 -0400 Subject: [PATCH 3/4] test: check synced lazy activation without timing assertions --- tests/perf.test.ts | 29 ----------------------------- tests/synced.test.ts | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 29 deletions(-) diff --git a/tests/perf.test.ts b/tests/perf.test.ts index 3b862155f..b52123ee8 100644 --- a/tests/perf.test.ts +++ b/tests/perf.test.ts @@ -1,4 +1,3 @@ -import { synced } from '../src/sync/synced'; import { ObservableHint } from '../src/ObservableHint'; import { linked } from '../src/linked'; import { observable } from '../src/observable'; @@ -81,34 +80,6 @@ describe('Perf', () => { expect(then - now).toBeLessThan(1); }); - test('Lazy activation perf with synced', () => { - const obj: Record = {}; - const Num = 10000; - for (let i = 0; i < Num; i++) { - obj['key' + i] = { - child: { - grandchild: { - value: 'hi', - }, - }, - }; - } - - const obs = observable( - synced>({ - get: () => null, - subscribe: ({ update }) => { - update({ value: obj }); - }, - }), - ); - - const now = performance.now(); - obs.get(); - const then = performance.now(); - - expect(then - now).toBeLessThan(5); - }); test('Lazy activation perf2', () => { const obj: Record = {}; const Num = 10000; diff --git a/tests/synced.test.ts b/tests/synced.test.ts index 85b6e5e28..5db40fa8f 100644 --- a/tests/synced.test.ts +++ b/tests/synced.test.ts @@ -2,6 +2,31 @@ import { observable, observe } from '@legendapp/state'; import { synced } from '@legendapp/state/sync'; import { promiseTimeout } from './testglobals'; +describe('Lazy activation', () => { + test('Reading a synced root does not traverse the subscribed data', () => { + const enumerateChild = jest.fn(Reflect.ownKeys); + const data: Record = {}; + for (let i = 0; i < 10000; i++) { + data['key' + i] = new Proxy<(typeof data)[string]>( + { child: { grandchild: { value: 'hi' } } }, + { ownKeys: enumerateChild }, + ); + } + const get = jest.fn(() => null); + const subscribe = jest.fn(({ update }) => update({ value: data })); + const obs$ = observable(synced({ get, subscribe })); + + expect(get).not.toHaveBeenCalled(); + expect(subscribe).not.toHaveBeenCalled(); + expect(obs$.get()).toBe(data); + expect(get).toHaveBeenCalledTimes(1); + expect(subscribe).toHaveBeenCalledTimes(1); + // Catch eager recursive activation without relying on a wall-clock deadline. + expect(enumerateChild).not.toHaveBeenCalled(); + expect(obs$.key9999.child.grandchild.value.get()).toBe('hi'); + }); +}); + describe('unsubscribe', () => { test('Canceling observe unsubscribes', async () => { let numObserves = 0; From 7c68964fa78502946ac40664a6e01afefd203337 Mon Sep 17 00:00:00 2001 From: David Eggertsen <3967142+deggertsen@users.noreply.github.com> Date: Thu, 17 Sep 2026 15:46:53 -0400 Subject: [PATCH 4/4] test: await sync metadata persistence --- tests/crud.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/crud.test.ts b/tests/crud.test.ts index 0eefa8351..697352f58 100644 --- a/tests/crud.test.ts +++ b/tests/crud.test.ts @@ -1999,7 +1999,8 @@ describe('lastSync', () => { syncState(obs$).sync(); expect(numLists).toEqual(2); - await promiseTimeout(10); + // lastSync is published after the persistence plugin finishes writing metadata. + await when(() => syncState(obs$).lastSync.get() === 2); expect(localStorage.getItem(persistName + '__m')!).toEqual(JSON.stringify({ lastSync: 2 })); });