diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c34bb51b..294adae12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ All notable changes to this project will be documented in this file. The format - Add `logTree` helper to inspect the component tree from the console ([#654](https://github.com/studiometa/js-toolkit/pull/654)) - Add performance benchmarks for tween, animate, transform, services and Base internals ([#722](https://github.com/studiometa/js-toolkit/pull/722)) - Add CodSpeed CI integration for continuous performance tracking ([#723](https://github.com/studiometa/js-toolkit/pull/723)) +- Add performance benchmarks for `__parent` resolution and component queries ([4112247e](https://github.com/studiometa/js-toolkit/commit/4112247e), [1ecf9d37](https://github.com/studiometa/js-toolkit/commit/1ecf9d37)) ### Changed @@ -39,6 +40,9 @@ All notable changes to this project will be documented in this file. The format - Improve `ScrollService.updateProps` by caching scroll max values and refreshing them on resize and scrolling-element size changes ([#721](https://github.com/studiometa/js-toolkit/pull/721), [001fbbef](https://github.com/studiometa/js-toolkit/commit/001fbbef), [d7352b26](https://github.com/studiometa/js-toolkit/commit/d7352b26), [b8b6cc8e](https://github.com/studiometa/js-toolkit/commit/b8b6cc8e)) - Improve `transform` and `tween` by replacing `isDefined()` with inline `!== undefined` checks ([#721](https://github.com/studiometa/js-toolkit/pull/721), [cdb5db8b](https://github.com/studiometa/js-toolkit/commit/cdb5db8b), [f6276353](https://github.com/studiometa/js-toolkit/commit/f6276353)) - Improve `getAllProperties` by replacing O(n²) array spread with O(n) push ([#721](https://github.com/studiometa/js-toolkit/pull/721), [d52459f6](https://github.com/studiometa/js-toolkit/commit/d52459f6)) + - Improve `$parent` and `$root` resolution by walking the DOM ancestors and reading each element's `__base__` map instead of scanning every instance, making it O(tree depth) instead of O(N instances) ([39895c51](https://github.com/studiometa/js-toolkit/commit/39895c51)) + - Improve element-scoped `$query` and `$queryAll` (`queryComponent`/`queryComponentAll` with a `from` element) by resolving matches through a scoped DOM traversal instead of a global instance scan ([7f12b364](https://github.com/studiometa/js-toolkit/commit/7f12b364)) +- Element-scoped `queryComponent`/`queryComponentAll` now return results in DOM order instead of mount order, and return a single instance when several live components share the same name on the same element ([7f12b364](https://github.com/studiometa/js-toolkit/commit/7f12b364)) ### Fixed diff --git a/packages/js-toolkit/Base/Base.ts b/packages/js-toolkit/Base/Base.ts index 7d1ab18f7..54f74a5e5 100644 --- a/packages/js-toolkit/Base/Base.ts +++ b/packages/js-toolkit/Base/Base.ts @@ -6,8 +6,7 @@ import { addInstance, deleteInstance, addToRegistry, - getInstances, - findClosestInstance, + hasInstance, } from './utils.js'; import { ChildrenManager, @@ -187,22 +186,30 @@ export class Base { * @internal */ get __parent(): (T['$parent'] & Base) | null { - const parents = new Set(); - - for (const instance of getInstances()) { - if (!instance.$config.components) continue; - - if ( - Object.values(instance.$config.components).includes(this.constructor) && - instance.$el.contains(this.$el) - ) { - parents.add(instance); + // Walk up the DOM ancestors and resolve the closest mounted instance whose + // config declares this component as a child, using each element's `__base__` + // map for an O(1)-per-ancestor lookup. This is O(tree depth) instead of the + // O(N_instances) global scan (plus `Set` copy) it replaces. + let el = this.$el.parentElement; + + while (el) { + const baseMap = (el as BaseEl).__base__; + + if (baseMap) { + for (const instance of baseMap.values()) { + if (instance === 'terminated' || !hasInstance(instance)) continue; + + const { components } = instance.$config; + if (components && Object.values(components).includes(this.constructor)) { + return instance as T['$parent'] & Base; + } + } } - } - const closest = findClosestInstance(this.$el, parents); + el = el.parentElement; + } - return closest ?? null; + return null; } /** diff --git a/packages/js-toolkit/Base/utils.ts b/packages/js-toolkit/Base/utils.ts index 3d6685ff2..8d69bb0f9 100644 --- a/packages/js-toolkit/Base/utils.ts +++ b/packages/js-toolkit/Base/utils.ts @@ -165,6 +165,15 @@ export function deleteInstance(instance: Base) { getElementsStorage().delete(instance.$el as HTMLElement & { __base__: Map }); } +/** + * Test whether an instance is currently registered in the global storage. + * Membership is added on `before-mounted` and removed on `after-destroyed`, + * so this is an O(1) check for "is this instance live" without copying the Set. + */ +export function hasInstance(instance: Base): boolean { + return getInstancesStorage().has(instance); +} + const registryKey = '__JS_TOOLKIT_REGISTRY__'; function registry() { diff --git a/packages/js-toolkit/helpers/queryComponent.ts b/packages/js-toolkit/helpers/queryComponent.ts index 0688955b7..a69f5bc94 100644 --- a/packages/js-toolkit/helpers/queryComponent.ts +++ b/packages/js-toolkit/helpers/queryComponent.ts @@ -1,5 +1,5 @@ import type { Base, BaseEl } from '../Base/index.js'; -import { getInstances } from '../Base/index.js'; +import { getInstances, hasInstance } from '../Base/utils.js'; import { memo } from '../utils/memo.js'; import { getAncestorWhere } from '../utils/dom/ancestors.js'; @@ -46,6 +46,81 @@ export function instanceIsMatching(instance: Base, parsedQuery: ParsedQuery): bo return true; } +/** + * Resolve the live instance attached to an element under a given name. + * + * Reads the element's `__base__` map (the same O(1)-per-element lookup + * `closestComponent` uses) and applies the single matching gate: + * - the entry must exist and not be the literal `'terminated'` string + * written by `$terminate` (`Base.ts`); + * - the instance must still be "live" (registered in the global storage + * between `before-mounted` and `after-destroyed`) — `__base__` keeps + * destroyed-but-not-terminated instances that `getInstances()` has already + * dropped, so `hasInstance` preserves the historical visibility window; + * - it must satisfy `instanceIsMatching` (name, CSS selector, state). + * + * The lookup is keyed on `parsedQuery.name` only — never on a name-derived DOM + * selector — because `__base__` membership is independent of whether the element + * carries a `data-component` attribute or a `w-name` tag. + */ +function resolveMatch(element: BaseEl, parsedQuery: ParsedQuery): T | undefined { + const baseMap = element.__base__; + if (!baseMap) return undefined; + + const instance = baseMap.get(parsedQuery.name); + if (!instance || instance === 'terminated') return undefined; + if (!hasInstance(instance) || !instanceIsMatching(instance, parsedQuery)) return undefined; + + return instance as T; +} + +/** + * Yield the instances matching the given query within the `from` root. + * + * When `from` is the document, there is no containment/attachment constraint, so + * every live matching instance is returned even if its `$el` is detached from the + * global document — the global instance storage is the source of truth here (a + * `document.querySelectorAll` scan would miss detached elements entirely). + * + * When `from` is an element, matches are resolved via a scoped + * `querySelectorAll('*')` descendant traversal plus an explicit check of `from` + * itself (which `querySelectorAll` excludes), each resolved through its + * `__base__` map. This scales with the size of the `from` subtree instead of the + * global instance count. `'*'` is required — a name-derived selector would miss + * components mounted on elements that carry no matching attribute/tag (e.g. + * `withName(Base, name)` on a plain element, or a component registered under an + * arbitrary CSS selector). + */ +function* queryMatches( + parsedQuery: ParsedQuery, + from: HTMLElement | Document, +): Generator { + if (from === document) { + for (const instance of getInstances()) { + if (hasInstance(instance) && instanceIsMatching(instance, parsedQuery)) { + yield instance as T; + } + } + return; + } + + const seen = new Set(); + + const selfMatch = resolveMatch(from as BaseEl, parsedQuery); + if (selfMatch) { + seen.add(selfMatch); + yield selfMatch; + } + + for (const element of from.querySelectorAll('*')) { + const match = resolveMatch(element as BaseEl, parsedQuery); + if (match && !seen.has(match)) { + seen.add(match); + yield match; + } + } +} + /** * Get the first instance of component with the given query. */ @@ -55,14 +130,12 @@ export function queryComponent( ): T | undefined { const parsedQuery = parseQuery(query); const { from = document } = options; - const instances = getInstances() as ReadonlySet; - - for (const instance of instances) { - if (!instanceIsMatching(instance, parsedQuery)) continue; - if (from !== document && !(from === instance.$el || from.contains(instance.$el))) continue; + for (const instance of queryMatches(parsedQuery, from)) { return instance; } + + return undefined; } /** @@ -74,17 +147,8 @@ export function queryComponentAll( ): T[] { const parsedQuery = parseQuery(query); const { from = document } = options; - const instances = getInstances() as ReadonlySet; - const selectedInstances = new Set(); - - for (const instance of instances) { - if (!instanceIsMatching(instance, parsedQuery)) continue; - if (from !== document && !(from === instance.$el || from.contains(instance.$el))) continue; - - selectedInstances.add(instance); - } - return Array.from(selectedInstances); + return Array.from(queryMatches(parsedQuery, from)); } /** diff --git a/packages/tests/Base/Base.spec.ts b/packages/tests/Base/Base.spec.ts index c6313718c..2e0ca7d51 100644 --- a/packages/tests/Base/Base.spec.ts +++ b/packages/tests/Base/Base.spec.ts @@ -314,6 +314,7 @@ describe('A Base instance', () => { const child = h('div', { dataComponent: 'ChildComponent' }); const div = h('div', [child]); + document.body.append(div); const component = new Component(div); await mount(component); diff --git a/packages/tests/__benchmarks__/parent-resolution.bench.ts b/packages/tests/__benchmarks__/parent-resolution.bench.ts new file mode 100644 index 000000000..b8e3903a7 --- /dev/null +++ b/packages/tests/__benchmarks__/parent-resolution.bench.ts @@ -0,0 +1,130 @@ +import { describe, bench } from 'vitest'; +import { Base } from '@studiometa/js-toolkit'; +import type { BaseConfig } from '@studiometa/js-toolkit'; +import { findClosestInstance } from '#private/Base/utils.js'; + +/** + * Benchmark for `__parent` resolution strategies. + * + * Current implementation (`Base/Base.ts` `__parent`) resolves the parent by + * scanning the whole global instance set on every access — O(N_instances), + * plus a full `Set` copy in `getInstances()`, plus an `$el.contains()` DOM walk + * for every instance whose config declares `this.constructor` as a child. + * + * The prototype walks the DOM ancestors of `$el` and uses each element's + * `__base__` map (the same O(1)-per-ancestor lookup `closestComponent` already + * uses) — O(tree depth), independent of the total number of mounted instances. + * + * The noise instances are `Parent`s, whose config declares `Child` as a child + * component. This is the worst case for the current algorithm: every noise + * instance passes the `.includes(target.constructor)` guard and therefore pays + * a full `$el.contains()` DOM walk, even though none of them is the real parent. + * + * Each scenario owns its own instance `Set` and its own detached DOM subtree, so + * the scenarios coexist without clobbering shared state and N genuinely varies + * at bench-run time (benches run after all `describe` bodies are collected). + */ + +class Child extends Base { + static config: BaseConfig = { name: 'Child' }; +} + +class Parent extends Base { + static config: BaseConfig = { name: 'Parent', components: { Child } }; +} + +/** Replicates the current `Base.ts` `__parent` getter, over an explicit set. */ +function resolveParentCurrent(target: Base, instances: Set): Base | null { + const parents = new Set(); + + // Mirror `getInstances()` copying the whole Set on every access. + for (const instance of new Set(instances)) { + if (!instance.$config.components) continue; + if ( + Object.values(instance.$config.components).includes(target.constructor as never) && + instance.$el.contains(target.$el) + ) { + parents.add(instance); + } + } + + return findClosestInstance(target.$el, parents) ?? null; +} + +/** Prototype: walk DOM ancestors, resolve via the per-element `__base__` map. */ +function resolveParentDomWalk(target: Base): Base | null { + let el: HTMLElement | null = target.$el.parentElement; + + while (el) { + const baseMap = (el as Base['$el']).__base__; + if (baseMap) { + for (const instance of baseMap.values()) { + if ((instance as unknown) === 'terminated') continue; + const components = instance.$config.components; + if (components && Object.values(components).includes(target.constructor as never)) { + return instance; + } + } + } + el = el.parentElement; + } + + return null; +} + +/** + * Build a self-contained scenario: a target Child nested `depth` levels under a + * real Parent, plus `noise` unrelated Parent instances. Everything lives in its + * own detached subtree and its own instance Set. + */ +function buildScenario(noise: number, depth: number): { child: Base; instances: Set } { + const instances = new Set(); + const root = document.createElement('div'); + + const parentEl = document.createElement('div'); + parentEl.dataset.component = 'Parent'; + root.appendChild(parentEl); + instances.add(new Parent(parentEl)); + + let host = parentEl; + for (let i = 0; i < depth; i++) { + const wrapper = document.createElement('div'); + host.appendChild(wrapper); + host = wrapper; + } + const childEl = document.createElement('div'); + childEl.dataset.component = 'Child'; + host.appendChild(childEl); + const child = new Child(childEl); + instances.add(child); + + for (let i = 0; i < noise; i++) { + const noiseEl = document.createElement('div'); + noiseEl.dataset.component = 'Parent'; + root.appendChild(noiseEl); + instances.add(new Parent(noiseEl)); + } + + return { child, instances }; +} + +const DEPTH = 5; + +for (const noise of [50, 500, 2000]) { + describe(`__parent resolution — ${noise} instances, depth ${DEPTH}`, () => { + const { child, instances } = buildScenario(noise, DEPTH); + + // Sanity: both strategies must agree on the resolved parent. + if (resolveParentCurrent(child, instances) !== resolveParentDomWalk(child)) { + throw new Error('Strategies disagree on the resolved parent'); + } + + bench('current (global scan)', () => { + resolveParentCurrent(child, instances); + }); + + bench('prototype (DOM ancestor walk)', () => { + resolveParentDomWalk(child); + }); + }); +} diff --git a/packages/tests/__benchmarks__/query-resolution.bench.ts b/packages/tests/__benchmarks__/query-resolution.bench.ts new file mode 100644 index 000000000..e46905194 --- /dev/null +++ b/packages/tests/__benchmarks__/query-resolution.bench.ts @@ -0,0 +1,140 @@ +import { describe, bench } from 'vitest'; +import { Base } from '@studiometa/js-toolkit'; +import type { BaseConfig } from '@studiometa/js-toolkit'; + +/** + * Benchmark for `queryComponent` / `queryComponentAll` resolution strategies + * when scoped to an element root (`{ from }`). + * + * The old implementation scanned the whole global instance set on every call — + * O(N_instances) — copying the `Set` (as `getInstances()` does) and running an + * `$el.contains()` DOM walk for every instance to enforce containment, even for + * instances that live entirely outside the `from` subtree. + * + * The new implementation resolves matches via a scoped `querySelectorAll('*')` + * descendant traversal plus a check of `from` itself, reading each element's + * `__base__` map (the same O(1)-per-element lookup `closestComponent` uses). It + * scales with the size of the `from` subtree, independent of how many unrelated + * instances are mounted elsewhere. + * + * The noise instances share the target's config name and are mounted OUTSIDE the + * `from` subtree — the worst case for the old algorithm: every noise instance + * matches by name and therefore pays a full `$el.contains()` DOM walk, yet the + * new strategy never visits them because they are not descendants of `from`. + * + * Each scenario owns its own instance `Set` and its own detached DOM subtree, so + * the scenarios coexist without clobbering shared global state and N genuinely + * varies at bench-run time (benches run after all `describe` bodies are + * collected). + */ + +class Query extends Base { + static config: BaseConfig = { name: 'Query' }; +} + +const NAME = 'Query'; + +/** Replicates the old global-scan strategy, over an explicit instance set. */ +function queryAllCurrent(from: HTMLElement, instances: Set): Base[] { + const selected = new Set(); + + // Mirror `getInstances()` copying the whole Set on every access. + for (const instance of new Set(instances)) { + if (instance.$config.name !== NAME) continue; + if (from === instance.$el || from.contains(instance.$el)) { + selected.add(instance); + } + } + + return Array.from(selected); +} + +/** New strategy: scoped `querySelectorAll('*')` + self + per-element `__base__`. */ +function queryAllDomScan(from: HTMLElement, live: Set): Base[] { + const selected = new Set(); + + const resolve = (el: Base['$el']) => { + const baseMap = el.__base__; + if (!baseMap) return; + const instance = baseMap.get(NAME); + // Skip terminated entries and instances no longer live (the `hasInstance` + // gate the real helper applies against the global storage). + if (!instance || (instance as unknown) === 'terminated' || !live.has(instance as Base)) return; + selected.add(instance as Base); + }; + + resolve(from as Base['$el']); + for (const el of from.querySelectorAll('*')) { + resolve(el as Base['$el']); + } + + return Array.from(selected); +} + +/** + * Build a self-contained scenario: a `from` container holding `MATCHES` target + * instances nested `depth` levels deep, plus `noise` unrelated instances of the + * same name mounted OUTSIDE `from`. Everything lives in its own detached subtree + * and its own instance Set. Constructing a `Query` populates the element's + * `__base__` map, so no mount is required. + */ +function buildScenario( + noise: number, + depth: number, + matches: number, +): { from: HTMLElement; instances: Set } { + const instances = new Set(); + const root = document.createElement('div'); + + const from = document.createElement('div'); + root.appendChild(from); + + let host = from; + for (let i = 0; i < depth; i++) { + const wrapper = document.createElement('div'); + host.appendChild(wrapper); + host = wrapper; + } + for (let i = 0; i < matches; i++) { + const el = document.createElement('div'); + host.appendChild(el); + instances.add(new Query(el)); + } + + // Noise: same-name instances OUTSIDE `from` (siblings under root). + for (let i = 0; i < noise; i++) { + const el = document.createElement('div'); + root.appendChild(el); + instances.add(new Query(el)); + } + + return { from, instances }; +} + +const DEPTH = 5; +const MATCHES = 5; + +function sameResult(a: Base[], b: Base[]): boolean { + if (a.length !== b.length) return false; + const setB = new Set(b); + return a.every((instance) => setB.has(instance)); +} + +for (const noise of [50, 500, 2000]) { + describe(`query resolution — ${noise} instances, depth ${DEPTH}`, () => { + const { from, instances } = buildScenario(noise, DEPTH, MATCHES); + + // Sanity: both strategies must resolve the same set of instances. + if (!sameResult(queryAllCurrent(from, instances), queryAllDomScan(from, instances))) { + throw new Error('Strategies disagree on the resolved instances'); + } + + bench('current (global scan)', () => { + queryAllCurrent(from, instances); + }); + + bench('prototype (scoped DOM scan)', () => { + queryAllDomScan(from, instances); + }); + }); +} diff --git a/packages/tests/helpers/queryComponent.spec.ts b/packages/tests/helpers/queryComponent.spec.ts index 34d34a70d..1993c9d78 100644 --- a/packages/tests/helpers/queryComponent.spec.ts +++ b/packages/tests/helpers/queryComponent.spec.ts @@ -121,6 +121,78 @@ describe('The `queryComponentAll` function', () => { }); }); +describe('The `queryComponent` / `queryComponentAll` __base__ resolution', () => { + it('returns detached elements with from=document (no attachment constraint)', async () => { + const name = randomName(); + const div = h('div'); // detached, never appended to the global document + const instance = new (withName(Base, name))(div); + await mount(instance); + expect(div.isConnected).toBe(false); + expect(queryComponent(name)).toBe(instance); + expect(queryComponentAll(name)).toEqual([instance]); + }); + + it('matches the `from` element itself as well as its descendants', async () => { + const name = randomName(); + const selfEl = h('div'); + const childEl = h('div'); + const root = h('div', [selfEl, childEl]); + const selfInstance = new (withName(Base, name))(selfEl); + const childInstance = new (withName(Base, name))(childEl); + await mount(selfInstance, childInstance); + + // Self match: `from` IS the instance element (querySelectorAll excludes it). + expect(queryComponent(name, { from: selfEl })).toBe(selfInstance); + expect(queryComponentAll(name, { from: selfEl })).toEqual([selfInstance]); + + // Descendants, in DOM order. + expect(queryComponentAll(name, { from: root })).toEqual([selfInstance, childInstance]); + }); + + it('resolves components on elements without a name-derived attribute', async () => { + const name = randomName(); + const plain = h('div'); // no data-component attribute, no `w-name` tag + const root = h('div', [plain]); + const instance = new (withName(Base, name))(plain); + await mount(instance); + + // A `getSelector`-based query would miss this element entirely; the + // `__base__` map lookup is keyed on config.name only, so it finds it. + expect(plain.getAttribute('data-component')).toBeNull(); + expect(queryComponent(name, { from: root })).toBe(instance); + expect(queryComponentAll(name, { from: root })).toEqual([instance]); + }); + + it('honors :mounted and :destroyed state filters from an element root', async () => { + const name = randomName(); + const div = h('div'); + const root = h('div', [div]); + const instance = new (withName(Base, name))(div); + await mount(instance); + + expect(queryComponent(`${name}:mounted`, { from: root })).toBe(instance); + expect(queryComponentAll(`${name}:mounted`, { from: root })).toEqual([instance]); + expect(queryComponent(`${name}:destroyed`, { from: root })).toBeUndefined(); + expect(queryComponentAll(`${name}:destroyed`, { from: root })).toEqual([]); + }); + + it('skips terminated __base__ entries', async () => { + const name = randomName(); + const div = h('div'); + const root = h('div', [div]); + const instance = new (withName(Base, name))(div); + await mount(instance); + expect(queryComponent(name, { from: root })).toBe(instance); + + await instance.$terminate(); + + // `$terminate` writes the literal `'terminated'` string in `__base__`. + expect((div as Base['$el']).__base__.get(name)).toBe('terminated'); + expect(queryComponent(name, { from: root })).toBeUndefined(); + expect(queryComponentAll(name, { from: root })).toEqual([]); + }); +}); + describe('The `closestComponent` function', () => { it('should return the closest instance matching the given query', async () => { const name = randomName();