diff --git a/packages/widgets/src/beszel-system-table/component.tsx b/packages/widgets/src/beszel-system-table/component.tsx index 8286335a6..33d277a82 100644 --- a/packages/widgets/src/beszel-system-table/component.tsx +++ b/packages/widgets/src/beszel-system-table/component.tsx @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useEffect, useMemo, useState } from "react"; +import { memo, useCallback, useEffect, useMemo, useState } from "react"; import { Center, Group, Indicator, Loader, Progress, Text } from "@mantine/core"; import type { DataTableColumn, DataTableSortStatus } from "mantine-datatable"; import { @@ -42,6 +42,34 @@ import { BeszelIntegrationErrorIndicator } from "../beszel/_shared/error-indicat import { BeszelSystemStatsModal } from "../beszel/_shared/system-stats-modal"; import { DiskUsage } from "../beszel/_shared/disk-usage"; +/** + * One cpu/memory/gpu cell. + * + * This lived inside the widget body, which made it a **new component type on every render**: React + * cannot match a new type to the previous one, so it unmounted and remounted every cell's subtree + * (Group, Text and Progress) instead of updating it. This widget re-renders about once a second from + * its live Beszel subscription, so that was three cells per system torn down and rebuilt every + * second. Declaring it here makes the type stable, and memo keeps a cell from re-rendering when its + * own value has not changed. + * + * `size` is passed rather than closed over — it was the only thing the nested version captured. + */ +const PercentCell = memo(function PercentCell({ value, size }: { value: number; size: SizeConfig }) { + return ( + + + {formatPercent(value)} + + + + ); +}); + const directionMultiplier: Record = { asc: 1, desc: -1 }; type SystemRowWithKey = BeszelSystemRow & { _key: string }; @@ -149,20 +177,6 @@ export default function BeszelSystemTableWidget({ }); }, [filteredSystems, sortStatus]); - const PercentCell = ({ value }: { value: number }) => ( - - - {formatPercent(value)} - - - - ); - const columns = useMemo((): DataTableColumn[] => { const cols: (DataTableColumn | false)[] = [ { @@ -195,7 +209,7 @@ export default function BeszelSystemTableWidget({ ), sortable: true, - render: (record) => , + render: (record) => , }, options.showMemory && { accessor: "memory", @@ -207,7 +221,7 @@ export default function BeszelSystemTableWidget({ ), sortable: true, - render: (record) => , + render: (record) => , }, options.showDisk && { accessor: "disk", @@ -239,7 +253,7 @@ export default function BeszelSystemTableWidget({ ), sortable: true, - render: (record) => , + render: (record) => , }, options.showLoadAvg && { accessor: "loadAvg", diff --git a/scripts/benchmarks/audit-widget-render.mjs b/scripts/benchmarks/audit-widget-render.mjs new file mode 100644 index 000000000..5534d0025 --- /dev/null +++ b/scripts/benchmarks/audit-widget-render.mjs @@ -0,0 +1,121 @@ +/** + * Static audit of every widget for render-cost anti-patterns. + * + * Measuring widgets one at a time found real problems — a Popover mounted per card, 80 cards + * rendered into a tile showing 7 — but it needs a board that happens to use the widget and data that + * happens to be large. This sweeps the source instead, so a widget nobody has on a test board is + * still covered. + * + * Every finding is a *candidate*, not a defect: the patterns below cost something when a list is long + * and nothing when it has three rows. Sorted so the ones inside a `.map()` come first, since those + * multiply by row count. + * + * node scripts/benchmarks/audit-widget-render.mjs + */ +import { readdirSync, readFileSync, statSync } from "node:fs"; +import path from "node:path"; + +const root = "packages/widgets/src"; + +const walk = (dir) => + readdirSync(dir).flatMap((entry) => { + const full = path.join(dir, entry); + return statSync(full).isDirectory() ? walk(full) : [full]; + }); + +/** Finds the body of each `.map(` callback by tracking bracket depth from the opening paren. */ +const mapBodies = (source) => { + const bodies = []; + const pattern = /\.map\(/g; + let match; + while ((match = pattern.exec(source)) !== null) { + let depth = 0; + let index = match.index + match[0].length - 1; + const start = index; + for (; index < source.length; index++) { + const char = source[index]; + if (char === "(") depth++; + else if (char === ")") { + depth--; + if (depth === 0) break; + } + } + if (index > start) bodies.push({ start, end: index, text: source.slice(start, index) }); + } + return bodies; +}; + +const lineOf = (source, offset) => source.slice(0, offset).split("\n").length; + +const findings = []; +for (const file of walk(root)) { + if (!/\.tsx$/.test(file) || /\.spec\./.test(file)) continue; + const source = readFileSync(file, "utf8"); + const bodies = mapBodies(source); + + for (const body of bodies) { + const line = lineOf(source, body.start); + + // Mantine mounts a Popover per Tooltip/Menu even when disabled, so one per row is one per row. + const overlay = [...body.text.matchAll(/<(Tooltip|TooltipFloating|Popover|Menu|HoverCard)\b/g)].map((m) => m[1]); + if (overlay.length > 0) { + findings.push({ file, line, inMap: true, kind: "overlay-per-row", detail: [...new Set(overlay)].join(", ") }); + } + + // A fresh object literal per row defeats any memoisation below it and re-triggers style diffing. + const inlineStyles = (body.text.match(/style=\{\{/g) ?? []).length; + if (inlineStyles > 0) { + findings.push({ file, line, inMap: true, kind: "inline-style-per-row", detail: `${inlineStyles} literal(s)` }); + } + + // An index key makes React reuse the wrong row when the list reorders, remounting subtrees. + if (/key=\{(\s*)(index|i|idx|\w*Index)\s*\}/.test(body.text) || /key=\{`[^`]*\$\{(index|i|idx|\w*Index)\}/.test(body.text)) { + findings.push({ file, line, inMap: true, kind: "index-key", detail: "key derived from array index" }); + } + + // An arrow prop is a new function per row per render; harmless alone, costly under a memo. + const inlineHandlers = (body.text.match(/on[A-Z]\w+=\{\(\)\s*=>/g) ?? []).length; + if (inlineHandlers > 2) { + findings.push({ file, line, inMap: true, kind: "inline-handlers-per-row", detail: `${inlineHandlers} handlers` }); + } + } + + // A component declared inside another component is a new type every render, so React unmounts and + // remounts its whole subtree rather than updating it. + for (const match of source.matchAll(/^(\s+)const\s+([A-Z]\w+)\s*=\s*\(?[^=]*\)?\s*=>\s*[({]/gm)) { + if (match[1].length >= 2) { + findings.push({ + file, + line: lineOf(source, match.index), + inMap: false, + kind: "nested-component", + detail: match[2], + }); + } + } +} + +const byKind = new Map(); +for (const finding of findings) { + const key = `${finding.kind}`; + byKind.set(key, (byKind.get(key) ?? 0) + 1); +} + +console.log("=== findings by kind ==="); +for (const [kind, count] of [...byKind.entries()].sort((a, b) => b[1] - a[1])) { + console.log(` ${String(count).padStart(4)} ${kind}`); +} + +const inMap = findings.filter((finding) => finding.inMap); +const byFile = new Map(); +for (const finding of inMap) { + const list = byFile.get(finding.file) ?? []; + list.push(finding); + byFile.set(finding.file, list); +} + +console.log(`\n=== inside a .map(), so multiplied by row count — worst files first ===`); +for (const [file, list] of [...byFile.entries()].sort((a, b) => b[1].length - a[1].length).slice(0, 18)) { + console.log(`\n ${file.replace("packages/widgets/src/", "")} (${list.length})`); + for (const finding of list) console.log(` :${finding.line} ${finding.kind} — ${finding.detail}`); +}