From e74bc0d6f669a6bd0dafdbc7ad7ad6128e04e36e Mon Sep 17 00:00:00 2001 From: "isaprettycoolguy@protonmail.com" <6576516+nyxsys@users.noreply.github.com> Date: Tue, 30 Jun 2026 06:57:16 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20eliminate=20O(n=C2=B2)=20layout=20thrash?= =?UTF-8?q?=20in=20renderCoordLabelsLayer=20(open-time=20stall)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit renderCoordLabelsLayer read each hex's offsetWidth/offsetLeft/offsetTop and created the label DOM inside the SAME loop. Every offset read after a DOM write forces a synchronous layout flush, so the loop was O(n²): on the chult map (3843 hexes) opening stalled the main thread ~21s (regression from commit 10475fd, the coord-label layer). Split into a read phase (collect all hex geometry into an array) then a write phase (create all labels) — matches the read-then-write pattern already used by renderPathOverlay/renderFactionOverlay. ~21s → ~0.1s (dev/coord-label-thrash- bench measures ~198× batched vs interleaved). Bakes in a regression guard: CLAUDE.md convention forbidding interleaved layout-read / DOM-write in per-hex loops, plus the committed benchmark. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 1 + dev/coord-label-thrash-bench.html | 131 ++++++++++++++++++++++++++++++ dev/coord-label-thrash-bench.mjs | 36 ++++++++ src/hex-map/HexMapView.ts | 40 ++++++--- 4 files changed, 196 insertions(+), 12 deletions(-) create mode 100644 dev/coord-label-thrash-bench.html create mode 100644 dev/coord-label-thrash-bench.mjs diff --git a/CLAUDE.md b/CLAUDE.md index ab4835a..cfb1fd7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -186,6 +186,7 @@ Renders an interactive hex-grid map for tabletop RPG world-building inside Obsid - Exclude notes whose `basename` starts with `"_"` whenever enumerating vault files for display, dropdowns, or linking. Pattern: `.filter(f => !f.basename.startsWith("_"))`. - **All modals must extend `DuckmageModal`** (`src/DuckmageModal.ts`) rather than Obsidian's `Modal` directly. `DuckmageModal` is the single place for shared modal behaviour. Any functionality needed by more than one modal belongs there, not in `utils.ts` or duplicated across files. - All modals call `this.makeDraggable()` in `onOpen` (inherited from `DuckmageModal`), which adds the `duckmage-editor-modal-drag` class and locks dragging to the title-bar area. (`FileLinkSuggestModal` is the sole exception — it extends `SuggestModal` and cannot inherit from `DuckmageModal`.) +- **NEVER interleave layout reads (`offsetWidth/Height/Left/Top`, `getBoundingClientRect`, `getComputedStyle`) with DOM writes (`createDiv`/`createEl`/`appendChild`/`setCssProps`/style changes) inside a per-hex loop.** Each read after a write forces a full synchronous layout flush → O(n²) thrash. On large maps (chult = 3843 hexes) this stalled the main thread for ~20s on open (regression in `renderCoordLabelsLayer`, commit 10475fd; fixed by batching). **Pattern:** read ALL geometry into a plain array first, THEN do all DOM writes. This is the established pattern in `renderPathOverlay`/`renderFactionOverlay` (build `centerMap` read-only, then build the SVG). The overlay/label builders in `HexMapView` are the hot path — any new one MUST follow read-then-write. Reproduce/validate with `dev/coord-label-thrash-bench.{html,mjs}` (~198× speedup batched vs interleaved). ### Obsidian reviewer-bot conventions (enforced beyond what eslint catches) diff --git a/dev/coord-label-thrash-bench.html b/dev/coord-label-thrash-bench.html new file mode 100644 index 0000000..488b190 --- /dev/null +++ b/dev/coord-label-thrash-bench.html @@ -0,0 +1,131 @@ + + + + + + + +
+ + + diff --git a/dev/coord-label-thrash-bench.mjs b/dev/coord-label-thrash-bench.mjs new file mode 100644 index 0000000..42dd5cf --- /dev/null +++ b/dev/coord-label-thrash-bench.mjs @@ -0,0 +1,36 @@ +#!/usr/bin/env node +/** Reproduces the renderCoordLabelsLayer layout-thrash regression (commit + * 10475fd) and validates the read-then-write fix, at the real chult map's + * hex count, with and without a bg image. Headless Chromium — absolute ms + * differ from the user's machine but the thrash→batch ratio is the signal. + */ +import path from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { withBrowser } from "/mnt/c/Users/markr/work/tools/chromium-driver/lib/browser.mjs"; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const sandbox = path.join(here, "coord-label-thrash-bench.html"); + +const ITERS = 3; +const r = await withBrowser(async ({ page }) => { + await page.goto(pathToFileURL(sandbox).href, { waitUntil: "networkidle0" }); + await page.waitForFunction(() => document.title === "ready", { timeout: 10000 }); + const hexCount = await page.evaluate(() => window.__hexCount); + const m = (which, bg) => page.evaluate((w, b, n) => window.__measure(w, b, n), which, bg, ITERS); + return { + hexCount, + iters: ITERS, + thrash_noBg: await m("thrash", false), + batch_noBg: await m("batch", false), + thrash_bg: await m("thrash", true), + batch_bg: await m("batch", true), + }; +}); + +const f = (n) => n.toFixed(1).padStart(9); +console.log(`\nrenderCoordLabelsLayer cost — hexes=${r.hexCount}, median of ${r.iters} (ms)\n`); +console.log(` no bg image with bg image`); +console.log(`thrash (current) ${f(r.thrash_noBg)} ${f(r.thrash_bg)} ← regression`); +console.log(`batch (fixed) ${f(r.batch_noBg)} ${f(r.batch_bg)}`); +console.log(`\nspeedup no-bg: ${(r.thrash_noBg / r.batch_noBg).toFixed(1)}× with-bg: ${(r.thrash_bg / r.batch_bg).toFixed(1)}×`); +console.log(`bg amplifies thrash by ${(r.thrash_bg / r.thrash_noBg).toFixed(1)}× (vs ${(r.batch_bg / r.batch_noBg).toFixed(1)}× when batched)\n`); diff --git a/src/hex-map/HexMapView.ts b/src/hex-map/HexMapView.ts index 7cf5292..96211d7 100644 --- a/src/hex-map/HexMapView.ts +++ b/src/hex-map/HexMapView.ts @@ -4462,14 +4462,17 @@ export class HexMapView extends ItemView { ?.remove(); const gw = gridContainer.offsetWidth || 1; const gh = gridContainer.offsetHeight || 1; - const layer = gridContainer.createDiv({ - cls: "duckmage-coord-labels-layer", - }); + + // CRITICAL: read every hex's offset geometry FIRST, into a plain array, + // before creating any label DOM. Interleaving offset reads with DOM writes + // forces a full synchronous layout per hex — O(n²) layout thrash that + // stalled opening large maps for ~20s at chult's 3843 hexes (see + // dev/coord-label-thrash-bench). With reads batched ahead of writes the + // browser flushes layout once, dropping this to ~100ms. + const placements: { x: number; y: number; ox: number; oy: number }[] = []; gridContainer .querySelectorAll(".duckmage-hex") .forEach((hexEl) => { - const x = Number(hexEl.dataset.x); - const y = Number(hexEl.dataset.y); // Hex center in gridContainer-local pixel coords (unaffected by // CSS transform; offsetParent walks stop at gridContainer). let ox = hexEl.offsetWidth / 2; @@ -4480,15 +4483,28 @@ export class HexMapView extends ItemView { oy += cur.offsetTop; cur = cur.offsetParent as HTMLElement | null; } - const label = layer.createDiv({ - cls: "duckmage-coord-label-html", - text: `${x},${y}`, - }); - label.setCssProps({ - "--duckmage-coord-x": `${((ox / gw) * 100).toFixed(3)}%`, - "--duckmage-coord-y": `${((oy / gh) * 100).toFixed(3)}%`, + placements.push({ + x: Number(hexEl.dataset.x), + y: Number(hexEl.dataset.y), + ox, + oy, }); }); + + // Write phase: no offset reads here, so layout stays clean throughout. + const layer = gridContainer.createDiv({ + cls: "duckmage-coord-labels-layer", + }); + for (const { x, y, ox, oy } of placements) { + const label = layer.createDiv({ + cls: "duckmage-coord-label-html", + text: `${x},${y}`, + }); + label.setCssProps({ + "--duckmage-coord-x": `${((ox / gw) * 100).toFixed(3)}%`, + "--duckmage-coord-y": `${((oy / gh) * 100).toFixed(3)}%`, + }); + } } private updatePathOverlay(): void {