Skip to content

perf: avoid quadratic removed-key scan and skip scalar auto-activation - #664

Open
deggertsen wants to merge 4 commits into
LegendApp:mainfrom
deggertsen:perf/updateNodes-set-and-activation-skip
Open

deggertsen wants to merge 4 commits into
LegendApp:mainfrom
deggertsen:perf/updateNodes-set-and-activation-skip

Conversation

@deggertsen

@deggertsen deggertsen commented Sep 10, 2026 •

Copy link
Copy Markdown

Summary

Reduce work when replacing observable objects and activating their contents while preserving child notifications and lazy activation.

  • Skip the removed-key membership scan for keys still at the same position. On the first mismatch, lazily build a Set when both key lists exceed 32 entries, avoiding quadratic scans for large objects. Smaller lists retain includes().
  • Bound the same-position lookup by the new key count so an out-of-bounds read cannot hide removal of an undefined Map key.
  • Skip primitive values before checking for observables or linked functions during recursive activation.

Set and includes() membership both use SameValueZero semantics, preserving object-key identity, NaN, and signed-zero equivalence. Deletion handling retains its original order.

Regression coverage

  • Object replacement and reordering at 32 and 33 keys, plus replacement of 2,000 keys: removed descendants, notification payloads and order, and retained listeners.

  • Map object-key identity, NaN, signed zero, and removal of an undefined key after 0, 1, or 33 retained keys.

  • Mixed-scalar activation with nested observables, arrays, automatic links, lazy links, and normal functions.

  • Replaced the single-sample 5 ms synced-activation assertion with a deterministic test in tests/synced.test.ts. It verifies lazy get/subscribe calls, delivery of 10,000 records without child enumeration, and subsequent access to a nested value. Disabling the plain-data hint in a temporary checkout makes the test fail with 20,000 child enumerations, confirming that it catches eager traversal. The original timing assertion failed on CI at 5.64 ms.

  • Stabilized the CRUD last-sync persistence test by awaiting the observable lastSync signal, which is published after metadata is written, instead of assuming persistence finishes within 10 ms. All existing assertions remain.

Validation

  • GitHub CI on 7c68964f: Test, Lint, and Format passed (run).
  • Full local Jest suite: 26 suites, 1,092 tests passed (npm test -- --runInBand --watchman=false).
  • Focused Jest suite: 152 tests passed across CRUD, synced, and performance tests. Bun also passed the synced suite and the metadata regression case.
  • npm run typecheck, npm run build, npm run lint:check, npm run format:check, and git diff --check passed.
  • The lazy-traversal test was verified against a deliberate regression in a temporary checkout, then that checkout was restored.

Local performance check

Measured September 10 on Apple M4 Pro, Node 26.3.1, using production-mode bundles of base commit 23b5ddeb and the runtime changes in this PR. Object replacement alternates two objects after 50 warmup updates, with 50 measured samples. Per-update milliseconds for 3,000 keys:

Scenario Base p50 / p90 Updated p50 / p90
Same key order, changed values 3.19 / 3.55 0.50 / 0.58
Reordered keys, changed values 3.21 / 3.67 0.62 / 0.80
All keys replaced 7.68 / 8.24 0.79 / 1.14

First root read of 3,000 string properties measured 0.165 ms → 0.090 ms median (50 warmups, 100 samples). Small-object cases at 16 and 33 keys remained in the low single-digit microseconds, with mixed small differences. These local measurements are not cross-engine guarantees or CI timing assertions.

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.
@deggertsen
deggertsen force-pushed the perf/updateNodes-set-and-activation-skip branch from d5380a0 to 96e05af Compare September 17, 2026 19:38
@deggertsen
deggertsen force-pushed the perf/updateNodes-set-and-activation-skip branch from 96e05af to 4a5179a Compare September 17, 2026 19:42
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.

1 participant