mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-07 15:09:30 +00:00
fix: the authoritative adoption keeps its own task (#520)
The r1 diagnosis was wrong, and the measurement in the code review proved it: removing the two declarations from `static properties` left the cold start at 19 update cycles, 4 model builds and 4 config epochs, exactly the numbers of the bug. Lit's forced first-update change does mark `_serverCfg` changed, but at that moment the body and `_cfgEpochPreservedConfig` are both null, `preserveGeometry` is true and the epoch does not move. The comment above `static properties` now says that; the declaration still stays out, because two owners of one reactivity is what #500 removed. The real cause is the `await`. Before #500 everything from `_adoptStructuralResponses` to the end of the load ran in one task: the adopted bodies, `_adoptInitialSpace`, the viewport restore, `_loadOk`, and the device seeding — whose `_syncNewDevices`/`_seedHiddenDevices` write the config back — all landed in a single Lit update. #500 made the adoption an async sequence, so the caller resumes one microtask later, after Lit has already painted the adopted config; the seeding writes then arrive as a second config epoch, a second model build and a second paint of a 60-room house. `GatedAdoptionInput` gains `afterAdopt`, the mirror of `beforeAdopt`: it runs synchronously at the end of the sequence, before the promise resolves. `_loadFromServer` moves the viewport restore, `_loadOk` and the device rebuild into it — `_syncNewDevices` refuses to write before `_loadOk`, so the order inside the hook matters — and the load tail now rebuilds devices only when nothing was adopted. `_reloadConfigOnly` takes the same route. Measured with the project's own runner, 7 samples per profile, base `a44fbd37` against this tree (Chromium 152, sandbox): interaction modelReadyMs 761.3 ≤ 950.56 (base 731.2) firstStableRenderMs 2567.2 ≤ 3000 (base 2542.7) longTask.maxSingleMs 690 ≤ 921 · cache.entries.cleanFloor 100 isometric modelReadyMs 1252.9 ≤ 1499.76 (base 1249.8) firstStableRenderMs 1378 ≤ 1610.16 (base 1341.8) Boot diagnostics on both trees: 18 update cycles, 3 model builds, 3 config epochs, with the same epoch trace — the candidate is no longer distinguishable from the base. Witnesses. `config-adoption.test.mjs` queues a microtask at the start of the adoption and pins that `afterAdopt` runs before it — the probe fails the moment the hook crosses an await; `config-adoption-ownership.test.mjs` pins the wiring in the card and the hook's place in the sequence. Mutants `adoption-tail-defers-caller-hook` (defers the hook by one microtask) and `authoritative-load-seeds-devices-after-the-await` (drops the rebuild from the hook) redden them. The initial View graph grows 40 B gzip, so the #438 ceiling is recentred 300 300 → 300 400 with the usual dated note; measured 299 812 B keeps 588 B above and 1 412 B below the band. The overall 301 066 B budget and the #367 headroom debt are untouched. Issue: #520 User-Visible: no
This commit is contained in:
+16
-2
@@ -401,6 +401,15 @@ export interface GatedAdoptionInput {
|
||||
profile: 'reload' | 'post-write';
|
||||
/** Runs once the gate passed, before adoption (connection flags of the initial load). */
|
||||
beforeAdopt?: () => void;
|
||||
/**
|
||||
* Runs synchronously at the end of the sequence, in the same task as the
|
||||
* adoption. Work that must reach the renderer together with the adopted
|
||||
* body belongs here and nowhere else: the caller resumes only after
|
||||
* `await`, and by then Lit has already flushed an update on the adopted
|
||||
* config, so the same work done there costs a second config epoch, a
|
||||
* second model build and a second paint of the whole plan (#520).
|
||||
*/
|
||||
afterAdopt?: () => void;
|
||||
}
|
||||
|
||||
export type GatedAdoptionResult =
|
||||
@@ -409,8 +418,12 @@ export type GatedAdoptionResult =
|
||||
|
||||
/**
|
||||
* The one adoption sequence (#500 §6.3): compare → backdrop readiness gate →
|
||||
* continuity candidate → adopt → profile tail. On `asset-wait` nothing was
|
||||
* adopted; a load retry is already scheduled.
|
||||
* continuity candidate → adopt → profile tail → `afterAdopt`. On `asset-wait`
|
||||
* nothing was adopted; a load retry is already scheduled.
|
||||
*
|
||||
* Everything from the adoption to the end of this function runs in one task,
|
||||
* uninterrupted by a render: that atomicity is what keeps a cold start at one
|
||||
* model build for the adopted config (#520).
|
||||
*/
|
||||
export async function adoptAuthoritativeGated(
|
||||
host: ConfigAdoptionHostPort,
|
||||
@@ -442,5 +455,6 @@ export async function adoptAuthoritativeGated(
|
||||
host._resumePendingNavMode();
|
||||
host._cacheSnapshot();
|
||||
}
|
||||
input.afterAdopt?.();
|
||||
return { status: 'adopted', spaceChanged: host._space !== visibleSpace };
|
||||
}
|
||||
|
||||
+43
-18
@@ -2541,18 +2541,23 @@ export class HouseplanCard extends LitElement {
|
||||
* #520: `_serverCfg` и `_layout` здесь НЕ объявляются, хотя они реактивны.
|
||||
*
|
||||
* С #500 их тела принадлежат `_adoption`, а карточка видит их через
|
||||
* собственные аксессоры прототипа. Lit на такое объявление ставит флаг
|
||||
* `wrapped` (`createProperty`) и на ПЕРВОМ обновлении принудительно кладёт
|
||||
* свойство в `changedProperties` со старым значением `undefined` — даже
|
||||
* если никто ничего не присваивал. `willUpdate` читает это как замену
|
||||
* конфига, поднимает `_cfgEpoch`, ключ памятки модели меняется, и большой
|
||||
* дом собирает и рисует модель второй раз: +550 мс до первого устойчивого
|
||||
* кадра (замерено против базы `a44fbd37`, 3 эпохи против 4).
|
||||
* собственные аксессоры прототипа. Объявление сделало бы вторым владельцем
|
||||
* реактивности сам Lit: он ставит такому свойству флаг `wrapped`
|
||||
* (`createProperty`) и на ПЕРВОМ обновлении принудительно кладёт его в
|
||||
* `changedProperties` со старым значением `undefined` — даже если никто
|
||||
* ничего не присваивал. Сегодня этот лишний вход в ветку `willUpdate`
|
||||
* безвреден ровно по совпадению: на первом обновлении и тело, и
|
||||
* `_cfgEpochPreservedConfig` равны `null`, поэтому `preserveGeometry`
|
||||
* истинно и эпоха не растёт. Совпадение — не контракт: любой ранний
|
||||
* приход конфига (тёплый кеш) превращает его в лишний бамп эпохи.
|
||||
*
|
||||
* Реактивность даёт `_adoption` через `onBodyReplaced` → `requestUpdate`:
|
||||
* `requestUpdate` не требует объявления, `getPropertyOptions` возвращает
|
||||
* умолчание, и `changed.has('_serverCfg')` работает как прежде.
|
||||
* `noAccessor: true` не помогает — `wrapped` ставится до его проверки.
|
||||
*
|
||||
* Регрессию первого кадра из #520 чинило не это, а атомарность усыновления
|
||||
* (`afterAdopt` в `_loadFromServer`); см. комментарий там.
|
||||
*/
|
||||
static properties = {
|
||||
_tabDrag: { state: true },
|
||||
@@ -4283,6 +4288,15 @@ export class HouseplanCard extends LitElement {
|
||||
this._loadTries++;
|
||||
const visibleSpace = this._space;
|
||||
const hadViewport = !!this._view;
|
||||
// #520: whoever rebuilds the devices for this attempt does it exactly
|
||||
// once. On the adopted path that is `afterAdopt`, inside the adoption
|
||||
// task; the tail below only covers the attempts that never adopted.
|
||||
let devicesRebuilt = false;
|
||||
const rebuildDevices = (): void => {
|
||||
devicesRebuilt = true;
|
||||
this._regSignature = '';
|
||||
this._maybeRebuildDevices();
|
||||
};
|
||||
try {
|
||||
const [cfgResp, layResp] = await Promise.all([
|
||||
this._getAuthoritativeConfig(),
|
||||
@@ -4294,15 +4308,27 @@ export class HouseplanCard extends LitElement {
|
||||
this._connectionWasLost = false;
|
||||
this._serverStorage = true;
|
||||
},
|
||||
// #520: everything that touches the viewport, the readiness flag or
|
||||
// the devices belongs in the same task as the adoption. Seeding
|
||||
// devices writes the config back (new devices, hidden filter), and
|
||||
// after the `await` Lit has already painted the adopted body — the
|
||||
// writes then cost a second config epoch, a second model build and a
|
||||
// second paint of a 60-room house, ~550 ms of the first stable frame.
|
||||
afterAdopt: () => {
|
||||
// DEV-B703-03: a warm re-mount already holds the exact viewport of
|
||||
// the instance that was thrown away; the centred restore here IS
|
||||
// the reported jerk. Only a genuine navigation (the hash/nav landed
|
||||
// us on another space) still needs it.
|
||||
if (this._warmVpArmed && this._space === this._warmVp?.space) this._warmVpArmed = false;
|
||||
else if (!hadViewport || this._space !== visibleSpace) this._restoreZoom();
|
||||
// `_syncNewDevices` and `_syncAreaRelocations` refuse to write
|
||||
// before the authoritative snapshot is usable, and it is usable
|
||||
// exactly here — the bodies are adopted.
|
||||
this._loadOk = true;
|
||||
rebuildDevices();
|
||||
},
|
||||
});
|
||||
if (adopted.status !== 'adopted') return;
|
||||
// DEV-B703-03: a warm re-mount already holds the exact viewport of the
|
||||
// instance that was thrown away; the centred restore here IS the
|
||||
// reported jerk. Only a genuine navigation (the hash/nav landed us on
|
||||
// another space) still needs it.
|
||||
if (this._warmVpArmed && this._space === this._warmVp?.space) this._warmVpArmed = false;
|
||||
else if (!hadViewport || this._space !== visibleSpace) this._restoreZoom();
|
||||
this._loadOk = true;
|
||||
// Trails and event subscriptions enrich an already complete snapshot.
|
||||
// A read-only HA session may reject these; that must never roll the
|
||||
// accepted config back into the mandatory load catch.
|
||||
@@ -4337,8 +4363,7 @@ export class HouseplanCard extends LitElement {
|
||||
// failure. Leaving it false on an early return stranded the controller
|
||||
// before its own two-second barrier could ever start.
|
||||
this._continuityDataReady = true;
|
||||
this._regSignature = '';
|
||||
this._maybeRebuildDevices();
|
||||
if (!devicesRebuilt) rebuildDevices();
|
||||
this.requestUpdate();
|
||||
}
|
||||
}
|
||||
@@ -4458,11 +4483,11 @@ export class HouseplanCard extends LitElement {
|
||||
const resp = await this._getAuthoritativeConfig();
|
||||
const adopted = await this._adoptAuthoritative({
|
||||
cfgResp: resp, reason: 'config-reload', profile: 'reload',
|
||||
// #520: same task as the adoption — see `_loadFromServer`.
|
||||
afterAdopt: () => { this._regSignature = ''; this._maybeRebuildDevices(); },
|
||||
});
|
||||
if (adopted.status !== 'adopted') return;
|
||||
if (adopted.spaceChanged) this._restoreZoom();
|
||||
this._regSignature = '';
|
||||
this._maybeRebuildDevices();
|
||||
this.requestUpdate();
|
||||
} catch (e: any) {
|
||||
// a failed reload leaves the card on its last known config; tell the user
|
||||
|
||||
Reference in New Issue
Block a user