mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 04:09:17 +00:00
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018qZfe7YS4rqEMKoVeS3GKd
This commit is contained in:
@@ -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');
|
||||
|
||||
@@ -91,6 +91,34 @@ export function observeDeviceHitGeometryScroll(
|
||||
};
|
||||
}
|
||||
|
||||
type DeviceLayerMutation = Pick<MutationRecord, 'target' | 'addedNodes' | 'removedNodes'>;
|
||||
|
||||
/**
|
||||
* 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<DeviceLayerMutation>,
|
||||
syncAdded: (node: Node) => void,
|
||||
): boolean {
|
||||
const checked = new Set<Node>();
|
||||
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 => {
|
||||
|
||||
+3
-15
@@ -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,
|
||||
|
||||
@@ -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';
|
||||
|
||||
+5
-2
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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, /<g class="hp-stair hp-stair-view /, 'слой View помечает свои лестницы');
|
||||
});
|
||||
|
||||
// #694: a View-layer host whose `_model` getter counts its reads, as the card's
|
||||
// getter fingerprints the whole config on each one.
|
||||
const FLOORS = [
|
||||
{ id: 'ground', title: 'Ground floor' },
|
||||
{ id: 'upper', title: 'Upper floor' },
|
||||
{ id: 'attic', title: 'Attic' },
|
||||
];
|
||||
const stairViewHost = (stairs, { mode = 'view', fixedFloor = false } = {}) => ({
|
||||
reads: 0,
|
||||
tips: [],
|
||||
get _model() { this.reads += 1; return FLOORS; },
|
||||
_mode: mode,
|
||||
_curSpaceCfg: { stairs },
|
||||
_space: 'ground',
|
||||
_hasFixedFloor: fixedFloor,
|
||||
_suppressClick: false,
|
||||
_cellCm: 5,
|
||||
_gridPitch: 1,
|
||||
_decorStyle: { color: '#607d8b', opacity: 1 },
|
||||
_tabClick() {},
|
||||
_t: (key, vars) => (vars ? `${key}:${vars.title}` : key),
|
||||
_showTip(event, title, meta) { this.tips.push([title, meta]); },
|
||||
_clearPointerHover() {},
|
||||
});
|
||||
/** The value bound right after `attribute=` in a lit template. */
|
||||
const boundValue = (template, attribute) => {
|
||||
const index = template.strings.findIndex((part) => part.trimEnd().endsWith(`${attribute}=`));
|
||||
assert.ok(index >= 0, `the stair template binds ${attribute}`);
|
||||
return template.values[index];
|
||||
};
|
||||
|
||||
test('#694 AC2 the View stair layer reads the card model once per render at any stair count', () => {
|
||||
const targets = ['upper', 'attic', 'ground', null, 'gone'];
|
||||
const titles = { upper: 'Upper floor', attic: 'Attic' };
|
||||
for (const count of [0, 1, 2, 7, 40]) {
|
||||
const stairs = Array.from({ length: count }, (_, index) => straight({
|
||||
id: `stair-${index}`, x: 0.1 + index * 0.02, target_space_id: targets[index % targets.length],
|
||||
}));
|
||||
const host = stairViewHost(stairs);
|
||||
const layer = new StairViewRuntime(host).renderLayer();
|
||||
assert.equal(host.reads, 1, `${count} stairs: one read of _model per render`);
|
||||
const items = layer.values[0];
|
||||
assert.equal(items.length, count);
|
||||
for (const item of items) boundValue(item, '@pointerenter')({});
|
||||
assert.deepEqual(host.tips, stairs.flatMap((stair) => (
|
||||
titles[stair.target_space_id] ? [[`stairs.tooltip_navigate:${titles[stair.target_space_id]}`, '']] : []
|
||||
)), `${count} stairs: each link still names its target floor, the rest name none`);
|
||||
assert.deepEqual(items.map((item) => boundValue(item, 'data-target-state')),
|
||||
stairs.map((stair) => ({ upper: 'active', attic: 'active', ground: 'self', gone: 'deleted' })[
|
||||
stair.target_space_id] ?? 'missing'));
|
||||
}
|
||||
for (const options of [{ fixedFloor: true }, { mode: 'plan' }]) {
|
||||
const host = stairViewHost([straight({ id: 'a' }), straight({ id: 'b', target_space_id: 'attic' })], options);
|
||||
const layer = new StairViewRuntime(host).renderLayer();
|
||||
for (const item of layer.values[0]) boundValue(item, '@pointerenter')({});
|
||||
assert.equal(host.reads, 1, `${JSON.stringify(options)}: one read`);
|
||||
assert.deepEqual(host.tips, [], `${JSON.stringify(options)}: no stair is a link, none announces a floor`);
|
||||
}
|
||||
});
|
||||
|
||||
+1
-1
@@ -24,7 +24,7 @@
|
||||
"src/virtual-light-state.ts", "src/config-store.ts", "src/config-reload-authority.ts", "src/config-write-conflict.ts", "src/summary-panel.ts", "src/summary-panel-metrics.ts", "src/summary-panel-i18n.ts", "src/summary-panel-identity.ts", "src/summary-panel-picker.ts", "src/summary-panel-runtime-loaded.ts", "src/header-menu.ts", "src/iso-materials.ts", "src/iso-first-frame.ts", "src/iso-tiles.ts", "src/iso-sun.ts",
|
||||
"src/backdrop-probe.ts",
|
||||
"src/types.ts", "src/canvas-constants.ts", "src/editors/dialog-baseline.ts", "src/editors/color-tile-ink.ts", "src/editors/general-form-state.ts", "src/editors/space-form-state.ts", "src/editors/marker-form-state.ts", "src/editors/room-form-state.ts",
|
||||
"src/space-geometry.ts", "src/stairs.ts", "src/clean-floor.ts", "src/stairs-editor-model.ts", "src/stairs-box.ts", "src/junction-limits.ts", "src/room-gear-drag.ts",
|
||||
"src/space-geometry.ts", "src/stairs.ts", "src/clean-floor.ts", "src/stairs-editor-model.ts", "src/stairs-box.ts", "src/stairs-view.ts", "src/junction-limits.ts", "src/room-gear-drag.ts",
|
||||
"src/space-order.ts", "src/card-editor-validation.ts",
|
||||
"src/signing.ts", "src/initial-load.ts", "src/space-model-selection.ts", "src/space-dialog.ts",
|
||||
"src/visual-continuity.ts", "src/version-recovery.ts", "src/version-recovery-card.ts", "src/mode-transition.ts", "src/viewport-transition.ts", "src/boot-soft-layout.ts", "src/room-fit.ts", "src/editor-runtime-loader.ts", "src/editor-secondary.ts", "src/pointer-modality.ts", "src/touch-gesture-click-guard.ts",
|
||||
|
||||
Reference in New Issue
Block a user