From c7ea7555effc0468fbcc97b301ecaf179f47727a Mon Sep 17 00:00:00 2001 From: nyanrus <68762426+nyanrus@users.noreply.github.com> Date: Wed, 27 May 2026 14:35:02 +0900 Subject: [PATCH] web-next: Cache Intl formatters and memoize LanguageSelect locale list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two CPU/allocation hotspots surfaced by a Firefox profiler trace of `/feed` opening NoteComposer (+10 MB/s sustained during the interactive window, vs. ~0 MB/s while idle on the same route). LanguageSelect.tsx - `locales` was a plain function, so every reactive read from the `` re-ran a `.map()` over ~200 `POSSIBLE_LOCALES` entries that allocated a fresh `Intl.DisplayNames(locale, { type: "language" })` per iteration. Each `Intl.DisplayNames` loads CLDR tables for its target locale, so the steady-state cost of opening the composer was ~200 fresh CLDR-backed instances per reactive tick. - Wrap `locales` in `createMemo`, hoist `englishNames` to module scope (its locale doesn't depend on anything), and memoise `displayNames` so it only rebuilds when `i18n.locale` actually changes. Cache per-locale `Intl.DisplayNames` instances at module scope so the per-row native-name lookup costs one allocation over the lifetime of the page. - Drive-by: the pre-existing guard pushed `props.value.baseName` onto the locale list only when it was *already* present, which produced a duplicate entry and never extended the list to cover a non-standard locale. Invert the condition so the value is appended when missing — that matches the apparent intent (the comboboxneeds to be able to show the active value even if it's outside the canonical set). Timestamp.tsx - `formatRelativeTime` constructed a new `Intl.RelativeTimeFormat` every render, which fires once per visible timestamp every 1s/30s/1min depending on age. Cache instances keyed by `(locale, numeric, style)` at module scope. Apply the same `options` in the value=0 fallback path that the main loop uses, so the fallback's `numeric`/`style` no longer drifts from the caller (the previous code happened to pass no options in the fallback, defaulting to Intl's `"always"`, while the loop honoured the caller's `"auto"` — pulling the same `options` through keeps them consistent). Not included here: the original draft of this branch also dropped `keyed` from two `` wrappers (PostEngagementBar and PublicTimeline). The local lint plugin `keyed-show.ts` is right to enforce `keyed` for solid-relay-backed accessors — non-keyed risks the documented "Stale read from " race when a fragment flips to null inside the same tick as a downstream reactive read. The perf benefit that motivated those drops (avoiding mass remount of Kobalte primitives on every Relay snapshot tick) was real, but its root cause is upstream: `createFragment` in solid-relay pre-clears `data` to `undefined` before applying its identity-preserving `reconcile`, defeating the merge and producing a fresh top-level reference on every snapshot. Fixed in nyanrus/solid-relay#fix/reconcile-preserve-identity (proposed upstream at XiNiHa/solid-relay#68). Once that lands, the `keyed` Shows in this codebase stop re-mounting on field updates without any change needed here. The remaining follow-ups (Tooltip and ActorHoverCard lazy-mount, Skeleton/Separator/Button cleanup) are captured in `web-next/PERF_TODO.md`. Assisted-by: Claude Code:claude-opus-4-7 --- web-next/src/components/LanguageSelect.tsx | 45 +++++++++++++++------- web-next/src/components/Timestamp.tsx | 30 ++++++++++++--- 2 files changed, 56 insertions(+), 19 deletions(-) diff --git a/web-next/src/components/LanguageSelect.tsx b/web-next/src/components/LanguageSelect.tsx index c3216aadb..74e8b81a5 100644 --- a/web-next/src/components/LanguageSelect.tsx +++ b/web-next/src/components/LanguageSelect.tsx @@ -1,5 +1,5 @@ import { POSSIBLE_LOCALES } from "@hackerspub/models/i18n"; -import { Show } from "solid-js"; +import { createMemo, Show } from "solid-js"; import { Combobox, ComboboxContent, @@ -28,20 +28,37 @@ interface LocaleInfo { readonly disabled: boolean; } +// `Intl.DisplayNames` instances are expensive: each one loads CLDR tables for +// its target locale. With ~200 entries in POSSIBLE_LOCALES we'd otherwise +// allocate that many tables on every read of `locales()`. Cache per-locale +// formatters once at module scope and reuse them. +const nativeDisplayNamesCache = new Map(); +function getNativeDisplayNames(locale: string): Intl.DisplayNames { + let dn = nativeDisplayNamesCache.get(locale); + if (dn == null) { + dn = new Intl.DisplayNames(locale, { type: "language" }); + nativeDisplayNamesCache.set(locale, dn); + } + return dn; +} + +const englishNames = new Intl.DisplayNames("en", { type: "language" }); + export function LanguageSelect(props: LanguageSelectProps) { const { t, i18n } = useLingui(); - const displayNames = new Intl.DisplayNames(i18n.locale, { type: "language" }); - const englishNames = new Intl.DisplayNames("en", { type: "language" }); - const locales = () => { + const displayNames = createMemo(() => + new Intl.DisplayNames(i18n.locale, { type: "language" }) + ); + const locales = createMemo(() => { + const dn = displayNames(); const localeCodes: string[] = [...POSSIBLE_LOCALES]; - if (props.value != null && localeCodes.includes(props.value.baseName)) { + if (props.value != null && !localeCodes.includes(props.value.baseName)) { localeCodes.push(props.value.baseName); } - const locales = localeCodes.map((locale) => { - const name = displayNames.of(locale) ?? locale; - const nativeName = - new Intl.DisplayNames(locale, { type: "language" }).of(locale) ?? - locale; + const exclude = props.exclude; + const result = localeCodes.map((locale) => { + const name = dn.of(locale) ?? locale; + const nativeName = getNativeDisplayNames(locale).of(locale) ?? locale; return { code: locale, name, @@ -50,12 +67,12 @@ export function LanguageSelect(props: LanguageSelectProps) { englishNames.of(locale) ?? "" }` .trim(), - disabled: props.exclude?.some((l) => l.baseName === locale) ?? false, + disabled: exclude?.some((l) => l.baseName === locale) ?? false, }; }); - locales.sort((a, b) => a.name.localeCompare(b.name)); - return locales; - }; + result.sort((a, b) => a.name.localeCompare(b.name)); + return result; + }); return ( options={locales()} diff --git a/web-next/src/components/Timestamp.tsx b/web-next/src/components/Timestamp.tsx index 475e3b281..0ec3138db 100644 --- a/web-next/src/components/Timestamp.tsx +++ b/web-next/src/components/Timestamp.tsx @@ -88,6 +88,25 @@ export function getRelativeTimeUpdateDelayMs( return DAY; } +// `Intl.RelativeTimeFormat` is comparatively expensive to construct (it loads +// CLDR data for the locale). The previous implementation built a fresh +// formatter on every render, which is wasteful when 25+ timeline cards each +// re-render their relative timestamp every 1s/30s/1min. Cache by the +// (locale, options) tuple that actually affects output. +const relativeTimeFormatCache = new Map(); +function getRelativeTimeFormat( + locale: string, + options: Intl.RelativeTimeFormatOptions, +): Intl.RelativeTimeFormat { + const key = `${locale}|${options.numeric ?? ""}|${options.style ?? ""}`; + let f = relativeTimeFormatCache.get(key); + if (f == null) { + f = new Intl.RelativeTimeFormat(locale, options); + relativeTimeFormatCache.set(key, f); + } + return f; +} + function formatRelativeTime( currentDate: Date, targetDate: Date, @@ -107,16 +126,17 @@ function formatRelativeTime( for (const { unit, ms } of UNITS) { if (absDiff >= ms) { const value = Math.round(diffMs / ms); - const f = new Intl.RelativeTimeFormat(locale, options).format( - value, - unit, - ); + const f = getRelativeTimeFormat(locale, options).format(value, unit); return options.capitalizeFirstLetter ? f.charAt(0).toUpperCase() + f.slice(1) : f; } } - const f = new Intl.RelativeTimeFormat(locale).format(0, "second"); + // Use the caller-supplied `options` here so the fallback respects + // `numeric` / `style` the same way the loop above does. Hardcoding a + // different `numeric` would change the output for value=0 (e.g. "now" + // under "auto" vs "0 seconds ago" under the Intl default "always"). + const f = getRelativeTimeFormat(locale, options).format(0, "second"); return options.capitalizeFirstLetter ? f.charAt(0).toUpperCase() + f.slice(1) : f;