From 933a54ead98ba9196255b6c12cb46fde1c08f761 Mon Sep 17 00:00:00 2001 From: Thomas Camlong Date: Sun, 9 Aug 2026 15:39:41 +0200 Subject: [PATCH] docs(perf): consolidated before/after, plus what is still unverified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All previous numbers were spread across rounds with different instruments and different workloads. This is one instrument, one workload, alternated, 2 runs per side, merge-base vs HEAD: server idle 141.9 -> 89.7 MiB (-37%) server loaded 257.3 -> 173.0 MiB (-33%) server peak 705.2 -> 568.4 MiB (-19%) after idle soak 332.0 -> 232.5 MiB (-30%) JS event listeners 10,126 -> 7,774 (-23%) React fibers 5,224 -> 3,588 (-31%) DOM nodes 9,175 -> 8,284 (-10%) browser JS heap 55.2 -> 51.1 MiB (-7%) image 410 -> 398 MB (-3%) container CPU 22.8 -> 22.8 s unchanged Recorded prominently rather than buried: under *sustained* 4-tab load the two builds converge, 368 vs 364 MiB. The wins are idle, steady state and peak. That is why hammering the app with five tabs did not feel 30% lighter — at saturation the footprint is live widget data and render churn, which this PR does not change. Also documents the gaps, since "tested" without them would be misleading: no multi-hour soak (the #5301 reports are about days), single user, SQLite only, the DevTools Profiler tab confirmed at bundle level rather than in the extension, and the App Router re-rendering ~1x/second still unfixed. verify-context-menu.mts now also covers touch long-press, and no longer asserts on a fiber total — that counter swung 7,454 to 16,149 between runs of the same build, so it was noise. probe-fiber-tree.mts is the stable counter and is what the -31% above comes from. --- PHASE2-FINDINGS.md | 82 ++++++++++++++++++++++ scripts/benchmarks/verify-context-menu.mts | 32 +++++++-- 2 files changed, 110 insertions(+), 4 deletions(-) diff --git a/PHASE2-FINDINGS.md b/PHASE2-FINDINGS.md index 9e549fab1..0551a8486 100644 --- a/PHASE2-FINDINGS.md +++ b/PHASE2-FINDINGS.md @@ -8,6 +8,66 @@ onboarding restore UI, signed in, then the busiest board (28 items / 16 widget k Images built locally from known commits so the comparison is not against a stale registry tag: baseline = merge-base `06724cfeb`, branch = `abec79abe`. +## Before / after + +One instrument, one workload, alternated runs (2 before, 2 after) on the same restored +backup: **4 tabs across 2 boards**, 3 reloads each, widget interaction, 2 min idle soak. +`before` = merge-base `06724cfeb`, `after` = this branch. Server figures are cgroup +anonymous memory; `memory.current` is not used because page cache swings it by 60+ MiB. + +### Server memory + +| stage | before | after | change | +| --- | --- | --- | --- | +| boot idle | 141.9 MiB | **89.7 MiB** | **−37%** | +| after restore | 143.2 MiB | **98.8 MiB** | **−31%** | +| after sign-in | 248.7 MiB | **158.5 MiB** | **−36%** | +| dashboard loaded | 257.3 MiB | **173.0 MiB** | **−33%** | +| after 3 reloads × 4 tabs | 368.4 MiB | 364.3 MiB | **−1%** | +| after 2 min idle soak | 332.0 MiB | **232.5 MiB** | **−30%** | +| peak | 705.2 MiB | **568.4 MiB** | **−19%** | + +**The honest caveat, and it matters:** under *sustained* multi-tab load the two builds +converge — 368 vs 364 MiB. The wins are at idle, at steady state, and at peak. That is +why hammering the app with five tabs did not feel 30% lighter: at that point the +footprint is dominated by live widget data and render churn, which this PR does not +change. Idle, first load and worst-case are materially better; saturation is not. + +### Browser + +| metric | before | after | change | +| --- | --- | --- | --- | +| JS event listeners | 10,126 | **7,774** | **−23%** | +| DOM nodes | 9,175 | **8,284** | **−10%** | +| React fibers (idle board) | 5,224 | **3,588** | **−31%** | +| browser JS heap | 55.2 MiB | **51.1 MiB** | **−7%** | + +### Cost and neutral metrics + +| metric | before | after | change | +| --- | --- | --- | --- | +| container CPU over the run | 22.8 s | 22.8 s | unchanged | +| median board load | 710 ms | 506 ms | faster, but ±200 ms run to run | +| image size | 410 MB | **398 MB** | −3% | + +CPU is unchanged at this workload. An isolated measurement of the V8 young-generation +bound showed ~8% more CPU on a lighter single-tab run; it does not show up here. + +### What produced each win + +Measured in isolation, cumulatively from the merge-base image: + +| change | size | effect | +| --- | --- | --- | +| `--max-semi-space-size=4` | **1 line** | −19% idle, −21% peak server | +| `serverExternalPackages` + `mysql2`/`pg` | **1 line** | −21 MiB idle server | +| `serverExternalPackages` + 8 more server-only deps | 1 block | −18 MiB loaded server | +| one shared board context menu | 3 files | **−23% listeners, −31% fibers** | +| integrations barrel diet | ~10 files | −40 MiB under load, 0 at idle | +| jsdom → linkedom | 3 files | image size + XSS hardening | +| V8 heap cap | 1 line | 0 — a drift ceiling, not a reduction | +| cache-invalidation layer + `next-cache-handler` | ~30 files | **0 — deleted** | + ## Metric choice `memory.current` moved between **192 and 261 MiB for the same baseline image** across runs, @@ -399,3 +459,25 @@ node --experimental-strip-types scripts/benchmarks/stress-restore.mts The backup is read from that path and never copied into the repo — it contains real integration secrets and the instance's encryption key. + +## What has not been verified + +Stating these plainly, because an unqualified "tested" would be wrong: + +- **No long-duration soak.** The reports behind #5301 are about growth over *days*; the + longest run here is 2 minutes of idle. Nothing here demonstrates the multi-GB drift is + fixed — the V8 heap cap bounds it by construction, but that is an argument, not a + measurement. +- **Single user, single browser.** No concurrent users, no multi-instance + (`REDIS_IS_EXTERNAL=true`) deployment, no non-SQLite database. The DB-driver + unbundling in particular is only exercised on the SQLite path. +- **The React DevTools Profiler tab itself.** The profiling build is verified at the + bundle level; a synthetic devtools hook cannot confirm the tab, so that last step needs + the real extension. +- **The dominant client cost is unfixed.** The App Router root re-renders ~1×/second and + that drives most of the churn. Everything shipped here is downstream of it. +- **Only the board path is covered end-to-end.** Manage pages, onboarding and integration + flows are covered by CI's E2E suite, not by these benchmarks. +- **Two tests are flaky in CI** (`pi-hole`, backup ZIP) and both pass in isolation + locally. They are pre-existing and unrelated to these changes, but they mean a green + run is not automatic. diff --git a/scripts/benchmarks/verify-context-menu.mts b/scripts/benchmarks/verify-context-menu.mts index e8f0fad1e..8410e8c91 100644 --- a/scripts/benchmarks/verify-context-menu.mts +++ b/scripts/benchmarks/verify-context-menu.mts @@ -131,6 +131,30 @@ const closed = await dropdown .catch(() => false); check(closed, "Escape closes the menu"); +// Touch long-press: the shared menu replaced Mantine's Menu.ContextMenu, which +// handled this natively, so it has to be re-proven rather than assumed. +const longPressItem = (index: number) => + page.evaluate((itemIndex) => { + const items = [...document.querySelectorAll('[data-homarr-dev-benchmark-board] [data-type="item"]')]; + const item = items[itemIndex]; + if (!item) return false; + const rect = item.getBoundingClientRect(); + const target = (item.querySelector("*") as HTMLElement | null) ?? (item as HTMLElement); + const x = rect.x + rect.width / 2; + const y = rect.y + rect.height / 2; + const touch = new Touch({ identifier: 1, target, clientX: x, clientY: y }); + target.dispatchEvent(new TouchEvent("touchstart", { bubbles: true, cancelable: true, touches: [touch] })); + return true; + }, index); + +await longPressItem(2 % Math.max(itemCount, 1)); +// Delay is 500ms in the component; allow margin. +await page.waitForTimeout(1_200); +const openedByTouch = await dropdown.isVisible().catch(() => false); +check(openedByTouch, "touch long-press opens the context menu"); +await page.keyboard.press("Escape").catch(() => undefined); +await dropdown.waitFor({ state: "hidden", timeout: 5_000 }).catch(() => undefined); + // A shared menu has to re-target; a per-item menu would trivially pass the first case. if (itemCount > 1) { await rightClickItem(1); @@ -143,11 +167,11 @@ if (itemCount > 1) { await dropdown.waitFor({ state: "hidden", timeout: 5_000 }).catch(() => undefined); } +// Fiber totals are reported for context only, never asserted on: this counter walks +// every root the devtools hook has seen, and portals opened by the menu make it swing +// by 2x between runs of the same build. Use probe-fiber-tree.mts for a stable count. const afterFibers = await countFibers(page); -console.log( - `\nfibers after opening/closing menus: ${afterFibers} (delta ${afterFibers - idleFibers >= 0 ? "+" : ""}${afterFibers - idleFibers})`, -); -check(afterFibers < idleFibers * 1.5, "tree did not balloon after using the menu"); +console.log(`\nfibers (context only, not asserted): idle ${idleFibers} -> ${afterFibers} after using the menu`); console.log(`\n${failures.length === 0 ? "ALL CHECKS PASSED" : `${failures.length} CHECK(S) FAILED`}`); console.log(JSON.stringify({ itemCount, idleFibers, afterFibers, failures }, null, 2));