From 9aa141ecd5be03314baf0b9a3c0748efb4e739f8 Mon Sep 17 00:00:00 2001 From: Oseltamivir <58582368+Oseltamivir@users.noreply.github.com> Date: Thu, 11 Jun 2026 21:19:38 -0700 Subject: [PATCH 1/2] perf(charts): eliminate redundant D3 rebuilds at mount Profiling the live site showed every chart runs 2-3 full (visually identical) D3 teardown/rebuild passes right after mount, each ~300ms of main-thread time. Two unstable identities cause this: - useThemeColors re-set themeColors via setTimeout(0) on mount even though the synchronous useState read already saw the correct computed styles (next-themes applies the theme class pre-hydration). The new object identity invalidates getCssColor -> layers -> full rebuild. Now only an actual resolvedTheme change re-reads colors; the setTimeout(0) is kept for real switches so the class flip settles first. - useResponsiveChartDimensions produced a new dimensions object from the ResizeObserver's initial callback (same width the ref callback just measured) and from same-value height updates. Same-size updates now keep the previous object so React bails out. Both fixes are identity-only: real theme switches and real resizes behave exactly as before. --- .../useResponsiveChartDimensions.test.ts | 180 ++++++++++++++++++ .../src/hooks/useResponsiveChartDimensions.ts | 19 +- packages/app/src/hooks/useThemeColors.test.ts | 173 +++++++++++++++++ packages/app/src/hooks/useThemeColors.ts | 23 ++- 4 files changed, 389 insertions(+), 6 deletions(-) create mode 100644 packages/app/src/hooks/useResponsiveChartDimensions.test.ts create mode 100644 packages/app/src/hooks/useThemeColors.test.ts diff --git a/packages/app/src/hooks/useResponsiveChartDimensions.test.ts b/packages/app/src/hooks/useResponsiveChartDimensions.test.ts new file mode 100644 index 00000000..5f205bbe --- /dev/null +++ b/packages/app/src/hooks/useResponsiveChartDimensions.test.ts @@ -0,0 +1,180 @@ +// @vitest-environment jsdom +import { act, createElement } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { + useResponsiveChartDimensions, + type UseResponsiveChartDimensionsResult, +} from './useResponsiveChartDimensions'; + +// Minimal ResizeObserver stand-in — jsdom doesn't implement it. Tests fire +// observations manually via `trigger`. +class MockResizeObserver { + static instances: MockResizeObserver[] = []; + callback: ResizeObserverCallback; + observed: Element[] = []; + disconnected = false; + + constructor(callback: ResizeObserverCallback) { + this.callback = callback; + MockResizeObserver.instances.push(this); + } + + observe(el: Element) { + this.observed.push(el); + } + + disconnect() { + this.disconnected = true; + } + + unobserve() {} + + trigger(width: number) { + act(() => { + this.callback( + [{ contentRect: { width } } as unknown as ResizeObserverEntry], + this as unknown as ResizeObserver, + ); + }); + } +} + +function renderHook(hook: () => T): { + result: { current: T }; + rerender: () => void; + unmount: () => void; + root: Root; +} { + const result = { current: undefined as unknown as T }; + function TestComponent() { + result.current = hook(); + return null; + } + const container = document.createElement('div'); + document.body.append(container); + const root = createRoot(container); + const render = () => { + act(() => { + root.render(createElement(TestComponent)); + }); + }; + render(); + return { + result, + rerender: render, + unmount: () => { + act(() => root.unmount()); + container.remove(); + }, + root, + }; +} + +/** Create a container div whose getBoundingClientRect reports `width`. */ +function makeContainer(width: number): HTMLDivElement { + const el = document.createElement('div'); + vi.spyOn(el, 'getBoundingClientRect').mockReturnValue({ width } as DOMRect); + return el; +} + +beforeEach(() => { + MockResizeObserver.instances = []; + vi.stubGlobal('ResizeObserver', MockResizeObserver); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe('useResponsiveChartDimensions', () => { + it('measures the container on attach', () => { + const { result, unmount } = renderHook(() => + useResponsiveChartDimensions({ height: 600 }), + ); + + act(() => { + result.current.setContainerRef(makeContainer(800)); + }); + + expect(result.current.dimensions).toEqual({ width: 800, height: 600 }); + unmount(); + }); + + it('keeps the dimensions object identity when an observation reports the same size', () => { + const { result, unmount } = renderHook(() => + useResponsiveChartDimensions({ height: 600 }), + ); + + act(() => { + result.current.setContainerRef(makeContainer(800)); + }); + const initial = result.current.dimensions; + + // ResizeObserver fires once right after observe() with the width the ref + // callback already measured. The object identity must not change — a new + // identity makes every chart treat it as a resize and fully rebuild. + MockResizeObserver.instances.at(-1)!.trigger(800); + + expect(result.current.dimensions).toBe(initial); + unmount(); + }); + + it('updates dimensions when an observation reports a new width', () => { + const { result, unmount } = renderHook(() => + useResponsiveChartDimensions({ height: 600 }), + ); + + act(() => { + result.current.setContainerRef(makeContainer(800)); + }); + const initial = result.current.dimensions; + + MockResizeObserver.instances.at(-1)!.trigger(1024); + + expect(result.current.dimensions).not.toBe(initial); + expect(result.current.dimensions).toEqual({ width: 1024, height: 600 }); + unmount(); + }); + + it('disconnects the previous observer when the container changes', () => { + const { result, unmount } = renderHook(() => + useResponsiveChartDimensions({ height: 600 }), + ); + + act(() => { + result.current.setContainerRef(makeContainer(800)); + }); + const first = MockResizeObserver.instances.at(-1)!; + + act(() => { + result.current.setContainerRef(makeContainer(640)); + }); + + expect(first.disconnected).toBe(true); + expect(result.current.dimensions).toEqual({ width: 640, height: 600 }); + unmount(); + }); + + it('detaches cleanly when the container is removed', () => { + const { result, unmount } = renderHook(() => + useResponsiveChartDimensions({ height: 600 }), + ); + + act(() => { + result.current.setContainerRef(makeContainer(800)); + }); + const observer = MockResizeObserver.instances.at(-1)!; + + act(() => { + result.current.setContainerRef(null); + }); + + expect(observer.disconnected).toBe(true); + // Last measured dimensions are retained (no reset to 0 on detach). + expect(result.current.dimensions).toEqual({ width: 800, height: 600 }); + unmount(); + }); +}); diff --git a/packages/app/src/hooks/useResponsiveChartDimensions.ts b/packages/app/src/hooks/useResponsiveChartDimensions.ts index a1200ec6..493200dc 100644 --- a/packages/app/src/hooks/useResponsiveChartDimensions.ts +++ b/packages/app/src/hooks/useResponsiveChartDimensions.ts @@ -33,6 +33,17 @@ export function useResponsiveChartDimensions( const [dimensions, setDimensions] = useState({ width: 0, height }); const resizeObserverRef = useRef(null); + // Keep the dimensions object referentially stable when nothing changed. + // ResizeObserver fires once right after observe() with the same width the + // ref callback just measured — without this guard that initial callback + // produces a new object identity, which downstream chart effects treat as + // a resize and answer with a full (visually identical) D3 rebuild. + const updateDimensions = useCallback((width: number, h: number) => { + setDimensions((prev) => + prev.width === width && prev.height === h ? prev : { width, height: h }, + ); + }, []); + // ref callback for initial dimension calculation and ResizeObserver setup const setContainerRef = useCallback( (element: HTMLDivElement | null) => { @@ -47,20 +58,20 @@ export function useResponsiveChartDimensions( if (element) { // set initial dimensions const initialWidth = element.getBoundingClientRect().width; - setDimensions({ width: initialWidth, height }); + updateDimensions(initialWidth, height); // set up ResizeObserver resizeObserverRef.current = new ResizeObserver((entries) => { if (entries[0]) { const { width: observedWidth } = entries[0].contentRect; - setDimensions({ width: observedWidth, height }); + updateDimensions(observedWidth, height); } }); resizeObserverRef.current.observe(element); } }, - [height], + [height, updateDimensions], ); // clean up on unmount or height change @@ -75,7 +86,7 @@ export function useResponsiveChartDimensions( // update dimensions when height changes useEffect(() => { - setDimensions((prev) => ({ ...prev, height })); + setDimensions((prev) => (prev.height === height ? prev : { ...prev, height })); }, [height]); return { diff --git a/packages/app/src/hooks/useThemeColors.test.ts b/packages/app/src/hooks/useThemeColors.test.ts new file mode 100644 index 00000000..7275e384 --- /dev/null +++ b/packages/app/src/hooks/useThemeColors.test.ts @@ -0,0 +1,173 @@ +// @vitest-environment jsdom +import { act, createElement } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Controllable next-themes mock — tests flip resolvedTheme and re-render. +const themeState = vi.hoisted(() => ({ resolvedTheme: 'dark' as string | undefined })); +vi.mock('next-themes', () => ({ + useTheme: () => ({ resolvedTheme: themeState.resolvedTheme }), +})); + +import { useThemeColors, type UseThemeColorsResult } from './useThemeColors'; + +// Lightweight renderHook — TLR isn't installed, so we mount a 1-component root +// and capture the latest hook return value in a ref-style object. +function renderHook(hook: () => T): { + result: { current: T }; + rerender: () => void; + unmount: () => void; + root: Root; +} { + const result = { current: undefined as unknown as T }; + function TestComponent() { + result.current = hook(); + return null; + } + const container = document.createElement('div'); + document.body.append(container); + const root = createRoot(container); + const render = () => { + act(() => { + root.render(createElement(TestComponent)); + }); + }; + render(); + return { + result, + rerender: render, + unmount: () => { + act(() => root.unmount()); + container.remove(); + }, + root, + }; +} + +const HW_KEYS = ['h100', 'mi300x', 'b200']; + +beforeEach(() => { + vi.useFakeTimers(); + themeState.resolvedTheme = 'dark'; +}); + +afterEach(() => { + vi.runOnlyPendingTimers(); + vi.useRealTimers(); +}); + +describe('useThemeColors mount behavior', () => { + it('keeps themeColors and getCssColor referentially stable across mount effects', () => { + const { result, rerender, unmount } = renderHook(() => + useThemeColors({ highContrast: false, activeKeys: HW_KEYS }), + ); + + const initialThemeColors = result.current.themeColors; + const initialGetCssColor = result.current.getCssColor; + + // Flush the post-mount timer window where the old implementation re-set + // themeColors (new object identity → full D3 rebuild in every chart). + act(() => { + vi.runAllTimers(); + }); + rerender(); + + expect(result.current.themeColors).toBe(initialThemeColors); + expect(result.current.getCssColor).toBe(initialGetCssColor); + unmount(); + }); + + it('stays stable through the next-themes hydration sequence (undefined → defined)', () => { + themeState.resolvedTheme = undefined; + const { result, rerender, unmount } = renderHook(() => + useThemeColors({ highContrast: false, activeKeys: HW_KEYS }), + ); + const initialThemeColors = result.current.themeColors; + + // next-themes mounts and reports the theme that the pre-hydration inline + // script already applied to — no visual change, so no update. + themeState.resolvedTheme = 'dark'; + rerender(); + act(() => { + vi.runAllTimers(); + }); + rerender(); + + expect(result.current.themeColors).toBe(initialThemeColors); + unmount(); + }); + + it('re-reads theme colors when the resolved theme actually changes', () => { + const { result, rerender, unmount } = renderHook(() => + useThemeColors({ highContrast: false, activeKeys: HW_KEYS }), + ); + const initialThemeColors = result.current.themeColors; + + themeState.resolvedTheme = 'light'; + rerender(); + act(() => { + vi.runAllTimers(); + }); + rerender(); + + expect(result.current.themeColors).not.toBe(initialThemeColors); + unmount(); + }); + + it('does not re-read when re-rendered with the same theme after a switch', () => { + const { result, rerender, unmount } = renderHook(() => + useThemeColors({ highContrast: false, activeKeys: HW_KEYS }), + ); + + themeState.resolvedTheme = 'light'; + rerender(); + act(() => { + vi.runAllTimers(); + }); + rerender(); + const afterSwitch = result.current.themeColors; + + rerender(); + act(() => { + vi.runAllTimers(); + }); + rerender(); + + expect(result.current.themeColors).toBe(afterSwitch); + unmount(); + }); +}); + +describe('useThemeColors color maps', () => { + it('generates vendor colors for active hardware keys and muted fallback for inactive', () => { + const { result, unmount } = renderHook(() => + useThemeColors({ highContrast: false, activeKeys: ['h100', 'mi300x'] }), + ); + + // Active keys resolve to concrete oklch colors from the vendor palette. + expect(result.current.resolveColor('h100')).toMatch(/^oklch\(/u); + expect(result.current.resolveColor('mi300x')).toMatch(/^oklch\(/u); + // NVIDIA and AMD land in different hue zones. + expect(result.current.resolveColor('h100')).not.toBe(result.current.resolveColor('mi300x')); + // Inactive keys fall back to the muted foreground variable. + expect(result.current.resolveColor('gb200')).toBe('var(--muted-foreground)'); + unmount(); + }); + + it('returns null colorMap when highContrast is off and a populated one when on', () => { + const { result: offResult, unmount: unmountOff } = renderHook(() => + useThemeColors({ highContrast: false, activeKeys: HW_KEYS }), + ); + expect(offResult.current.colorMap).toBeNull(); + unmountOff(); + + const { result: onResult, unmount: unmountOn } = renderHook(() => + useThemeColors({ highContrast: true, activeKeys: HW_KEYS }), + ); + expect(onResult.current.colorMap).not.toBeNull(); + for (const key of HW_KEYS) { + expect(onResult.current.colorMap![key]).toMatch(/^#[0-9a-f]{6}$/iu); + } + unmountOn(); + }); +}); diff --git a/packages/app/src/hooks/useThemeColors.ts b/packages/app/src/hooks/useThemeColors.ts index ce1a3a23..5034b536 100644 --- a/packages/app/src/hooks/useThemeColors.ts +++ b/packages/app/src/hooks/useThemeColors.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useTheme } from 'next-themes'; import { generateHighContrastColors } from '@/lib/chart-utils'; import { getChartThemeColors } from '@/lib/chart-rendering'; @@ -87,8 +87,27 @@ export function useThemeColors(options: UseThemeColorsOptions): UseThemeColorsRe // get base theme colors const [themeColors, setThemeColors] = useState(() => getChartThemeColors()); - // update theme colors on next tick + // Re-read theme colors when the resolved theme actually changes. + // + // The first defined `resolvedTheme` is skipped on purpose: next-themes applies + // the theme class to in a blocking inline script before hydration, so + // the synchronous useState read above already saw the correct computed styles. + // Re-setting state here would only change the `themeColors` object identity, + // which invalidates `getCssColor` and forces every consuming chart through a + // full (and visually identical) D3 rebuild right after mount. + // + // The setTimeout(0) on real theme switches is load-bearing: the class flip on + // must be applied/recomputed before we trigger consumers to re-resolve + // CSS variables. + const appliedThemeRef = useRef(null); useEffect(() => { + if (!resolvedTheme) return; // next-themes not mounted yet + if (appliedThemeRef.current === null) { + appliedThemeRef.current = resolvedTheme; + return; + } + if (appliedThemeRef.current === resolvedTheme) return; + appliedThemeRef.current = resolvedTheme; const timeoutId = setTimeout(() => { setThemeColors(getChartThemeColors()); }, 0); From 3ec823f0ddf092cd226bbf07a89797eac0cdcf6d Mon Sep 17 00:00:00 2001 From: Oseltamivir <58582368+Oseltamivir@users.noreply.github.com> Date: Thu, 11 Jun 2026 22:46:49 -0700 Subject: [PATCH 2/2] perf(charts): cache iwanthue palettes per (vendor, theme, count, mode) iwanthue's force-vector clustering (quality 50 x 5 attempts) costs tens of milliseconds per call and showed up repeatedly in the profile's useMemo render paths whenever high-contrast colors recompute (mount, legend toggles, theme switches). The output is fully deterministic (seeded RNG) and independent of the key names - a vendor group's palette depends only on item count, vendor zone/ban mode, theme seed, and lightness bounds - so identical requests across renders, charts, and tabs now share one cached entry. Key space is tiny (vendors x themes x counts x 3 modes); no eviction needed. Existing color-quality tests (brand zones, bans, min distances) pass unchanged, confirming identical output. --- packages/app/src/lib/chart-utils.test.ts | 45 +++++++++++++++ packages/app/src/lib/chart-utils.ts | 71 +++++++++++++++--------- 2 files changed, 91 insertions(+), 25 deletions(-) diff --git a/packages/app/src/lib/chart-utils.test.ts b/packages/app/src/lib/chart-utils.test.ts index b1c24122..dc29e31f 100644 --- a/packages/app/src/lib/chart-utils.test.ts +++ b/packages/app/src/lib/chart-utils.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi } from 'vitest'; +import iwanthue from 'iwanthue'; import type * as ConstantsModule from '@/lib/constants'; import type { AggDataEntry, ChartDefinition, InferenceData } from '@/components/inference/types'; @@ -29,6 +30,10 @@ vi.mock('@/lib/constants', async (importOriginal) => { }; }); +// spy-wrap iwanthue (real implementation) so the palette-cache tests can +// assert how often the expensive clustering actually runs. +vi.mock('iwanthue', { spy: true }); + // --------------------------------------------------------------------------- // fixture factory // --------------------------------------------------------------------------- @@ -439,6 +444,46 @@ describe('generateHighContrastColors', () => { expect(isNotReddish(parseRgb(result['h100_vllm']))).toBe(true); expect(isNotGreenish(parseRgb(result['mi300x_sglang']))).toBe(true); }); + + // ---------- Palette caching ---------- + // iwanthue's force-vector clustering costs tens of ms per call; results are + // deterministic per (vendor, theme, count, mode), so repeats must be free. + // These tests use signatures (5 NVIDIA keys, light theme) no other test in + // this file requests, since the cache is module-level. + + it('caches palettes — a repeated request does not re-run iwanthue', () => { + const keys = ['h100_vllm', 'h200_vllm', 'b200_vllm', 'b300_vllm', 'gb200_vllm']; + + const callsBefore = vi.mocked(iwanthue).mock.calls.length; + const first = generateHighContrastColors(keys, 'light'); + expect(vi.mocked(iwanthue).mock.calls.length).toBe(callsBefore + 1); + + const second = generateHighContrastColors(keys, 'light'); + expect(vi.mocked(iwanthue).mock.calls.length).toBe(callsBefore + 1); // cache hit + expect(second).toEqual(first); + }); + + it('cache is key-name independent — same vendor/count/theme reuses the palette', () => { + // Same signature as the test above (NVIDIA × 5 × light), different names: + // palettes depend on the group shape, not on which GPUs are in it. + const callsBefore = vi.mocked(iwanthue).mock.calls.length; + const result = generateHighContrastColors( + ['h100_sglang', 'h200_sglang', 'b200_sglang', 'b300_sglang', 'gb300_sglang'], + 'light', + ); + expect(vi.mocked(iwanthue).mock.calls.length).toBe(callsBefore); // cache hit + expect(Object.keys(result)).toHaveLength(5); + expect(new Set(Object.values(result)).size).toBe(5); // still distinct colors + }); + + it('a different count is a different cache entry', () => { + const callsBefore = vi.mocked(iwanthue).mock.calls.length; + generateHighContrastColors( + ['h100_trt', 'h200_trt', 'b200_trt', 'b300_trt', 'gb200_trt', 'gb300_trt', 'gb200_vllm'], + 'light', + ); + expect(vi.mocked(iwanthue).mock.calls.length).toBe(callsBefore + 1); + }); }); // =========================================================================== diff --git a/packages/app/src/lib/chart-utils.ts b/packages/app/src/lib/chart-utils.ts index 9bf83032..b111c770 100644 --- a/packages/app/src/lib/chart-utils.ts +++ b/packages/app/src/lib/chart-utils.ts @@ -46,6 +46,18 @@ const PREFERRED_MAX = 4; /** Beyond this count per vendor, drop the hue ban entirely for best spacing. */ const BAN_MAX = 10; +/** + * Palette cache. iwanthue's force-vector clustering costs tens of + * milliseconds per call (quality 50 × 5 attempts) and shows up as main-thread + * time on every render that recomputes high-contrast colors. The output is + * fully deterministic (seeded RNG) and — crucially — independent of the key + * *names*: a vendor group's palette depends only on the item count, vendor + * zone/ban mode, theme seed, and lightness bounds. Identical requests across + * renders, charts, and tabs therefore share one entry. Key space is tiny + * (vendors × themes × counts × 3 modes), so no eviction is needed. + */ +const PALETTE_CACHE = new Map(); + /** * Generates high-contrast colors using iwanthue (k-means in CIELab space). * @@ -90,31 +102,40 @@ export const generateHighContrastColors = ( const usePreferred = preferred && count <= PREFERRED_MAX; const useBan = !usePreferred && isBanned && count <= BAN_MAX; - const palette = iwanthue(count, { - colorSpace: usePreferred - ? { - hmin: preferred.hmin, - hmax: preferred.hmax, - cmin: preferred.cmin ?? 30, - cmax: 100, - lmin: Math.max(lmin, preferred.lmin ?? 0), - lmax, - } - : { hmin: 0, hmax: 360, cmin: 30, cmax: 100, lmin, lmax }, - ...(useBan && - isBanned && { - colorFilter: (_rgb: [number, number, number], lab: [number, number, number]) => { - // Enforce lightness bounds — force-vector can drift outside colorSpace - if (lab[0] < lmin || lab[0] > lmax) return false; - const hue = ((Math.atan2(lab[2], lab[1]) * 180) / Math.PI + 360) % 360; - return !isBanned(hue); - }, - }), - seed: `${vendor}-${theme}`, - clustering: 'force-vector', - quality: 50, - attempts: 5, - }); + // Everything iwanthue's output depends on (the ban filter and preferred + // zone are functions of vendor; the seed is vendor+theme). + const mode = usePreferred ? 'pref' : useBan ? 'ban' : 'open'; + const cacheKey = `${vendor}|${theme}|${count}|${mode}|${lmin}|${lmax}`; + + let palette = PALETTE_CACHE.get(cacheKey); + if (!palette) { + palette = iwanthue(count, { + colorSpace: usePreferred + ? { + hmin: preferred.hmin, + hmax: preferred.hmax, + cmin: preferred.cmin ?? 30, + cmax: 100, + lmin: Math.max(lmin, preferred.lmin ?? 0), + lmax, + } + : { hmin: 0, hmax: 360, cmin: 30, cmax: 100, lmin, lmax }, + ...(useBan && + isBanned && { + colorFilter: (_rgb: [number, number, number], lab: [number, number, number]) => { + // Enforce lightness bounds — force-vector can drift outside colorSpace + if (lab[0] < lmin || lab[0] > lmax) return false; + const hue = ((Math.atan2(lab[2], lab[1]) * 180) / Math.PI + 360) % 360; + return !isBanned(hue); + }, + }), + seed: `${vendor}-${theme}`, + clustering: 'force-vector', + quality: 50, + attempts: 5, + }); + PALETTE_CACHE.set(cacheKey, palette); + } vendorKeys.sort(); vendorKeys.forEach((key, i) => {