From 6f17b6cd4cc6da5a416a18afaab6d732ef65670a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 02:01:45 +0300 Subject: [PATCH] perf(card): a floor switch stops re-querying the same subtrees (#694) A floor switch replaces the whole stage, so the card's pointer-hover MutationObserver receives hundreds of records whose targets are the same few containers. Each record re-ran `matches` and a `.devlayer` subtree `querySelector` on its target, and kept doing so after the device layer had already been found. The batch logic moves to `deviceLayerMutated` in device-hit-owner.ts: a node is checked at most once per batch, the first hit ends the checks, and every added node still goes through `_syncPointerHoverSubtree` in record order. The card shrinks by 12 lines. The View stair layer read the card's `_model` getter once more for every navigable stair; the getter rebuilds the config fingerprint on each read. `renderLayer` now reads it once. `languageRenderGate` wrote `lang` on the host on every render. It now writes it only when the value differs (language switch, English fallback, a foreign value); an unchanged value is left alone. No behaviour changes: DOM, tooltips and pixels are the same. Unit tests count subtree queries per node, `_model` reads per render and `lang` writes; one mutant per change restores the old behaviour. Issue: #694 User-Visible: no Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018qZfe7YS4rqEMKoVeS3GKd --- scripts/mutation-registry.mjs | 35 +++++++++++++++++ src/device-hit-owner.ts | 28 ++++++++++++++ src/houseplan-card.ts | 18 ++------- src/i18n/language-runtime.ts | 6 ++- src/stairs-view.ts | 7 +++- test/device-hit-owner.test.mjs | 71 +++++++++++++++++++++++++++++++++- test/i18n-runtime.test.mjs | 38 ++++++++++++++++++ test/stairs.test.mjs | 61 +++++++++++++++++++++++++++++ tsconfig.test.json | 2 +- 9 files changed, 246 insertions(+), 20 deletions(-) diff --git a/scripts/mutation-registry.mjs b/scripts/mutation-registry.mjs index d59c01f3..8bd15685 100644 --- a/scripts/mutation-registry.mjs +++ b/scripts/mutation-registry.mjs @@ -13808,6 +13808,41 @@ const MUTANT_DEFINITIONS = [ replace: '', }], }, + // #694: цена переключения этажа — три правки без изменения поведения. + { + id: 'pointer-hover-batch-requeries-shared-subtree', + guard: 'node --test --test-name-pattern="#694 AC1" test/device-hit-owner.test.mjs', + because: '#694 AC1: a floor switch delivers hundreds of MutationObserver records into the ' + + 'same subtrees; each node is queried once per batch, not once per record', + patches: [{ + file: 'src/device-hit-owner.ts', + find: ' if (node.nodeType !== 1 || checked.has(node)) return false;\n', + replace: ' if (node.nodeType !== 1) return false; // mutant: no per-batch memo\n', + }], + }, + { + id: 'stairs-view-reads-model-per-stair', + guard: 'node --test --test-name-pattern="#694 AC2" test/stairs.test.mjs', + because: '#694 AC2: the card _model getter fingerprints the whole config on every read; ' + + 'the View stair layer reads it once per render, not once more per navigable stair', + patches: [{ + file: 'src/stairs-view.ts', + find: " ? model.find((item) => item.id === stair.target_space_id)?.title ?? ''\n", + replace: " ? this.owner._model.find((item) => item.id === stair.target_space_id)?.title ?? ''" + + ' // mutant\n', + }], + }, + { + id: 'language-gate-rewrites-unchanged-lang', + guard: 'node --test --test-name-pattern="#694 AC3" test/i18n-runtime.test.mjs', + because: '#694 AC3: every card render passes the locale gate; an unchanged inherited lang ' + + 'must not be written again', + patches: [{ + file: 'src/i18n/language-runtime.ts', + find: " if (host.getAttribute?.('lang') !== lang) host.setAttribute('lang', lang);\n", + replace: " host.setAttribute('lang', lang); // mutant: written on every render\n", + }], + }, ]; const mutationCardSource = readFileSync(join(repoRoot, 'src/houseplan-card.ts'), 'utf8'); diff --git a/src/device-hit-owner.ts b/src/device-hit-owner.ts index d30046b1..aa4cd88a 100644 --- a/src/device-hit-owner.ts +++ b/src/device-hit-owner.ts @@ -91,6 +91,34 @@ export function observeDeviceHitGeometryScroll( }; } +type DeviceLayerMutation = Pick; + +/** + * One batch of the card's pointer-hover MutationObserver: every added node goes + * through `syncAdded`, and the result says whether the device layer changed. + * A floor switch delivers hundreds of records into the same subtrees (#694), + * so a node is queried at most once per batch and the first hit ends the queries. + */ +export function deviceLayerMutated( + records: Iterable, + syncAdded: (node: Node) => void, +): boolean { + const checked = new Set(); + const inDeviceLayer = (node: Node): boolean => { + if (node.nodeType !== 1 || checked.has(node)) return false; + checked.add(node); + const element = node as Element; + return element.matches('.devlayer, .devlayer *') || !!element.querySelector?.('.devlayer'); + }; + let changed = false; + for (const record of records) { + for (const node of record.addedNodes) syncAdded(node); + if (!changed && (inDeviceLayer(record.target) + || [...record.addedNodes, ...record.removedNodes].some(inDeviceLayer))) changed = true; + } + return changed; +} + const EPSILON = 1e-7; const distanceSquared = (a: DeviceHitPoint, b: DeviceHitPoint): number => { diff --git a/src/houseplan-card.ts b/src/houseplan-card.ts index 6af6f3a5..a275ee94 100755 --- a/src/houseplan-card.ts +++ b/src/houseplan-card.ts @@ -71,7 +71,7 @@ import { type FixedFloorSelection, type InitialSpaceSelection, } from './initial-load'; import { TouchGestureClickGuard } from './touch-gesture-click-guard'; -import { DeviceHitController, observeDeviceHitGeometryScroll } from './device-hit-owner'; +import { DeviceHitController, deviceLayerMutated, observeDeviceHitGeometryScroll } from './device-hit-owner'; import { selectActiveSpaceModel, selectSpaceModelById } from './space-model-selection'; import { roomTempRangeFromDraft, type SpaceDialogState } from './space-dialog'; import { mdiHomeCityOutline } from '@mdi/js'; @@ -2542,21 +2542,9 @@ export class HouseplanCard extends LitElement { const PointerHoverObserver = this.ownerDocument.defaultView?.MutationObserver; if (PointerHoverObserver) { this._pointerHoverObserver = new PointerHoverObserver((records) => { - let deviceGeometryChanged = false; - for (const record of records) { - for (const node of record.addedNodes) this._syncPointerHoverSubtree(node); - const inDeviceLayer = (node: Node): boolean => { - if (node.nodeType !== Node.ELEMENT_NODE) return false; - const element = node as Element; - return element.matches('.devlayer, .devlayer *') - || !!element.querySelector?.('.devlayer'); - }; - if (inDeviceLayer(record.target) - || [...record.addedNodes, ...record.removedNodes].some(inDeviceLayer)) { - deviceGeometryChanged = true; - } + if (deviceLayerMutated(records, (node) => this._syncPointerHoverSubtree(node))) { + this._invalidateDeviceHitGeometry(); } - if (deviceGeometryChanged) this._invalidateDeviceHitGeometry(); }); this._pointerHoverObserver.observe(this.renderRoot, { childList: true, diff --git a/src/i18n/language-runtime.ts b/src/i18n/language-runtime.ts index 5bec5861..06fe7772 100644 --- a/src/i18n/language-runtime.ts +++ b/src/i18n/language-runtime.ts @@ -95,6 +95,7 @@ interface LanguageHostElement { isConnected: boolean; requestUpdate(): void; setAttribute(name: string, value: string): void; + getAttribute?(name: string): string | null; removeAttribute(name: string): void; } @@ -114,7 +115,10 @@ export function languageRenderGate( host.removeAttribute('aria-busy'); } if (code) { - host.setAttribute('lang', state === 'fallback' ? 'en' : code); + // #694: `lang` is inherited; a write, even of the same value, can + // invalidate style for the whole shadow tree. Write it only on change. + const lang = state === 'fallback' ? 'en' : code; + if (host.getAttribute?.('lang') !== lang) host.setAttribute('lang', lang); committedHosts.add(host); } return 'ready'; diff --git a/src/stairs-view.ts b/src/stairs-view.ts index b68363cc..a3833ab3 100644 --- a/src/stairs-view.ts +++ b/src/stairs-view.ts @@ -61,7 +61,10 @@ export class StairViewRuntime { } public renderLayer(): TemplateResult { - const spaceIds = new Set(this.owner._model.map((item) => item.id)); + // #694: the card's `_model` getter fingerprints the whole config on every + // read; read it once per layer, not once more per navigable stair. + const model = this.owner._model; + const spaceIds = new Set(model.map((item) => item.id)); const interactive = this.owner._mode === 'view'; const items = this.stairs.map((stair) => { const geometry = cachedStairRenderGeometry(stair, this.owner._cellCm); @@ -73,7 +76,7 @@ export class StairViewRuntime { // #676 К8: the tooltip has exactly the link's condition — `active` — so a // missing, self, deleted or fixed-floor target never announces a floor. const targetTitle = active - ? this.owner._model.find((item) => item.id === stair.target_space_id)?.title ?? '' + ? model.find((item) => item.id === stair.target_space_id)?.title ?? '' : ''; const tip = (event: PointerEvent): void => { if (!active) return; diff --git a/test/device-hit-owner.test.mjs b/test/device-hit-owner.test.mjs index e6645d7b..fbcc3d24 100644 --- a/test/device-hit-owner.test.mjs +++ b/test/device-hit-owner.test.mjs @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import { - DeviceHitIndex, DevicePointerOwnerLatch, deviceHitScrollSources, + DeviceHitIndex, DevicePointerOwnerLatch, deviceHitScrollSources, deviceLayerMutated, observeDeviceHitGeometryScroll, pointInDeviceCapsule, resolveDeviceHitOwner, } from '../test-build/device-hit-owner.js'; @@ -130,3 +130,72 @@ test('#613 scroll observation crosses shadow hosts and tears down exactly once', assert.equal(invalidations, 4); reconnect(); }); + +// #694: an instrumented element counts every selector query run against it. +// `device` answers `.devlayer, .devlayer *`; `holdsDevice` answers the subtree +// query for `.devlayer` below it. +const probeNode = (name, { device = false, holdsDevice = false } = {}) => ({ + name, + nodeType: 1, + queries: 0, + matches(selector) { + assert.equal(selector, '.devlayer, .devlayer *'); + this.queries += 1; + return device; + }, + querySelector(selector) { + assert.equal(selector, '.devlayer'); + this.queries += 1; + return holdsDevice ? {} : null; + }, +}); +const record = (target, addedNodes = [], removedNodes = []) => ({ target, addedNodes, removedNodes }); + +test('#694 AC1 a batch into one subtree queries each node at most once', () => { + const stage = probeNode('stage'); + const children = Array.from({ length: 40 }, (_, index) => probeNode(`child-${index}`)); + const text = { nodeType: 3, name: 'text' }; + const records = [ + ...children.map((child) => record(stage, [child])), + ...children.map((child) => record(child)), + record(stage, [text], [children[0]]), + ]; + const synced = []; + assert.equal(deviceLayerMutated(records, (node) => synced.push(node.name)), false); + // matches + querySelector: one check is two queries, never more. + assert.equal(stage.queries, 2, 'the shared record target is checked once per batch'); + for (const child of children) assert.equal(child.queries, 2, `${child.name} is checked once`); + assert.deepEqual(synced, [...children.map((child) => child.name), 'text'], + 'every added node still reaches the pointer-hover sync, in record order'); +}); + +test('#694 AC1 the first device-layer hit ends the queries, not the hover sync', () => { + const stage = probeNode('stage'); + const devlayer = probeNode('devlayer', { device: true }); + const later = Array.from({ length: 12 }, (_, index) => probeNode(`later-${index}`)); + const added = later.map((_, index) => probeNode(`added-${index}`)); + const records = [ + record(stage), + record(devlayer), + ...later.map((target, index) => record(target, [added[index]], [probeNode('gone')])), + ]; + const synced = []; + assert.equal(deviceLayerMutated(records, (node) => synced.push(node.name)), true); + assert.equal(stage.queries, 2); + assert.equal(devlayer.queries, 1, 'a match needs no subtree query'); + for (const node of [...later, ...added]) { + assert.equal(node.queries, 0, `${node.name} is not queried after the first hit`); + } + assert.deepEqual(synced, added.map((node) => node.name), + 'added nodes after the hit still reach the pointer-hover sync'); +}); + +test('#694 AC1 a device layer is still found in targets, added and removed nodes', () => { + const plain = () => probeNode('plain'); + const none = () => {}; + assert.equal(deviceLayerMutated([record(plain(), [plain()], [plain()])], none), false); + assert.equal(deviceLayerMutated([record(plain()), record(probeNode('dev', { device: true }))], none), true); + assert.equal(deviceLayerMutated([record(plain(), [probeNode('floor', { holdsDevice: true })])], none), true); + assert.equal(deviceLayerMutated([record(plain(), [], [probeNode('old', { holdsDevice: true })])], none), true); + assert.equal(deviceLayerMutated([record({ nodeType: 3 }, [{ nodeType: 3 }])], none), false); +}); diff --git a/test/i18n-runtime.test.mjs b/test/i18n-runtime.test.mjs index 62a0961e..12e7712f 100644 --- a/test/i18n-runtime.test.mjs +++ b/test/i18n-runtime.test.mjs @@ -130,6 +130,44 @@ test('locale render gate exposes the fallback language to assistive technology', assert.equal(host.attrs.get('lang'), 'en'); }); +// #694: a real element answers getAttribute; count only the `lang` writes. +class AttributeHost extends FakeHost { + langWrites = []; + getAttribute(name) { return this.attrs.has(name) ? this.attrs.get(name) : null; } + setAttribute(name, value) { + if (name === 'lang') this.langWrites.push(value); + super.setAttribute(name, value); + } +} + +test('#694 AC3 the locale render gate writes lang only when the value changes', async () => { + const runtime = new LanguageRuntime([ + { code: 'en', dictionary: {} }, + { code: 'de', dictionary: {} }, + { code: 'fr', loadDictionary: async () => { throw new Error('offline'); } }, + ], 'build', () => {}); + const host = new AttributeHost(); + const renders = (code, times = 3) => { + for (let index = 0; index < times; index += 1) languageRenderGate(host, runtime, code); + }; + renders('en'); + assert.deepEqual(host.langWrites, ['en'], 'repeated renders keep the attribute that is already right'); + renders('de'); + assert.deepEqual(host.langWrites, ['en', 'de'], 'a language switch writes the new language once'); + assert.equal(languageRenderGate(host, runtime, 'fr'), 'warm'); + await runtime.ensure('fr'); + renders('fr'); + assert.deepEqual(host.langWrites, ['en', 'de', 'en'], 'the English fallback is written once'); + assert.equal(host.attrs.get('lang'), 'en'); + renders('de'); + renders(null); + assert.deepEqual(host.langWrites, ['en', 'de', 'en', 'de'], 'no language code writes nothing'); + host.attrs.set('lang', 'ru'); + renders('de'); + assert.deepEqual(host.langWrites, ['en', 'de', 'en', 'de', 'de'], 'a foreign value is corrected'); + assert.equal(host.attrs.get('lang'), 'de'); +}); + test('the production runtime is the tested class, not a handwritten twin (#354)', async () => { const registry = await import('../test-build/i18n/registry.js'); assert.ok( diff --git a/test/stairs.test.mjs b/test/stairs.test.mjs index 57b06a42..4b8adaa6 100644 --- a/test/stairs.test.mjs +++ b/test/stairs.test.mjs @@ -29,6 +29,7 @@ import { stairPhysicalSizeCm, stairTargetState, } from '../test-build/stairs-editor-model.js'; +import { StairViewRuntime } from '../test-build/stairs-view.js'; const straight = (extra = {}) => ({ id: 'straight', kind: 'straight', x: 0.5, y: 0.5, angle: 0, @@ -397,3 +398,63 @@ test('#693 курсор move над телом лестницы — только const view = readFileSync(new URL('../src/stairs-view.ts', import.meta.url), 'utf8'); assert.match(view, /