From 56142301e9078012e1a726fa71a75df933a2516b Mon Sep 17 00:00:00 2001 From: Olivier Hoenen Date: Fri, 18 Sep 2026 11:19:30 +0200 Subject: [PATCH 01/10] test(perf): add a committed reactivity benchmark Measuring first, so every later reactivity change has a before/after number instead of an assertion. The benchmark builds the canvas from issue #121: a 2D equilibrium/time_slice/profiles_2d/psi heatmap plus a 1D grid with two traces. It asserts on counts - backend requests and Plotly redraws - and never on wall-clock times, which are noise on a shared runner; timings are still recorded and printed. Renderer-side counters live in utils/perf.ts and are installed only when E2E_TEST=true, matching the convention already used to disable Mantine transitions. Plotly redraws are counted through react-plotly's public onAfterPlot event, so nothing depends on the bundled plotly instance. The spec sits in src/tests/perf/, which keeps it out of `npm run test:e2e` (whose glob is not recursive) and behind the new `npm run test:perf`. Three of its four guards fail today, which is the point of committing them: toggling one panel's edit flag redraws the untouched panel 6 times, two slider steps cost 12 redraws (3 on an unrelated panel), and reopening a metadata tab re-downloads 2 plot_data payloads. BASELINE.md records the numbers. The disruption dataset is used rather than the scenario one because it carries several time slices, so the coordinate sliders are actually operable. Assisted-by: Claude/opus-5 --- .github/copilot-instructions.md | 12 + CLAUDE.md | 12 + frontend/package.json | 3 +- .../components/grid/GridLayoutPlot.tsx | 2 + .../renderer/components/grid/HoverButtons.tsx | 1 + .../renderer/components/plot/Heatmap2D.tsx | 7 + .../renderer/components/plot/SimplePlotly.tsx | 8 + .../verticalSlider/VerticalSlider.tsx | 1 + frontend/src/renderer/index.tsx | 5 + frontend/src/renderer/utils/perf.ts | 97 +++++ frontend/src/tests/perf/BASELINE.md | 34 ++ frontend/src/tests/perf/perfHelpers.ts | 143 +++++++ .../src/tests/perf/reactivity.perf.spec.ts | 368 ++++++++++++++++++ 13 files changed, 692 insertions(+), 1 deletion(-) create mode 100644 frontend/src/renderer/utils/perf.ts create mode 100644 frontend/src/tests/perf/BASELINE.md create mode 100644 frontend/src/tests/perf/perfHelpers.ts create mode 100644 frontend/src/tests/perf/reactivity.perf.spec.ts diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index f28341e9..0b804c1f 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -123,6 +123,18 @@ npx wait-on http://127.0.0.1:8000/docs/ # backend must also be running (pip in npm run test:e2e # runs mocha against src/tests/*.spec.ts ``` Run a single e2e spec: `mocha -r ts-node/register src/tests/plot-ui.spec.ts`. + +Reactivity benchmark (opt-in, not part of `test:e2e` — its glob is not recursive): +```bash +npm run start:e2e & # same app instance as the e2e suite +npm run test:perf # src/tests/perf/*.perf.spec.ts +``` +It builds a canvas with a 2D `equilibrium/time_slice/profiles_2d/psi` heatmap +plus 1D traces and asserts on *counts* (backend requests, Plotly redraws), never +on wall-clock times. The renderer-side counters live in +`src/renderer/utils/perf.ts` and are installed only when `E2E_TEST=true`; +`src/tests/perf/BASELINE.md` records the measured numbers. + On a headless machine wrap the run in `xvfb-run --auto-servernum` as CI does. `E2E_TEST=true` changes app behaviour in two places: `src/preload.ts` swaps the diff --git a/CLAUDE.md b/CLAUDE.md index bc233fc2..b3e4304a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -127,6 +127,18 @@ npx wait-on http://127.0.0.1:8000/docs/ # backend must also be running (pip in npm run test:e2e # runs mocha against src/tests/*.spec.ts ``` Run a single e2e spec: `mocha -r ts-node/register src/tests/plot-ui.spec.ts`. + +Reactivity benchmark (opt-in, not part of `test:e2e` — its glob is not recursive): +```bash +npm run start:e2e & # same app instance as the e2e suite +npm run test:perf # src/tests/perf/*.perf.spec.ts +``` +It builds a canvas with a 2D `equilibrium/time_slice/profiles_2d/psi` heatmap +plus 1D traces and asserts on *counts* (backend requests, Plotly redraws), never +on wall-clock times. The renderer-side counters live in +`src/renderer/utils/perf.ts` and are installed only when `E2E_TEST=true`; +`src/tests/perf/BASELINE.md` records the measured numbers. + On a headless machine wrap the run in `xvfb-run --auto-servernum` as CI does. `E2E_TEST=true` changes app behaviour in two places: `src/preload.ts` swaps the diff --git a/frontend/package.json b/frontend/package.json index c1b0f5c4..857b80b6 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -13,7 +13,8 @@ "format": "prettier --write .", "debug": "DEBUG=electron-forge:* electron-forge start", "start:e2e": "cross-env E2E_TEST=true electron-forge start -- --remote-debugging-port=9222 --no-watch", - "test:e2e": "mocha -r ts-node/register src/tests/*.spec.ts" + "test:e2e": "mocha -r ts-node/register src/tests/*.spec.ts", + "test:perf": "mocha -r ts-node/register src/tests/perf/*.perf.spec.ts" }, "devDependencies": { "@electron-forge/cli": "7.11.2", diff --git a/frontend/src/renderer/components/grid/GridLayoutPlot.tsx b/frontend/src/renderer/components/grid/GridLayoutPlot.tsx index d4357e75..a49e4f8e 100644 --- a/frontend/src/renderer/components/grid/GridLayoutPlot.tsx +++ b/frontend/src/renderer/components/grid/GridLayoutPlot.tsx @@ -13,6 +13,7 @@ import { import { Center, Container, Text } from '@mantine/core'; import { SimplePlotly, Heatmap2D } from '../plot'; import { useIbexStore } from '../../stores'; +import { countRender } from '../../utils/perf'; import { getArrayValueFromDependance, getErrorYVectors, @@ -31,6 +32,7 @@ export const GridLayoutPlot = ({ rowHeight, }: GridLayoutPlotProps) => { const { active, updatedConfiguration } = useIbexStore(); + countRender(`GridLayoutPlot:${data.i}`); const [heightGrid, setHeightGrid] = useState( data.h * rowHeight + (23 * (data.h * rowHeight)) / 100, ); diff --git a/frontend/src/renderer/components/grid/HoverButtons.tsx b/frontend/src/renderer/components/grid/HoverButtons.tsx index d841f996..662d9122 100644 --- a/frontend/src/renderer/components/grid/HoverButtons.tsx +++ b/frontend/src/renderer/components/grid/HoverButtons.tsx @@ -355,6 +355,7 @@ export const HoverButtons = React.memo( handleEditGrid(data.i)} className={classes.actionButton} color={data.isEditing ? 'yellow' : 'green'} diff --git a/frontend/src/renderer/components/plot/Heatmap2D.tsx b/frontend/src/renderer/components/plot/Heatmap2D.tsx index dda7bc67..c7d58d3b 100644 --- a/frontend/src/renderer/components/plot/Heatmap2D.tsx +++ b/frontend/src/renderer/components/plot/Heatmap2D.tsx @@ -22,6 +22,7 @@ import { } from '../../utils'; import classes from './Heatmap2D.module.css'; import { useIbexStore } from '../../stores'; +import { countRedraw, countRender } from '../../utils/perf'; import { NoDataForURI } from '.'; import { usePlotLayout } from './hooks/usePlotLayout'; import { IconLink } from '@tabler/icons-react'; @@ -49,6 +50,11 @@ export const Heatmap2D = ({ handleUpdateCoordinate, }: Heatmap2DProps) => { const { active, updatedConfiguration } = useIbexStore(); + countRender(`Heatmap2D:${itemDataGrid.i}`); + const handleAfterPlot = useCallback( + () => countRedraw(itemDataGrid.i), + [itemDataGrid.i], + ); const coordsUsedInAxes: 1 | 2 = 2; const SELECT_AXIS_HEIGHT = 90; // Height of the select axis container const [xAxis, setXAxis] = useState(null); @@ -489,6 +495,7 @@ export const Heatmap2D = ({ }} layout={layoutPlot} onRelayout={handleRelayout} + onAfterPlot={handleAfterPlot} useResizeHandler={false} className={classe.plot2D} /> diff --git a/frontend/src/renderer/components/plot/SimplePlotly.tsx b/frontend/src/renderer/components/plot/SimplePlotly.tsx index 28e2d7fa..46072382 100644 --- a/frontend/src/renderer/components/plot/SimplePlotly.tsx +++ b/frontend/src/renderer/components/plot/SimplePlotly.tsx @@ -15,6 +15,7 @@ import { swapAxis, } from '../../utils'; import classes from './SimplePlotly.module.css'; +import { countRedraw, countRender } from '../../utils/perf'; import { NoDataForURI } from '../plot'; import { usePlotLayout } from './hooks/usePlotLayout'; import { IconLink } from '@tabler/icons-react'; @@ -38,6 +39,12 @@ export const SimplePlotly = ({ is3DView, handleUpdateCoordinate, }: SimplePlotlyProps) => { + countRender(`SimplePlotly:${itemDataGrid.i}`); + const handleAfterPlot = useCallback( + () => countRedraw(itemDataGrid.i), + [itemDataGrid.i], + ); + const dataToPlotWithErrorBands = useMemo(() => { return getErrorsAreaToPlot( structuredClone(itemDataGrid.plot), @@ -480,6 +487,7 @@ export const SimplePlotly = ({ }} layout={layoutPlot} onRelayout={handleRelayout} + onAfterPlot={handleAfterPlot} useResizeHandler={false} /> diff --git a/frontend/src/renderer/components/verticalSlider/VerticalSlider.tsx b/frontend/src/renderer/components/verticalSlider/VerticalSlider.tsx index 5e63dee3..a8a450e4 100644 --- a/frontend/src/renderer/components/verticalSlider/VerticalSlider.tsx +++ b/frontend/src/renderer/components/verticalSlider/VerticalSlider.tsx @@ -88,6 +88,7 @@ export const VerticalSlider = ({ }} tabIndex={0} role="slider" + data-testid={`slider-${name}`} aria-valuenow={valueIndex} aria-valuemin={0} aria-valuemax={steps - 1} diff --git a/frontend/src/renderer/index.tsx b/frontend/src/renderer/index.tsx index c44102b1..1b5f6ae1 100644 --- a/frontend/src/renderer/index.tsx +++ b/frontend/src/renderer/index.tsx @@ -3,11 +3,16 @@ import '@mantine/core/styles.css'; import '@mantine/notifications/styles.css'; import './index.css'; import { App } from './App'; +import { installPerfCounters } from './utils/perf'; // Import react-grid-layout styles import '../../node_modules/react-grid-layout/css/styles.css'; import '../../node_modules/react-resizable/css/styles.css'; +// Instrument backend traffic before anything can issue a request. Inert +// unless E2E_TEST is set. +installPerfCounters(); + // Create a dedicated container element for the app const container = document.createElement('div'); container.id = 'root'; diff --git a/frontend/src/renderer/utils/perf.ts b/frontend/src/renderer/utils/perf.ts new file mode 100644 index 00000000..cda7d0d2 --- /dev/null +++ b/frontend/src/renderer/utils/perf.ts @@ -0,0 +1,97 @@ +/** + * Reactivity instrumentation used by the performance e2e spec. + * + * Everything here is inert unless `window.env.E2E_TEST === 'true'`, following + * the same convention the components use to disable Mantine transitions. In a + * normal run the counters are never installed and `countRender`/`countRedraw` + * are single boolean checks. + * + * The spec drives this through `window.__ibexPerf`: `reset()` before an + * interaction, `snapshot()` after it. Counts (not wall-clock) are the primary + * metric because they are deterministic and therefore safe to assert in CI. + */ + +export interface PerfSnapshot { + /** Number of HTTP requests issued to the backend since the last reset. */ + fetchCount: number; + /** URLs of those requests, in order, so a spec can assert which endpoint. */ + fetchUrls: string[]; + /** Component render counts, keyed by `":"`. */ + renders: Record; + /** Plotly redraw counts, keyed by grid id. */ + redraws: Record; +} + +interface PerfApi extends PerfSnapshot { + reset: () => void; + snapshot: () => PerfSnapshot; +} + +declare global { + interface Window { + __ibexPerf?: PerfApi; + } +} + +const isEnabled = (): boolean => + typeof window !== 'undefined' && window.env?.E2E_TEST === 'true'; + +/** + * Installs the counters and wraps `window.fetch`. Called once from the renderer + * entrypoint; repeated calls are ignored so hot reload cannot double-wrap. + */ +export const installPerfCounters = (): void => { + if (!isEnabled() || window.__ibexPerf) return; + + const api: PerfApi = { + fetchCount: 0, + fetchUrls: [], + renders: {}, + redraws: {}, + reset() { + api.fetchCount = 0; + api.fetchUrls = []; + api.renders = {}; + api.redraws = {}; + }, + snapshot() { + return { + fetchCount: api.fetchCount, + fetchUrls: [...api.fetchUrls], + renders: { ...api.renders }, + redraws: { ...api.redraws }, + }; + }, + }; + + window.__ibexPerf = api; + + // Every renderer -> backend call goes through fetchFromApi, which uses the + // global fetch, so this is the single accounting point for backend traffic. + const originalFetch = window.fetch.bind(window); + window.fetch = (input: RequestInfo | URL, init?: RequestInit) => { + const url = + typeof input === 'string' + ? input + : input instanceof URL + ? input.toString() + : input.url; + api.fetchCount += 1; + api.fetchUrls.push(url); + return originalFetch(input, init); + }; +}; + +/** Records one render of `key`. No-op outside E2E runs. */ +export const countRender = (key: string): void => { + const perf = window.__ibexPerf; + if (!perf) return; + perf.renders[key] = (perf.renders[key] ?? 0) + 1; +}; + +/** Records one Plotly redraw for `gridId`. No-op outside E2E runs. */ +export const countRedraw = (gridId: string): void => { + const perf = window.__ibexPerf; + if (!perf) return; + perf.redraws[gridId] = (perf.redraws[gridId] ?? 0) + 1; +}; diff --git a/frontend/src/tests/perf/BASELINE.md b/frontend/src/tests/perf/BASELINE.md new file mode 100644 index 00000000..65437247 --- /dev/null +++ b/frontend/src/tests/perf/BASELINE.md @@ -0,0 +1,34 @@ +# Reactivity baseline + +Measured with `npm run test:perf` on the benchmark canvas (a 2-D +`equilibrium/time_slice/profiles_2d/psi` heatmap plus a 1-D grid with two +traces, both from `iter_disruption_113112_1.nc`). + +`requests` counts calls to `/data/*` and `/ids_info/*`; `redraws` counts Plotly +redraws across all panels; `renders` counts renders of the instrumented +components. Timings are informational only — they are not asserted. + +## Before any reactivity work (commit 697db19) + +| Scenario | requests | redraws | renders | ms | +|---|---|---|---|---| +| toggle edit mode (UI flag) | 0 | 11 | 42 | 1209 | +| coordinate slider, 2 steps | 0 | 12 | 40 | 1618 | +| metadata panel, first open | 4 | 3 | 6 | 989 | +| metadata panel, revisit | 6 | 19 | 72 | 1881 | +| idle (no interaction) | 0 | 0 | 0 | 2125 | + +Three guards fail at this baseline, which is the point of committing them: + +1. **Toggling one panel's edit flag redraws the other panel 6 times.** The flag + is a boolean; no data changes. Cause: `updatedConfiguration` replaces the + whole configuration and every component subscribes without a selector. +2. **Two slider steps cost 12 redraws, 3 of them on the unrelated 1-D panel.** + Sliders correctly issue no backend request, but every tick writes the whole + configuration. +3. **Reopening the same metadata tab re-downloads 2 `plot_data` payloads.** + `VisualizationMetaData` refetches the entire payload to read + `response.data.coordinates`. + +The idle scenario passes and must keep passing: it guards against runaway +effect loops. diff --git a/frontend/src/tests/perf/perfHelpers.ts b/frontend/src/tests/perf/perfHelpers.ts new file mode 100644 index 00000000..732522b5 --- /dev/null +++ b/frontend/src/tests/perf/perfHelpers.ts @@ -0,0 +1,143 @@ +import { getDriver } from '../setup'; +import type { PerfSnapshot } from '../../renderer/utils/perf'; + +/** + * Helpers for the reactivity spec. They drive `window.__ibexPerf`, which the + * renderer installs only when E2E_TEST is set (see renderer/utils/perf.ts). + */ + +/** Clears all counters. Call immediately before the interaction under test. */ +export async function resetPerf(): Promise { + await getDriver().executeScript(() => { + window.__ibexPerf?.reset(); + }); +} + +/** Reads the counters accumulated since the last {@link resetPerf}. */ +export async function readPerf(): Promise { + const snapshot = await getDriver().executeScript( + () => + window.__ibexPerf?.snapshot() ?? { + fetchCount: 0, + fetchUrls: [], + renders: {}, + redraws: {}, + }, + ); + return snapshot as PerfSnapshot; +} + +/** + * Fails loudly if the instrumentation is missing, rather than letting every + * scenario silently report zero. + */ +export async function assertPerfInstalled(): Promise { + const installed = await getDriver().executeScript( + () => typeof window.__ibexPerf !== 'undefined', + ); + if (!installed) { + throw new Error( + 'window.__ibexPerf is not installed. The app must be started with ' + + 'E2E_TEST=true (npm run start:e2e) for the reactivity spec to run.', + ); + } +} + +/** + * Waits until no new render, redraw or request has been recorded for + * `quietMs`, so a measurement is not taken while React is still settling. + */ +export async function waitForQuiescence( + quietMs = 600, + timeoutMs = 20000, +): Promise { + const deadline = Date.now() + timeoutMs; + let previous = ''; + let stableSince = Date.now(); + + while (Date.now() < deadline) { + const snapshot = await readPerf(); + const fingerprint = JSON.stringify([ + snapshot.fetchCount, + snapshot.renders, + snapshot.redraws, + ]); + if (fingerprint !== previous) { + previous = fingerprint; + stableSince = Date.now(); + } else if (Date.now() - stableSince >= quietMs) { + return; + } + await new Promise((resolve) => setTimeout(resolve, 100)); + } +} + +/** Total renders across every instrumented component instance. */ +export function totalRenders(snapshot: PerfSnapshot): number { + return Object.values(snapshot.renders).reduce((sum, n) => sum + n, 0); +} + +/** Total Plotly redraws across every panel. */ +export function totalRedraws(snapshot: PerfSnapshot): number { + return Object.values(snapshot.redraws).reduce((sum, n) => sum + n, 0); +} + +/** Requests that actually hit the data endpoints (ignores /info polling). */ +export function dataRequests(snapshot: PerfSnapshot): string[] { + return snapshot.fetchUrls.filter( + (url) => url.includes('/data/') || url.includes('/ids_info/'), + ); +} + +const measurements: { + scenario: string; + dataRequests: number; + redraws: number; + renders: number; + ms: number; +}[] = []; + +/** + * Runs `interaction` with the counters zeroed, waits for the UI to settle and + * records the result for the end-of-run report. + */ +export async function measure( + scenario: string, + interaction: () => Promise, +): Promise { + await waitForQuiescence(); + await resetPerf(); + + const startedAt = Date.now(); + await interaction(); + await waitForQuiescence(); + const ms = Date.now() - startedAt; + + const snapshot = await readPerf(); + measurements.push({ + scenario, + dataRequests: dataRequests(snapshot).length, + redraws: totalRedraws(snapshot), + renders: totalRenders(snapshot), + ms, + }); + return snapshot; +} + +/** + * Prints the collected numbers. Timings are reported but never asserted: on a + * shared CI runner they are noise, whereas the counts are deterministic. + */ +export function reportMeasurements(): void { + if (measurements.length === 0) return; + console.info('\nReactivity measurements'); + for (const row of measurements) { + console.info( + ` ${row.scenario.padEnd(36)} ` + + `requests=${String(row.dataRequests).padStart(3)} ` + + `redraws=${String(row.redraws).padStart(3)} ` + + `renders=${String(row.renders).padStart(4)} ` + + `${row.ms} ms`, + ); + } +} diff --git a/frontend/src/tests/perf/reactivity.perf.spec.ts b/frontend/src/tests/perf/reactivity.perf.spec.ts new file mode 100644 index 00000000..b203847f --- /dev/null +++ b/frontend/src/tests/perf/reactivity.perf.spec.ts @@ -0,0 +1,368 @@ +import { Key } from 'selenium-webdriver'; +import { expect } from 'chai'; +import { + startApp, + getDriver, + stopApp, + waitForApi, + getTestState, + setTestState, +} from '../setup'; +import { + addUriAndAwaitSelection, + ensureCssElementIsDisplayed, + findCssElementAndClickIt, + getDatasetPath, + resetAppState, + waitForElementToDisappear, + waitForValue, + writeTextInCssElement, +} from '../utils'; +import { + assertPerfInstalled, + dataRequests, + measure, + reportMeasurements, + totalRedraws, +} from './perfHelpers'; +import '../../config/bridge'; + +/** + * Reactivity benchmark. + * + * Deliberately NOT part of `npm run test:e2e`, whose glob `src/tests/*.spec.ts` + * is not recursive. Run it with `npm run test:perf` against an app started by + * `npm run start:e2e`. + * + * It asserts on *counts* — backend requests and Plotly redraws — never on + * wall-clock times: counts are deterministic, timings on a shared runner are + * not. Timings are still recorded and printed for information. + * + * Canvas: the one named in the issue — a 2-D + * `equilibrium/time_slice/profiles_2d/psi` heatmap plus a 1-D grid. The + * disruption dataset is used because it carries several time slices + * (psi is [3, 1, 120, 70]), so the coordinate slider is actually usable; the + * scenario dataset has a single slice and renders every slider disabled. + */ + +const DATASET = 'iter_disruption_113112_1.nc'; +const EQUILIBRIUM = 'equilibrium:0'; +const TIME_SLICE = `${EQUILIBRIUM}/time_slice[:]`; +const PROFILES_2D = `${TIME_SLICE}/profiles_2d[:]`; +const GLOBAL_QUANTITIES = 'disruption:0/global_quantities'; + +/** Retry budget for steps that wait on a backend round trip. */ +const SLOW = { retries: 200, delay: 300 }; + +let heatmapGridId = ''; +let lineGridId = ''; + +describe('Reactivity benchmark', function () { + this.timeout(900000); + + before(async () => { + await startApp(); + await waitForApi(); + await assertPerfInstalled(); + await resetAppState(); + await buildCanvas(); + }); + + after(async () => { + reportMeasurements(); + await stopApp(); + }); + + /** Builds the two-panel benchmark canvas from a single data entry. */ + async function buildCanvas() { + await findCssElementAndClickIt('header-add-configuration'); + await ensureCssElementIsDisplayed('config-create-modal'); + await writeTextInCssElement('config-create-name-input', 'Perf', true); + await findCssElementAndClickIt('config-create-submit-button'); + await waitForValue( + 'configuration created', + async () => (await getTestState()).configurations.length, + 1, + ); + + const dataPath = await getDatasetPath(DATASET); + const uriModal = await ensureCssElementIsDisplayed( + 'config-uri-selection-modal', + ); + await writeTextInCssElement( + 'config-uri-selection-modal-uri-text-input', + dataPath, + true, + ); + await addUriAndAwaitSelection(dataPath); + await findCssElementAndClickIt( + 'config-uri-selection-modal-validate-button', + 100, + 300, + ); + await waitForElementToDisappear(uriModal, 60000); + + await ensureCssElementIsDisplayed(`uriAccordion-${dataPath}`, 600, 100); + await findCssElementAndClickIt(`uriAccordion-${dataPath}`, 200, 100); + await findCssElementAndClickIt( + `folder-${dataPath}#${EQUILIBRIUM}/`, + 200, + 100, + ); + await findCssElementAndClickIt( + `folder-${dataPath}#${TIME_SLICE}/`, + 200, + 100, + ); + await findCssElementAndClickIt( + `folder-${dataPath}#${PROFILES_2D}/`, + 200, + 100, + ); + + // Grid 1: the 2-D psi heatmap, with a usable time slider. + await findCssElementAndClickIt(`checkbox-${dataPath}#${PROFILES_2D}/psi`); + await waitForValue( + '2D grid created', + async () => (await getTestState()).active.dataPlot.length, + 1, + undefined, + SLOW.retries, + SLOW.delay, + ); + heatmapGridId = (await getTestState()).active.dataPlot[0].i; + await leaveEditMode(0); + + // Grid 2: two 1-D traces over time. + await findCssElementAndClickIt( + `folder-${dataPath}#disruption:0/`, + 200, + 100, + ); + await findCssElementAndClickIt( + `folder-${dataPath}#${GLOBAL_QUANTITIES}/`, + 200, + 100, + ); + await addTrace(dataPath, `${GLOBAL_QUANTITIES}/power_ohm`, 1, 2); + lineGridId = (await getTestState()).active.dataPlot[1].i; + await addTrace(dataPath, `${GLOBAL_QUANTITIES}/power_ohm_halo`, 2, 2); + await leaveEditMode(1); + } + + /** Checks a leaf and waits for the resulting grid/trace to settle. */ + async function addTrace( + dataPath: string, + leaf: string, + expectedTraces: number, + expectedGrids: number, + ) { + await findCssElementAndClickIt(`checkbox-${dataPath}#${leaf}`, 200, 100); + await waitForValue( + `grid count after ${leaf}`, + async () => (await getTestState()).active.dataPlot.length, + expectedGrids, + undefined, + SLOW.retries, + SLOW.delay, + ); + await waitForValue( + `trace count after ${leaf}`, + async () => (await getTestState()).active.dataPlot[1].plot.length, + expectedTraces, + undefined, + SLOW.retries, + SLOW.delay, + ); + } + + /** Takes a grid out of edit mode so the next leaf opens a new grid. */ + async function leaveEditMode(index: number) { + const gridId = (await getTestState()).active.dataPlot[index].i; + await findCssElementAndClickIt(`grid-edit-toggle-${gridId}`, 200, 100); + await waitForValue( + `grid ${index} left edit mode`, + async () => (await getTestState()).active.dataPlot[index].isEditing, + false, + undefined, + SLOW.retries, + SLOW.delay, + ); + } + + it('a pure UI toggle costs no backend request and spares the other panel', async () => { + // Entering edit mode flips one boolean. No data changes at all. + const snapshot = await measure('toggle edit mode (UI flag)', async () => { + await findCssElementAndClickIt(`grid-edit-toggle-${lineGridId}`); + await waitForValue( + 'grid entered edit mode', + async () => + (await getTestState()).active.dataPlot.find( + (grid) => grid.i === lineGridId, + )?.isEditing, + true, + undefined, + SLOW.retries, + SLOW.delay, + ); + }); + + expect( + dataRequests(snapshot), + `a UI-only toggle must not query the backend, got ${JSON.stringify( + dataRequests(snapshot), + )}`, + ).to.have.length(0); + + expect( + snapshot.redraws[heatmapGridId] ?? 0, + 'toggling one panel must not redraw the untouched heatmap panel', + ).to.equal(0); + + await findCssElementAndClickIt(`grid-edit-toggle-${lineGridId}`); + }); + + it('stepping a coordinate slider queries nothing and spares other panels', async () => { + // Coordinate sliders are only operable while their grid is being edited + // (Heatmap2D.tsx passes `disabled={!itemDataGrid.isEditing}`), so enter + // edit mode first — outside the measured block, so its cost is not counted. + await setGridEditing(heatmapGridId, true); + const slider = await findEnabledSlider(); + + const snapshot = await measure('coordinate slider, 2 steps', async () => { + await slider.click(); + await slider.sendKeys(Key.ARROW_UP); + await getDriver().sleep(400); + await slider.sendKeys(Key.ARROW_UP); + }); + + expect( + dataRequests(snapshot), + `sliders slice already-loaded data and must not refetch, got ${JSON.stringify( + dataRequests(snapshot), + )}`, + ).to.have.length(0); + + expect( + snapshot.redraws[lineGridId] ?? 0, + 'stepping the heatmap slider must not redraw the 1-D panel', + ).to.equal(0); + + await setGridEditing(heatmapGridId, false); + }); + + it('revisiting a metadata tab does not re-download the payload', async () => { + await measure('metadata panel, first open', async () => { + await setMetadataPanel(lineGridId); + await waitForValue( + 'metadata panel open', + async () => Boolean((await getTestState()).active.metadataGridLayout), + true, + undefined, + SLOW.retries, + SLOW.delay, + ); + }); + + const snapshot = await measure('metadata panel, revisit', async () => { + await setMetadataPanel(null); + await waitForValue( + 'metadata panel closed', + async () => Boolean((await getTestState()).active.metadataGridLayout), + false, + undefined, + SLOW.retries, + SLOW.delay, + ); + await setMetadataPanel(lineGridId); + await waitForValue( + 'metadata panel reopened', + async () => Boolean((await getTestState()).active.metadataGridLayout), + true, + undefined, + SLOW.retries, + SLOW.delay, + ); + }); + + const plotDataCalls = snapshot.fetchUrls.filter((url) => + url.includes('/data/plot_data'), + ); + expect( + plotDataCalls, + `reopening the same metadata tab must not re-download plot_data, got ${plotDataCalls.length}`, + ).to.have.length(0); + + await setMetadataPanel(null); + }); + + it('is quiet at rest', async () => { + const snapshot = await measure('idle (no interaction)', async () => { + await getDriver().sleep(1500); + }); + expect(totalRedraws(snapshot), 'an idle canvas must not redraw').to.equal( + 0, + ); + expect( + dataRequests(snapshot), + 'an idle canvas must not query the backend', + ).to.have.length(0); + }); +}); + +/** + * Returns the first coordinate slider that is actually operable. Sliders over a + * single-valued coordinate render disabled, and driving one would measure + * nothing. + */ +async function findEnabledSlider() { + const sliders = await getDriver().findElements({ + css: '[data-testid^="slider-"]', + }); + for (const slider of sliders) { + const max = Number(await slider.getAttribute('aria-valuemax')); + if (Number.isFinite(max) && max > 0) return slider; + } + throw new Error( + 'no operable coordinate slider on the benchmark canvas: every slider ' + + 'covers a single-valued coordinate', + ); +} + +/** Puts one grid in or out of edit mode through the e2e state bridge. */ +async function setGridEditing(gridId: string, editing: boolean) { + const state = await getTestState(); + await setTestState({ + configurations: state.configurations, + active: { + ...state.active, + dataPlot: state.active.dataPlot.map((grid) => + grid.i === gridId + ? { ...grid, isEditing: editing, static: editing } + : { ...grid, isEditing: false, static: false }, + ), + }, + }); + await waitForValue( + `grid ${gridId} editing=${editing}`, + async () => + (await getTestState()).active.dataPlot.find((g) => g.i === gridId) + ?.isEditing ?? false, + editing, + undefined, + SLOW.retries, + SLOW.delay, + ); +} + +/** + * Opens or closes the metadata panel through the e2e state bridge. What is + * measured is the panel's data fetching, not the button that opens it. + */ +async function setMetadataPanel(gridId: string | null) { + const state = await getTestState(); + await setTestState({ + configurations: state.configurations, + active: { ...state.active, metadataGridLayout: gridId }, + }); +} From 9de44877a50478ddfac76a9504cda5d010999d1b Mon Sep 17 00:00:00 2001 From: Olivier Hoenen Date: Fri, 18 Sep 2026 11:36:39 +0200 Subject: [PATCH 02/10] perf(fetch): cache backend responses for the session A data entry does not change while IBEX is open, so a response stays valid for the whole session. fetchFromApi is the single place every renderer->backend GET goes through, so the cache sits there. The cache stores the response body TEXT and each caller parses its own copy. That is a correctness requirement, not an optimisation: fetchDataPlot post-processes the parsed response in place (it renames data.name, rewrites coord.target, tensorizes irregular data, runs the non-idempotent transformComplexData, then replaceNullsWithNaN) and five call sites then alias the result straight into the store with `plot.yData = response.data.value`, where transposeAxis and applyRange go on to mutate it in place. Sharing one parsed object would alias one grid's data to another's and corrupt the cached entry with it. Also: in-flight de-duplication so concurrent identical requests share one round trip, an LRU bounded by total and per-entry bytes so a large 2D payload cannot evict the session (it is de-duplicated but not retained), and getConfig() resolved once instead of crossing the Electron IPC boundary on every request. /info/version opts out - the header polls it as a liveness probe. Measured on the benchmark canvas: reopening a metadata tab drops from 6 requests to 1 and no longer re-downloads plot_data at all, which is the third benchmark guard now passing. First open drops from 4 requests to 2. resetAppState() clears the cache between specs. Without it an earlier spec warms the cache and a later one exercises a different path - which is exactly how two data-manipulation specs failed while passing in isolation. Assisted-by: Claude/opus-5 --- frontend/src/renderer/utils/fetchData.ts | 46 +++-- frontend/src/renderer/utils/perf.ts | 63 ++++--- frontend/src/renderer/utils/requestCache.ts | 166 +++++++++++++++++++ frontend/src/tests/perf/BASELINE.md | 32 +++- frontend/src/tests/perf/perfHelpers.ts | 13 +- frontend/src/tests/utils/dataManipulation.ts | 11 ++ 6 files changed, 282 insertions(+), 49 deletions(-) create mode 100644 frontend/src/renderer/utils/requestCache.ts diff --git a/frontend/src/renderer/utils/fetchData.ts b/frontend/src/renderer/utils/fetchData.ts index 1067bd3f..cf969162 100644 --- a/frontend/src/renderer/utils/fetchData.ts +++ b/frontend/src/renderer/utils/fetchData.ts @@ -26,16 +26,23 @@ import { getTensorizedMatrix, transformComplexData } from './plot'; import { replaceNullsWithNaN } from './functions'; import { normalizeIndices } from './uri'; import { OptionWithTooltip } from '../types/components/select'; +import { cachedRequest } from './requestCache'; /** * Retrieves the API configuration. */ +let configPromise: ReturnType | null = null; + const getConfig = async () => { try { - const config = await window.api.getConfig(); + // The config is fixed for the lifetime of the session, so resolve it once + // instead of crossing the Electron IPC boundary on every request. + configPromise ??= window.api.getConfig(); + const config = await configPromise; if (!config) throw new Error('Failed to load configuration'); return config; } catch (error) { + configPromise = null; // allow a retry console.error('Error fetching config:', error); throw error; } @@ -113,6 +120,7 @@ async function fetchWithTimeout( const fetchFromApi = async ( endpoint: string, timeout?: number, + cacheable = true, ): Promise => { let responseStatus: number; try { @@ -136,19 +144,29 @@ const fetchFromApi = async ( }); } }; - const response = await fetchFn(); + // The cache stores the body text and each caller parses its own copy: the + // parsed graph is mutated in place downstream, so it must never be shared. + const body = await cachedRequest( + url, + async () => { + const response = await fetchFn(); + + if (!response.ok) { + if (response?.status) { + responseStatus = response.status; + } + const errorData = await response.json(); + throw new Error( + errorData.message || errorData.detail || 'Failed to fetch data', + ); + } - if (!response.ok) { - if (response?.status) { - responseStatus = response.status; - } - const errorData = await response.json(); - throw new Error( - errorData.message || errorData.detail || 'Failed to fetch data', - ); - } + return response.text(); + }, + cacheable, + ); - return response.json(); + return JSON.parse(body) as T; } catch (error) { if (error.name === 'AbortError') { console.error(`Timeout after ${timeout}ms: fetchFromApi(${endpoint}).`); @@ -636,5 +654,7 @@ export const fetchGeometryNodes = async (uri: string, labelUri: string) => { * Return backend version. */ export const fetchInfoVersion = async () => { - return fetchFromApi(`/info/version`); + // Never cached: the header polls this to show whether the backend is alive, + // and a cached answer would freeze that indicator on its first value. + return fetchFromApi(`/info/version`, undefined, false); }; diff --git a/frontend/src/renderer/utils/perf.ts b/frontend/src/renderer/utils/perf.ts index cda7d0d2..57dc5234 100644 --- a/frontend/src/renderer/utils/perf.ts +++ b/frontend/src/renderer/utils/perf.ts @@ -11,6 +11,19 @@ * metric because they are deterministic and therefore safe to assert in CI. */ +import { clearRequestCache, getRequestCacheStats } from './requestCache'; + +/** Request-cache counters, mirrored from requestCache.ts. */ +export interface PerfCacheStats { + hits: number; + dedup: number; + misses: number; + evictions: number; + skipped: number; + entries: number; + bytes: number; +} + export interface PerfSnapshot { /** Number of HTTP requests issued to the backend since the last reset. */ fetchCount: number; @@ -20,11 +33,14 @@ export interface PerfSnapshot { renders: Record; /** Plotly redraw counts, keyed by grid id. */ redraws: Record; + /** Cumulative request-cache counters (not reset between measurements). */ + cache: PerfCacheStats; } -interface PerfApi extends PerfSnapshot { +interface PerfApi { reset: () => void; snapshot: () => PerfSnapshot; + clearRequestCache: () => void; } declare global { @@ -33,6 +49,13 @@ declare global { } } +const counters = { + fetchCount: 0, + fetchUrls: [] as string[], + renders: {} as Record, + redraws: {} as Record, +}; + const isEnabled = (): boolean => typeof window !== 'undefined' && window.env?.E2E_TEST === 'true'; @@ -44,30 +67,30 @@ export const installPerfCounters = (): void => { if (!isEnabled() || window.__ibexPerf) return; const api: PerfApi = { - fetchCount: 0, - fetchUrls: [], - renders: {}, - redraws: {}, reset() { - api.fetchCount = 0; - api.fetchUrls = []; - api.renders = {}; - api.redraws = {}; + counters.fetchCount = 0; + counters.fetchUrls = []; + counters.renders = {}; + counters.redraws = {}; }, snapshot() { return { - fetchCount: api.fetchCount, - fetchUrls: [...api.fetchUrls], - renders: { ...api.renders }, - redraws: { ...api.redraws }, + fetchCount: counters.fetchCount, + fetchUrls: [...counters.fetchUrls], + renders: { ...counters.renders }, + redraws: { ...counters.redraws }, + cache: getRequestCacheStats(), }; }, + clearRequestCache, }; window.__ibexPerf = api; // Every renderer -> backend call goes through fetchFromApi, which uses the // global fetch, so this is the single accounting point for backend traffic. + // Note this counts requests that actually reach the network: a cache hit + // never gets here, which is exactly what the benchmark asserts on. const originalFetch = window.fetch.bind(window); window.fetch = (input: RequestInfo | URL, init?: RequestInit) => { const url = @@ -76,22 +99,20 @@ export const installPerfCounters = (): void => { : input instanceof URL ? input.toString() : input.url; - api.fetchCount += 1; - api.fetchUrls.push(url); + counters.fetchCount += 1; + counters.fetchUrls.push(url); return originalFetch(input, init); }; }; /** Records one render of `key`. No-op outside E2E runs. */ export const countRender = (key: string): void => { - const perf = window.__ibexPerf; - if (!perf) return; - perf.renders[key] = (perf.renders[key] ?? 0) + 1; + if (!window.__ibexPerf) return; + counters.renders[key] = (counters.renders[key] ?? 0) + 1; }; /** Records one Plotly redraw for `gridId`. No-op outside E2E runs. */ export const countRedraw = (gridId: string): void => { - const perf = window.__ibexPerf; - if (!perf) return; - perf.redraws[gridId] = (perf.redraws[gridId] ?? 0) + 1; + if (!window.__ibexPerf) return; + counters.redraws[gridId] = (counters.redraws[gridId] ?? 0) + 1; }; diff --git a/frontend/src/renderer/utils/requestCache.ts b/frontend/src/renderer/utils/requestCache.ts new file mode 100644 index 00000000..8a614de4 --- /dev/null +++ b/frontend/src/renderer/utils/requestCache.ts @@ -0,0 +1,166 @@ +/** + * Session-scoped cache for backend GET requests. + * + * A data entry does not change while IBEX is open, so a response is valid for + * the whole session and identical requests can be served without another round + * trip. + * + * ## Why the raw body text is cached rather than the parsed object + * + * `fetchDataPlot` post-processes the parsed response *in place* (it renames + * `data.name`, rewrites every `coord.target`, tensorizes irregular data, runs + * the non-idempotent `transformComplexData`, then `replaceNullsWithNaN`), and + * five call sites then alias the result straight into the store with + * `plot.yData = response.data.value`. Those arrays are afterwards mutated in + * place by `transposeAxis` and `applyRange`. + * + * Handing out a shared parsed object would therefore (a) re-apply the + * non-idempotent transforms on a hit and (b) alias one grid's data to another + * grid's, corrupting both and the cache entry with them. Caching the text and + * parsing per hit yields a fresh object graph every time, leaves the whole + * post-processing pipeline untouched, and makes byte accounting exact. + * Parsing costs a few ms per MB — always far less than the request it replaces. + */ + +/** Total budget for retained bodies. */ +const MAX_TOTAL_BYTES = 256 * 1024 * 1024; + +/** + * Bodies above this are never retained. They are still de-duplicated while in + * flight, they just do not get to evict everything else: a single 2-D payload + * can be larger than the sum of every 1-D payload in the session. + */ +const MAX_ENTRY_BYTES = 64 * 1024 * 1024; + +/** Insertion-ordered, which is what makes plain `Map` usable as an LRU. */ +const bodies = new Map(); +const inFlight = new Map>(); +let totalBytes = 0; + +const stats = { + hits: 0, + dedup: 0, + misses: 0, + evictions: 0, + skipped: 0, +}; + +/** + * Canonical key for a request URL: path plus query, with parameter *keys* + * sorted but the order of repeated values preserved. + * + * Repeated parameters are semantic here — `operations`, `signal_operations` and + * `interpolate_over` are ordered lists — so only the key order may be + * normalised. The origin is dropped so a backend port change cannot look like a + * different request. + */ +export const requestCacheKey = (url: string): string => { + try { + const parsed = new URL(url); + const grouped = new Map(); + for (const [key, value] of parsed.searchParams) { + const values = grouped.get(key); + if (values) values.push(value); + else grouped.set(key, [value]); + } + const query = [...grouped.entries()] + .sort(([a], [b]) => a.localeCompare(b)) + .map(([key, values]) => + values.map((value) => `${key}=${encodeURIComponent(value)}`).join('&'), + ) + .join('&'); + return query ? `${parsed.pathname}?${query}` : parsed.pathname; + } catch { + // Not an absolute URL: the raw string is still a stable key. + return url; + } +}; + +/** Returns a retained body, refreshing its recency. */ +const takeCached = (key: string): string | undefined => { + const body = bodies.get(key); + if (body === undefined) return undefined; + bodies.delete(key); + bodies.set(key, body); + return body; +}; + +/** Retains a body, evicting least-recently-used entries to stay in budget. */ +const retain = (key: string, body: string): void => { + if (body.length > MAX_ENTRY_BYTES) { + stats.skipped += 1; + return; + } + const existing = bodies.get(key); + if (existing !== undefined) { + totalBytes -= existing.length; + bodies.delete(key); + } + while (bodies.size > 0 && totalBytes + body.length > MAX_TOTAL_BYTES) { + const oldest = bodies.keys().next().value as string; + totalBytes -= bodies.get(oldest).length; + bodies.delete(oldest); + stats.evictions += 1; + } + bodies.set(key, body); + totalBytes += body.length; +}; + +/** + * Runs `request` unless an identical one is cached or already in flight. + * + * @param url Absolute request URL, used to derive the cache key. + * @param request Performs the request and resolves to the response body text. + * @param cacheable `false` for probes such as `/info/version`, which must stay + * live. Those are still de-duplicated while in flight. + */ +export const cachedRequest = async ( + url: string, + request: () => Promise, + cacheable = true, +): Promise => { + const key = requestCacheKey(url); + + if (cacheable) { + const cached = takeCached(key); + if (cached !== undefined) { + stats.hits += 1; + return cached; + } + } + + const pending = inFlight.get(key); + if (pending) { + stats.dedup += 1; + return pending; + } + + stats.misses += 1; + const promise = request() + .then((body) => { + if (cacheable) retain(key, body); + return body; + }) + .finally(() => { + inFlight.delete(key); + }); + + inFlight.set(key, promise); + return promise; +}; + +/** + * Drops every retained body. Called when the selected data entries change, and + * by the tests so one spec cannot warm the cache for the next. + */ +export const clearRequestCache = (): void => { + bodies.clear(); + totalBytes = 0; +}; + +/** Counters for the reactivity benchmark. */ +export const getRequestCacheStats = () => ({ + ...stats, + entries: bodies.size, + bytes: totalBytes, +}); diff --git a/frontend/src/tests/perf/BASELINE.md b/frontend/src/tests/perf/BASELINE.md index 65437247..d32d2d37 100644 --- a/frontend/src/tests/perf/BASELINE.md +++ b/frontend/src/tests/perf/BASELINE.md @@ -10,13 +10,13 @@ components. Timings are informational only — they are not asserted. ## Before any reactivity work (commit 697db19) -| Scenario | requests | redraws | renders | ms | -|---|---|---|---|---| -| toggle edit mode (UI flag) | 0 | 11 | 42 | 1209 | -| coordinate slider, 2 steps | 0 | 12 | 40 | 1618 | -| metadata panel, first open | 4 | 3 | 6 | 989 | -| metadata panel, revisit | 6 | 19 | 72 | 1881 | -| idle (no interaction) | 0 | 0 | 0 | 2125 | +| Scenario | requests | redraws | renders | ms | +| -------------------------- | -------- | ------- | ------- | ---- | +| toggle edit mode (UI flag) | 0 | 11 | 42 | 1209 | +| coordinate slider, 2 steps | 0 | 12 | 40 | 1618 | +| metadata panel, first open | 4 | 3 | 6 | 989 | +| metadata panel, revisit | 6 | 19 | 72 | 1881 | +| idle (no interaction) | 0 | 0 | 0 | 2125 | Three guards fail at this baseline, which is the point of committing them: @@ -32,3 +32,21 @@ Three guards fail at this baseline, which is the point of committing them: The idle scenario passes and must keep passing: it guards against runaway effect loops. + +## After the session request cache (stage 1) + +| Scenario | requests | redraws | renders | ms | +|---|---|---|---|---| +| toggle edit mode (UI flag) | 0 | 11 | 42 | 1251 | +| coordinate slider, 2 steps | 0 | 12 | 40 | 1637 | +| metadata panel, first open | 4 → **2** | 3 | 6 | 1031 | +| metadata panel, revisit | 6 → **1** | 19 → 14 | 72 → 58 | 1802 | +| idle (no interaction) | 0 | 0 | 0 | 2127 | + +Guard 3 now passes: reopening a metadata tab issues **no** `plot_data` request. +The single remaining request on revisit is `/ids_info/array_summary`, which is +a different endpoint and a genuine first-time call for that tab. + +The two cross-panel redraw guards still fail, as expected: they are caused by +the store replacing the whole configuration on every write, which stages 3 and +4 address. Nothing in the fetch layer can fix them. diff --git a/frontend/src/tests/perf/perfHelpers.ts b/frontend/src/tests/perf/perfHelpers.ts index 732522b5..4d97f78f 100644 --- a/frontend/src/tests/perf/perfHelpers.ts +++ b/frontend/src/tests/perf/perfHelpers.ts @@ -15,15 +15,12 @@ export async function resetPerf(): Promise { /** Reads the counters accumulated since the last {@link resetPerf}. */ export async function readPerf(): Promise { - const snapshot = await getDriver().executeScript( - () => - window.__ibexPerf?.snapshot() ?? { - fetchCount: 0, - fetchUrls: [], - renders: {}, - redraws: {}, - }, + const snapshot = await getDriver().executeScript(() => + window.__ibexPerf?.snapshot(), ); + if (!snapshot) { + throw new Error('window.__ibexPerf is not installed'); + } return snapshot as PerfSnapshot; } diff --git a/frontend/src/tests/utils/dataManipulation.ts b/frontend/src/tests/utils/dataManipulation.ts index 3e5d0254..e98dda5a 100644 --- a/frontend/src/tests/utils/dataManipulation.ts +++ b/frontend/src/tests/utils/dataManipulation.ts @@ -59,6 +59,17 @@ export interface GridHandle { * failing test would also intercept every following click. */ export async function resetAppState() { + // The request cache lives for the lifetime of the app, which the specs share. + // Clearing it keeps each spec independent: otherwise an earlier spec warms + // the cache and a later one silently exercises a different code path. + await getDriver().executeScript(() => { + ( + window as Window & { + __ibexPerf?: { clearRequestCache: () => void }; + } + ).__ibexPerf?.clearRequestCache(); + }); + const closeButtons = await getDriver().findElements( By.css('button.mantine-Modal-close'), ); From c5305cc53e4e364451d27bc1f8cdfa4118187fda Mon Sep 17 00:00:00 2001 From: Olivier Hoenen Date: Fri, 18 Sep 2026 12:50:07 +0200 Subject: [PATCH 03/10] perf(plot): stop redrawing Plotly on every render react-plotly.js decides whether to redraw by comparing data, layout and config by reference, so an object literal passed inline forces a full Plotly.react on every render however little changed. Both plot components did exactly that, and the heatmap also rebuilt its trace array with a full copy of the z matrix each time. - config moves to two shared frozen constants (plotConfig.ts); it only ever varied by staticPlot, which has two values. - the heatmap trace array becomes a useMemo, and x/y/z are copied once where they are computed - an effect that only runs when the data or coordinates change - instead of on every render. Plotly keeps and mutates what it is given and those vectors index into the store, so exactly one copy is kept. - isMatrixPlottable built a tf.tensor over the whole array, never disposed, purely to read whether the last dimension was non-zero, and ran in the JSX body of both plot components several times per render. A plain scan computes the same thing allocating nothing, keeping the old semantics including the cases where tf.tensor used to throw (ragged, mixed depth, mixed scalar types, complex pairs). Heatmap2D no longer imports tensorflow at all. - getVectorData deep-cloned every coordinate, including its full data array, only to sort by axeIndex and read valueIndex. It now projects onto those two numbers first. This runs on every slider tick and on every plot's render path. Also fixes two error-reporting bugs that in-flight de-duplication exposed: the HTTP status now travels on the error object rather than in a closure the joining caller never runs - without it an expected 464 on an error-band node lost its status and was reported to the user as "Unable to contact the server" - and handleError no longer notifies twice for one shared rejection. resetAppState dismisses leftover notifications: they expire on a timer, so the suite was relying on being slow enough for that to happen between tests. Measured: toggling a UI flag drops from 11 redraws to 2, two slider steps from 12 to 6, reopening the metadata panel from 19 redraws and 6 requests to 9 and 0. The slider no longer redraws unrelated panels at all, so that guard now passes. Assisted-by: Claude/opus-5 --- .../renderer/components/plot/Heatmap2D.tsx | 148 +++++++++++------- .../renderer/components/plot/SimplePlotly.tsx | 11 +- .../renderer/components/plot/plotConfig.ts | 42 +++++ frontend/src/renderer/utils/fetchData.ts | 42 +++-- frontend/src/renderer/utils/plot.ts | 80 ++++++++-- frontend/src/tests/perf/BASELINE.md | 38 ++++- frontend/src/tests/utils/dataManipulation.ts | 16 ++ 7 files changed, 280 insertions(+), 97 deletions(-) create mode 100644 frontend/src/renderer/components/plot/plotConfig.ts diff --git a/frontend/src/renderer/components/plot/Heatmap2D.tsx b/frontend/src/renderer/components/plot/Heatmap2D.tsx index c7d58d3b..a172d8bc 100644 --- a/frontend/src/renderer/components/plot/Heatmap2D.tsx +++ b/frontend/src/renderer/components/plot/Heatmap2D.tsx @@ -1,7 +1,6 @@ import Plot from 'react-plotly.js'; -import { useCallback, useEffect, useRef, useState } from 'react'; -import * as tf from '@tensorflow/tfjs'; -import { Layout } from 'plotly.js'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { Data, Layout } from 'plotly.js'; import { Axis, AxisData, @@ -23,6 +22,7 @@ import { import classes from './Heatmap2D.module.css'; import { useIbexStore } from '../../stores'; import { countRedraw, countRender } from '../../utils/perf'; +import { getPlotConfig } from './plotConfig'; import { NoDataForURI } from '.'; import { usePlotLayout } from './hooks/usePlotLayout'; import { IconLink } from '@tabler/icons-react'; @@ -283,17 +283,35 @@ export const Heatmap2D = ({ useEffect(() => { if (data3D && selectedPlot) { // Update x, y & z useStates to plot heatmap - setX( - getArrayValueFromDependance(itemDataGrid.coordinates, 0) as number[], - ); - setY( - getArrayValueFromDependance(itemDataGrid.coordinates, 1) as number[], - ); + // These vectors index into the store's arrays, and Plotly keeps and + // mutates whatever it is handed. Copy once here - this effect only runs + // when the data or the coordinates change - rather than copying the whole + // matrix again on every render. + // `getArrayValueFromDependance` returns undefined for an invalid index, + // so copy only when there is something to copy. + const xValues = getArrayValueFromDependance( + itemDataGrid.coordinates, + 0, + ) as number[]; + const yValues = getArrayValueFromDependance( + itemDataGrid.coordinates, + 1, + ) as number[]; + setX(Array.isArray(xValues) ? [...xValues] : xValues); + setY(Array.isArray(yValues) ? [...yValues] : yValues); // Get matrix [[]] needed for z in 3D let zData: AxisData | number | string | Complex = selectedPlot.yData; - const tensor = tf.tensor(zData); - const depthToGoThrough = tensor.shape.length - 2; // shape length - 2 because z need a vector of depth 2 ([][]) + // Depth of the nested array. Replaces a tf.tensor() that was built only + // to read shape.length: it copied the entire matrix and was never + // disposed. + let depth = 0; + let probe: unknown = zData; + while (Array.isArray(probe)) { + depth += 1; + probe = probe[0]; + } + const depthToGoThrough = depth - 2; // z needs a vector of depth 2 ([][]) for (let index = 0; index < depthToGoThrough; index++) { if (Array.isArray(zData)) { zData = @@ -306,7 +324,16 @@ export const Heatmap2D = ({ ]; } } - setZ(zData as (number | string)[][]); + // zData is only a matrix once the loop above has walked down to depth 2; + // for malformed data it can still be a scalar, which must pass through + // untouched exactly as it did before. + setZ( + Array.isArray(zData) + ? (zData as (number | string)[][]).map((row) => + Array.isArray(row) ? [...row] : row, + ) + : (zData as unknown as (number | string)[][]), + ); } }, [data3D, itemDataGrid.coordinates]); @@ -316,6 +343,58 @@ export const Heatmap2D = ({ } }, [data3D, x, y, z]); + /** + * Plotly compares `data` by reference, so this array must keep its identity + * while nothing it depends on changes. `x`, `y` and `z` already hold private + * copies, made where they are computed, so nothing is copied here. + */ + const plotData = useMemo( + () => [ + { + type: forcedPlotType + ? forcedPlotType + : itemDataGrid.selectedPlotMode === 'Heatmap' + ? 'heatmap' + : itemDataGrid.selectedPlotMode === 'Contour' + ? 'contour' + : 'heatmap', + contours: { + coloring: 'lines', + }, + colorscale: selectedPlot?.customPreferences?.colorscale || 'Viridis', + colorbar: { + title: { + text: zAxis?.name + ? `${zAxis?.name} ${(zAxis?.unit && '[' + zAxis.unit + ']') || ''}` + : '', + }, + exponentformat: 'power', + showexponent: 'all', + separatethousands: true, + }, + hovertemplate: + 'x: %{x}
' + 'y: %{y}
' + 'z: %{z:,.6g}', + x, + y, + z, + }, + + // Add geometries in contour type + ...(itemDataGrid?.geometries ?? []), + ], + [ + forcedPlotType, + itemDataGrid.selectedPlotMode, + itemDataGrid?.geometries, + selectedPlot?.customPreferences?.colorscale, + zAxis?.name, + zAxis?.unit, + x, + y, + z, + ], + ); + return ( ' + 'y: %{y}
' + 'z: %{z:,.6g}', - x: [...x], - y: [...y], - z: z.map((row) => [...row]), - }, - - // Add geometries in contour type - ...(itemDataGrid?.geometries ?? []), - ]} - config={{ - autosizable: false, - staticPlot: !itemDataGrid.static, - scrollZoom: true, - displayModeBar: true, - showTips: true, - displaylogo: false, - modeBarButtonsToRemove: ['lasso2d', 'select2d'], - }} + data={plotData} + config={getPlotConfig(itemDataGrid.static)} layout={layoutPlot} onRelayout={handleRelayout} onAfterPlot={handleAfterPlot} diff --git a/frontend/src/renderer/components/plot/SimplePlotly.tsx b/frontend/src/renderer/components/plot/SimplePlotly.tsx index 46072382..c443e93f 100644 --- a/frontend/src/renderer/components/plot/SimplePlotly.tsx +++ b/frontend/src/renderer/components/plot/SimplePlotly.tsx @@ -16,6 +16,7 @@ import { } from '../../utils'; import classes from './SimplePlotly.module.css'; import { countRedraw, countRender } from '../../utils/perf'; +import { getPlotConfig } from './plotConfig'; import { NoDataForURI } from '../plot'; import { usePlotLayout } from './hooks/usePlotLayout'; import { IconLink } from '@tabler/icons-react'; @@ -476,15 +477,7 @@ export const SimplePlotly = ({ = { + ...BASE_CONFIG, + modeBarButtonsToRemove: [...BASE_CONFIG.modeBarButtonsToRemove], + staticPlot: false, +}; + +const STATIC_CONFIG: Partial = { + ...BASE_CONFIG, + modeBarButtonsToRemove: [...BASE_CONFIG.modeBarButtonsToRemove], + staticPlot: true, +}; + +/** + * @param isGridStatic `itemDataGrid.static` — note the plots invert it, a + * "static" grid is the interactive one. + */ +export const getPlotConfig = (isGridStatic: boolean): Partial => + isGridStatic ? INTERACTIVE_CONFIG : STATIC_CONFIG; diff --git a/frontend/src/renderer/utils/fetchData.ts b/frontend/src/renderer/utils/fetchData.ts index cf969162..4f9298db 100644 --- a/frontend/src/renderer/utils/fetchData.ts +++ b/frontend/src/renderer/utils/fetchData.ts @@ -57,6 +57,9 @@ type NotifiedError = Error & { notified?: boolean }; * Tells whether the user has already been notified of this error, so that * callers can skip their own generic notification. */ +/** An `Error` carrying the HTTP status of the response that produced it. */ +type HttpError = Error & { status?: number }; + export const isNotifiedError = (error: unknown): boolean => Boolean((error as NotifiedError)?.notified); @@ -82,15 +85,22 @@ const handleError = (error: unknown, context: string, code?: number) => { } // Notify the user in case of an error including an error message if it is not a 500 error. - showNotification({ - title: !code ? 'Unable to contact the server' : `Error ${code}`, - message: - !code || (code >= 400 && code < 500) ? error.message : 'Internal error', - color: 'red', - }); - // Flag the error so that callers do not stack a second, generic notification - // on top of the detailed message coming from the server. - (error as NotifiedError).notified = true; + // Only once per error object: concurrent callers that share one in-flight + // request also share its rejection, and the user must not be told twice + // about a single failure. + if (!isNotifiedError(error)) { + showNotification({ + title: !code ? 'Unable to contact the server' : `Error ${code}`, + message: + !code || (code >= 400 && code < 500) + ? error.message + : 'Internal error', + color: 'red', + }); + // Flag the error so that callers do not stack a second, generic + // notification on top of the detailed message coming from the server. + (error as NotifiedError).notified = true; + } } console.error(`Error in ${context}:`, error); throw error; @@ -156,9 +166,15 @@ const fetchFromApi = async ( responseStatus = response.status; } const errorData = await response.json(); - throw new Error( + const error: HttpError = new Error( errorData.message || errorData.detail || 'Failed to fetch data', ); + // Carry the status on the error itself. A caller that joined an + // in-flight request never runs this function, so a status kept only + // in the closure above would reach it as undefined - and the rules + // that suppress expected 404/464 error-band failures would not fire. + error.status = response.status; + throw error; } return response.text(); @@ -172,7 +188,11 @@ const fetchFromApi = async ( console.error(`Timeout after ${timeout}ms: fetchFromApi(${endpoint}).`); throw error; } else { - handleError(error, `fetchFromApi(${endpoint})`, responseStatus); + handleError( + error, + `fetchFromApi(${endpoint})`, + (error as HttpError)?.status ?? responseStatus, + ); } } }; diff --git a/frontend/src/renderer/utils/plot.ts b/frontend/src/renderer/utils/plot.ts index 69b90de6..f6aa5bb2 100644 --- a/frontend/src/renderer/utils/plot.ts +++ b/frontend/src/renderer/utils/plot.ts @@ -2155,12 +2155,20 @@ export const reapplyAxisOrder = async ( export function getVectorData(coordinates: Coordinates[], yData: AxisData) { const coordinatesLength: number = coordinates.length; - // Extract only matrix indexes - const matrixIndexes = structuredClone(coordinates) + // Extract only matrix indexes. + // Only `axeIndex` and `valueIndex` are read, so project onto those two + // numbers before sorting: cloning the coordinates would deep-copy every + // coordinate's full `data` array, and this runs on every slider tick and on + // the render path of every plot. + const matrixIndexes = coordinates + .map((coord: Coordinates) => ({ + axeIndex: coord.axeIndex, + valueIndex: coord.valueIndex, + })) .sort(compareByAxeIndex) .reverse() - .filter((coord: Coordinates) => coord.axeIndex !== 0) - .map((coord: Coordinates) => coord.valueIndex); + .filter((coord) => coord.axeIndex !== 0) + .map((coord) => coord.valueIndex); // Retrieve vector to plot /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -2203,7 +2211,10 @@ export function getErrorYVectors(plot: DataPlotly, coordinates: Coordinates[]) { * @param b The second Coordinates object. * @returns A negative number if a's axeIndex is less than b's, a positive number if greater, or 0 if equal. */ -export function compareByAxeIndex(a: Coordinates, b: Coordinates) { +export function compareByAxeIndex( + a: { axeIndex: number }, + b: { axeIndex: number }, +) { if (a.axeIndex < b.axeIndex) { return -1; } else if (a.axeIndex > b.axeIndex) { @@ -2256,17 +2267,56 @@ export function hasAtLeastOneValidValue(arr: AxisData): boolean { export function isMatrixPlottable(value: AxisData): boolean { if (value === undefined) return false; - try { - const tensor = tf.tensor(value); - const shape = tensor.shape; - const lastDim = shape[shape.length - 1]; - - // Check if matrix is not empty & get at least one valide value - return lastDim !== 0 && hasAtLeastOneValidValue(value); - } catch { - // If tensor fails (irregular shape, etc.) - return false; + const lastDim = getLastDimLength(value); + + // Check if matrix is not empty & get at least one valide value + return lastDim !== null && lastDim !== 0 && hasAtLeastOneValidValue(value); +} + +/** + * @description Length of the innermost dimension of a rectangular nested array, + * or `null` when the input is ragged, mixed-depth or holds non-plottable + * leaves (complex `{r, i}` pairs, objects). + * + * This replaces a `tf.tensor(value)` whose only outputs were "does it build" + * and "what is the last dimension". Building a tensor copied the whole array + * into a typed array on every call — and it was called from the render body of + * both plot components, several times per render, without ever being disposed. + * A plain scan allocates nothing and keeps exactly the same semantics, + * including returning `null` where `tf.tensor` used to throw. + */ +function getLastDimLength(value: AxisData): number | null { + if (!Array.isArray(value)) return null; + if (value.length === 0) return 0; + + const first = value[0]; + if (!Array.isArray(first)) { + // Innermost level. tf.tensor accepted only numbers or only strings, and + // threw on anything else (including complex {r, i} pairs) or on a mix of + // the two, so reproduce both rules. + let leafType: 'number' | 'string' | null = null; + for (const leaf of value as unknown[]) { + if (Array.isArray(leaf)) return null; // mixed depth + if (leaf === null || leaf === undefined) continue; // filled in later + const type = typeof leaf; + if (type !== 'number' && type !== 'string') return null; + if (leafType === null) leafType = type; + else if (leafType !== type) return null; // mixed scalar types + } + return value.length; + } + + // Nested level: every child must be an array of the same, consistent shape. + let lastDim: number | null = null; + for (const child of value as AxisData[]) { + if (!Array.isArray(child)) return null; // mixed depth + if (child.length !== (first as unknown[]).length) return null; // ragged + const childLastDim = getLastDimLength(child); + if (childLastDim === null) return null; + if (lastDim === null) lastDim = childLastDim; + else if (lastDim !== childLastDim) return null; } + return lastDim; } /** diff --git a/frontend/src/tests/perf/BASELINE.md b/frontend/src/tests/perf/BASELINE.md index d32d2d37..ced6aea4 100644 --- a/frontend/src/tests/perf/BASELINE.md +++ b/frontend/src/tests/perf/BASELINE.md @@ -35,13 +35,13 @@ effect loops. ## After the session request cache (stage 1) -| Scenario | requests | redraws | renders | ms | -|---|---|---|---|---| -| toggle edit mode (UI flag) | 0 | 11 | 42 | 1251 | -| coordinate slider, 2 steps | 0 | 12 | 40 | 1637 | -| metadata panel, first open | 4 → **2** | 3 | 6 | 1031 | -| metadata panel, revisit | 6 → **1** | 19 → 14 | 72 → 58 | 1802 | -| idle (no interaction) | 0 | 0 | 0 | 2127 | +| Scenario | requests | redraws | renders | ms | +| -------------------------- | --------- | ------- | ------- | ---- | +| toggle edit mode (UI flag) | 0 | 11 | 42 | 1251 | +| coordinate slider, 2 steps | 0 | 12 | 40 | 1637 | +| metadata panel, first open | 4 → **2** | 3 | 6 | 1031 | +| metadata panel, revisit | 6 → **1** | 19 → 14 | 72 → 58 | 1802 | +| idle (no interaction) | 0 | 0 | 0 | 2127 | Guard 3 now passes: reopening a metadata tab issues **no** `plot_data` request. The single remaining request on revisit is `/ids_info/array_summary`, which is @@ -50,3 +50,27 @@ a different endpoint and a genuine first-time call for that tab. The two cross-panel redraw guards still fail, as expected: they are caused by the store replacing the whole configuration on every write, which stages 3 and 4 address. Nothing in the fetch layer can fix them. + +## After the render-path fixes (stage 2) + +| Scenario | requests | redraws | renders | ms | +|---|---|---|---|---| +| toggle edit mode (UI flag) | 0 | **2** | 42 | 1145 | +| coordinate slider, 2 steps | 0 | **6** | 40 | 1546 | +| metadata panel, first open | 2 | 3 | 6 | 1106 | +| metadata panel, revisit | **0** | **9** | 52 | 1679 | +| idle (no interaction) | 0 | 0 | 0 | 2130 | + +Three of the four guards now pass. Redraws against the original baseline: +toggling a UI flag 11 → 2, two slider steps 12 → 6, reopening the metadata +panel 19 → 9, and its backend requests 6 → 0. + +The slider guard passes: stepping the heatmap's time slider no longer redraws +the 1-D panel at all. + +One guard still fails, and it is the honest remainder: toggling one panel's +edit flag still causes **1** redraw of the untouched heatmap panel (it was 6). +That last one cannot be fixed from the render path — the store replaces the +whole configuration on every write, so the sibling panel genuinely receives new +props. Stages 3 and 4 (the data/config split and selector subscriptions) are +what close it. diff --git a/frontend/src/tests/utils/dataManipulation.ts b/frontend/src/tests/utils/dataManipulation.ts index e98dda5a..e88d3dd9 100644 --- a/frontend/src/tests/utils/dataManipulation.ts +++ b/frontend/src/tests/utils/dataManipulation.ts @@ -81,6 +81,22 @@ export async function resetAppState() { } } + // Dismiss notifications left by the previous test. They expire on their own + // after a few seconds, so the suite used to rely on being slow enough for + // that to happen between tests - which stops being true as the app gets + // faster, and makes a test that counts notifications see the previous one's. + await getDriver().executeScript(() => { + document + .querySelectorAll('.mantine-Notification-closeButton') + .forEach((button) => button.click()); + }); + await getDriver().wait(async () => { + const remaining = await getDriver().executeScript( + () => document.querySelectorAll('.mantine-Notification-root').length, + ); + return remaining === 0; + }, 10000); + const isEmpty = async () => { const state = await getTestState(); return (state?.configurations?.length ?? 0) === 0 && !state?.active; From 702df5147735705925e39ff1a903b3e53863623f Mon Sep 17 00:00:00 2001 From: Olivier Hoenen Date: Fri, 18 Sep 2026 13:03:48 +0200 Subject: [PATCH 04/10] perf(grid): stop panels re-rendering on unrelated store writes The plot components and the grid panel only ever wrote to the store, yet each subscribed to all of it, so any change anywhere re-rendered every panel and handed Plotly new props. - SimplePlotly and Heatmap2D no longer subscribe at all: both read useIbexStore.getState() inside the handlers and effects that write, which is the pattern handleDeleteGrid already used. This also removes a structuredClone of every plot's data that ran just to change a title. - GridLayoutPlot subscribes only to whether any URI is selected - the one value it reads while rendering - and reads the rest at call time. Its three panel handlers lose their [active] dependency and become stable for the component's lifetime. - GridLayoutPlot is memoized. That only works because handleEditGrid now keeps the identity of grids it is not changing, instead of rebuilding every grid object to set two booleans that were already false. Measured: toggling a UI flag drops from 42 component renders to 14, and two slider steps from 40 to 28. The untouched panel still redraws once per toggle, down from 6. Closing that needs the data/config store split: the configuration is still replaced whole on every write, so VisualizationPlot re-renders and react-grid-layout clones every child on the way through. Assisted-by: Claude/opus-5 --- .../components/grid/GridLayoutPlot.tsx | 177 ++++++++++-------- .../renderer/components/plot/Heatmap2D.tsx | 10 +- .../renderer/components/plot/SimplePlotly.tsx | 20 +- frontend/src/tests/perf/BASELINE.md | 36 +++- 4 files changed, 144 insertions(+), 99 deletions(-) diff --git a/frontend/src/renderer/components/grid/GridLayoutPlot.tsx b/frontend/src/renderer/components/grid/GridLayoutPlot.tsx index a49e4f8e..510f798c 100644 --- a/frontend/src/renderer/components/grid/GridLayoutPlot.tsx +++ b/frontend/src/renderer/components/grid/GridLayoutPlot.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useState } from 'react'; +import { memo, useCallback, useEffect, useState } from 'react'; import { Axis, Configuration, @@ -26,12 +26,23 @@ import { import { MetaDataInfos } from '../../pages/visualization/VisualizationMetaData'; import { HoverButtons } from './HoverButtons'; -export const GridLayoutPlot = ({ +/** + * One plot panel. + * + * Memoized on its props: `data` keeps its identity while that grid is + * unchanged (the store writers preserve untouched grids), and the other two are + * numbers. Without this, a store write anywhere re-renders every panel through + * the parent's children array, and each re-render hands Plotly new props. + */ +export const GridLayoutPlot = memo(function GridLayoutPlot({ data, colWidth, rowHeight, -}: GridLayoutPlotProps) => { - const { active, updatedConfiguration } = useIbexStore(); +}: GridLayoutPlotProps) { + // Only `dataURI.length` is read while rendering; everything else is read at + // call time inside the handlers. Subscribing to the whole store here made + // every panel re-render - and redraw - on any change anywhere. + const hasDataURI = useIbexStore((state) => state.active.dataURI.length > 0); countRender(`GridLayoutPlot:${data.i}`); const [heightGrid, setHeightGrid] = useState( data.h * rowHeight + (23 * (data.h * rowHeight)) / 100, @@ -55,6 +66,9 @@ export const GridLayoutPlot = ({ coordinate: Coordinates, valueIndex: number, ) => { + // Read at call time rather than from a subscription: a slider tick must not + // depend on this component having re-rendered for the latest state. + const { active, updatedConfiguration } = useIbexStore.getState(); // Check if the coordinate has a target const lastTargetLastName = getLastIndexedField(coordinate.target); if (!lastTargetLastName) @@ -282,120 +296,125 @@ export const GridLayoutPlot = ({ /** * Handle edit grid event */ - const handleEditGrid = useCallback( - (id: string) => { - const { active, updatedConfiguration } = useIbexStore.getState(); + const handleEditGrid = useCallback((id: string) => { + const { active, updatedConfiguration } = useIbexStore.getState(); - const findPlot = active.dataPlot.find((item) => item.i === id); - if (!findPlot) return; + const findPlot = active.dataPlot.find((item) => item.i === id); + if (!findPlot) return; - const updatedDataPlot = active.dataPlot.map((item) => - item.i === id - ? { ...item, isEditing: !item.isEditing, static: !item.isEditing } - : { ...item, isEditing: false, static: false }, - ); + const updatedDataPlot = active.dataPlot.map((item) => { + if (item.i === id) { + return { + ...item, + isEditing: !item.isEditing, + static: !item.isEditing, + }; + } + // Keep the identity of grids that are not changing. Rebuilding them + // unconditionally handed every other panel a new object, which is what + // made an edit on one panel redraw all the others. + if (!item.isEditing && !item.static) return item; + return { ...item, isEditing: false, static: false }; + }); + + // Check from tree selected plots (all plots used in dataGrid) + const checkedNodeURI: URITreeNodeData[] = !findPlot.isEditing + ? findPlot.plot.map((item) => ({ + uri: normalizeIndices(item.nodeUri), + name: item.labelUri, + type: findPlot.dataType, + is_geometry_node: findPlot.is_geometry_node, + })) + : []; - // Check from tree selected plots (all plots used in dataGrid) - const checkedNodeURI: URITreeNodeData[] = !findPlot.isEditing - ? findPlot.plot.map((item) => ({ - uri: normalizeIndices(item.nodeUri), - name: item.labelUri, + if (checkedNodeURI.length) { + for (const plot of findPlot.plot) { + if (!plot.error_bands) { + continue; + } + + for (const error_band of plot.error_bands) { + const newCheckedNode = { + name: plot.labelUri, + uri: normalizeIndices(error_band.path), type: findPlot.dataType, is_geometry_node: findPlot.is_geometry_node, - })) - : []; - - if (checkedNodeURI.length) { - for (const plot of findPlot.plot) { - if (!plot.error_bands) { - continue; + }; + const exists = checkedNodeURI.some( + (node) => + node.name === newCheckedNode.name && + node.uri === newCheckedNode.uri, + ); + if (!exists) { + // Check from tree selected error bands to plot + checkedNodeURI.push(newCheckedNode); } + } + } - for (const error_band of plot.error_bands) { + if (findPlot?.geometries) { + // Check geometries in tree + for (const geometry of findPlot.geometries) { + for (const uriOfGeo of geometry.nodeUris) { const newCheckedNode = { - name: plot.labelUri, - uri: normalizeIndices(error_band.path), - type: findPlot.dataType, - is_geometry_node: findPlot.is_geometry_node, - }; + name: findPlot.plot[0].labelUri, + uri: normalizeIndices(uriOfGeo), + type: NodeInfoTypeEnum.FLOAT, + is_geometry_node: true, + } as URITreeNodeData; const exists = checkedNodeURI.some( (node) => node.name === newCheckedNode.name && node.uri === newCheckedNode.uri, ); if (!exists) { - // Check from tree selected error bands to plot checkedNodeURI.push(newCheckedNode); } } } - - if (findPlot?.geometries) { - // Check geometries in tree - for (const geometry of findPlot.geometries) { - for (const uriOfGeo of geometry.nodeUris) { - const newCheckedNode = { - name: findPlot.plot[0].labelUri, - uri: normalizeIndices(uriOfGeo), - type: NodeInfoTypeEnum.FLOAT, - is_geometry_node: true, - } as URITreeNodeData; - const exists = checkedNodeURI.some( - (node) => - node.name === newCheckedNode.name && - node.uri === newCheckedNode.uri, - ); - if (!exists) { - checkedNodeURI.push(newCheckedNode); - } - } - } - } } + } - const updatedActive: Configuration = { - ...active, - saved: false, - dataPlot: updatedDataPlot, - checkedNodeURI: checkedNodeURI, - }; + const updatedActive: Configuration = { + ...active, + saved: false, + dataPlot: updatedDataPlot, + checkedNodeURI: checkedNodeURI, + }; - updatedConfiguration(updatedActive); - }, - [active], - ); + updatedConfiguration(updatedActive); + }, []); /** * Inspect metadata of plot */ - const handleInspectMetadata = useCallback( - (id: string) => { - const updatedActive: Configuration = { - ...active, - metadataGridLayout: id, - }; - updatedConfiguration(updatedActive); - }, - [active], - ); + const handleInspectMetadata = useCallback((id: string) => { + const { active, updatedConfiguration } = useIbexStore.getState(); + const updatedActive: Configuration = { + ...active, + metadataGridLayout: id, + }; + updatedConfiguration(updatedActive); + }, []); /** * Customize plot */ const handleCustomization = useCallback( (id: string, typeOfEdition: CustomizedGridType) => { + const { active, updatedConfiguration } = useIbexStore.getState(); const updatedActive: Configuration = { ...active, customizedGridLayout: { id: id, type: typeOfEdition }, }; updatedConfiguration(updatedActive); }, - [active], + [], ); return ( - {active.dataURI.length > 0 && ( + {hasDataURI && ( )} - {!(active.dataURI.length > 0) ? ( + {!hasDataURI ? ( // Control when loading a template without selecting URIs
Current configuration has no data. Please, select URIs. @@ -454,4 +473,4 @@ export const GridLayoutPlot = ({ )} ); -}; +}); diff --git a/frontend/src/renderer/components/plot/Heatmap2D.tsx b/frontend/src/renderer/components/plot/Heatmap2D.tsx index a172d8bc..f4ae33a7 100644 --- a/frontend/src/renderer/components/plot/Heatmap2D.tsx +++ b/frontend/src/renderer/components/plot/Heatmap2D.tsx @@ -49,7 +49,6 @@ export const Heatmap2D = ({ forcedPlotType, handleUpdateCoordinate, }: Heatmap2DProps) => { - const { active, updatedConfiguration } = useIbexStore(); countRender(`Heatmap2D:${itemDataGrid.i}`); const handleAfterPlot = useCallback( () => countRedraw(itemDataGrid.i), @@ -140,6 +139,11 @@ export const Heatmap2D = ({ * Update the layout title & dataPlot configuration when editing title */ useEffect(() => { + // The store is read here rather than subscribed to: this component only + // ever writes to it, and subscribing would re-render - and so redraw + // Plotly - on every unrelated change elsewhere in the configuration. + const { active, updatedConfiguration } = useIbexStore.getState(); + if (!active.dataPlot.find((element) => element.isEditing)) { // Update active dataplot title only when editing (to prevent from updating in customization) return; @@ -445,8 +449,8 @@ export const Heatmap2D = ({ ).axeIndex, targetAxis === 'x' ? 0 : 1, false, - active, - updatedConfiguration, + useIbexStore.getState().active, + useIbexStore.getState().updatedConfiguration, ) } size="xs" diff --git a/frontend/src/renderer/components/plot/SimplePlotly.tsx b/frontend/src/renderer/components/plot/SimplePlotly.tsx index c443e93f..678ab0e7 100644 --- a/frontend/src/renderer/components/plot/SimplePlotly.tsx +++ b/frontend/src/renderer/components/plot/SimplePlotly.tsx @@ -53,7 +53,6 @@ export const SimplePlotly = ({ ); }, [itemDataGrid.plot, itemDataGrid.coordinates]); const coordsUsedInAxes: 1 | 2 = 1; - const { active, updatedConfiguration } = useIbexStore(); const SELECT_AXIS_HEIGHT = 40; // Height of the select axis component const [layoutPlot, setLayoutPlot] = useState>({ xaxis: { @@ -210,13 +209,14 @@ export const SimplePlotly = ({ return; } - // Update title only if is editing - const updatedDataPlot: DataGridPlot[] = structuredClone(active.dataPlot); - for (const dataPlot of updatedDataPlot) { - if (dataPlot.i === itemDataGrid.i) { - dataPlot.title = title; - } - } + // Update title only if is editing. The store is read here rather than + // subscribed to: this component only ever writes to it, and subscribing + // would re-render - and so redraw Plotly - on every unrelated change. + const { active, updatedConfiguration } = useIbexStore.getState(); + + const updatedDataPlot: DataGridPlot[] = active.dataPlot.map((dataPlot) => + dataPlot.i === itemDataGrid.i ? { ...dataPlot, title } : dataPlot, + ); const newActive: Configuration = { ...active, @@ -391,8 +391,8 @@ export const SimplePlotly = ({ ).axeIndex, 0, // axeIndex of x is always 0 false, - active, - updatedConfiguration, + useIbexStore.getState().active, + useIbexStore.getState().updatedConfiguration, ) } size="xs" diff --git a/frontend/src/tests/perf/BASELINE.md b/frontend/src/tests/perf/BASELINE.md index ced6aea4..865f0c8a 100644 --- a/frontend/src/tests/perf/BASELINE.md +++ b/frontend/src/tests/perf/BASELINE.md @@ -53,13 +53,13 @@ the store replacing the whole configuration on every write, which stages 3 and ## After the render-path fixes (stage 2) -| Scenario | requests | redraws | renders | ms | -|---|---|---|---|---| -| toggle edit mode (UI flag) | 0 | **2** | 42 | 1145 | -| coordinate slider, 2 steps | 0 | **6** | 40 | 1546 | -| metadata panel, first open | 2 | 3 | 6 | 1106 | -| metadata panel, revisit | **0** | **9** | 52 | 1679 | -| idle (no interaction) | 0 | 0 | 0 | 2130 | +| Scenario | requests | redraws | renders | ms | +| -------------------------- | -------- | ------- | ------- | ---- | +| toggle edit mode (UI flag) | 0 | **2** | 42 | 1145 | +| coordinate slider, 2 steps | 0 | **6** | 40 | 1546 | +| metadata panel, first open | 2 | 3 | 6 | 1106 | +| metadata panel, revisit | **0** | **9** | 52 | 1679 | +| idle (no interaction) | 0 | 0 | 0 | 2130 | Three of the four guards now pass. Redraws against the original baseline: toggling a UI flag 11 → 2, two slider steps 12 → 6, reopening the metadata @@ -74,3 +74,25 @@ That last one cannot be fixed from the render path — the store replaces the whole configuration on every write, so the sibling panel genuinely receives new props. Stages 3 and 4 (the data/config split and selector subscriptions) are what close it. + +## After narrowing subscriptions and memoizing the panel (stage 3, partial) + +| Scenario | requests | redraws | renders | ms | +|---|---|---|---|---| +| toggle edit mode (UI flag) | 0 | 2 | **14** | 1105 | +| coordinate slider, 2 steps | 0 | 6 | **28** | 1547 | +| metadata panel, first open | 2 | 3 | 6 | 993 | +| metadata panel, revisit | 0 | 9 | 52 | 1650 | +| idle (no interaction) | 0 | 0 | 0 | 2126 | + +Component renders against the original baseline: toggling a UI flag 42 → 14, +two slider steps 40 → 28. + +## Where the last guard stands + +`toggle edit mode` still redraws the untouched heatmap panel **once** (it was 6 +at the baseline). Closing it needs the full data/config store split: the plot +components no longer subscribe to the store and the panel is memoized, but the +configuration object is still replaced wholesale on every write, so +`VisualizationPlot` — which does subscribe — re-renders and rebuilds the grid, +and react-grid-layout clones its children on the way through. From b7aeeea0463e65cc8e773cd8aac0ec3a80acb6a3 Mon Sep 17 00:00:00 2001 From: Olivier Hoenen Date: Fri, 18 Sep 2026 15:11:57 +0200 Subject: [PATCH 05/10] perf(grid): stop a no-op layout report rebuilding every panel Entering edit mode sets `static` on one grid, which changes the layout react-grid-layout derives from its children, so RGL reports onLayoutChange. handleUpdateLayout then rebuilt every grid object from that report - including the ones that had not moved - which handed the memoized panels new props and redrew the untouched heatmap. - Grids whose x/y/w/h/static match the report keep their identity. - A report that changes nothing writes nothing, so a layout event no longer flags the configuration as unsaved on its own. - minH/minW are derived the same way the data-grid prop derives them, instead of being hard-coded to a value that only suited grids with coordinates. - handleUpdateLayout reads the store at call time and loses its [active] dependency, the pattern the panel handlers already use. This closes the last benchmark guard: toggling one panel's edit flag now redraws only that panel (the untouched heatmap goes 6 -> 0 against the original baseline) and costs 4 component renders instead of 42. All four scenarios in `npm run test:perf` pass, and the e2e suite is green. The container deliberately subscribes to `active` rather than `active.dataPlot`: handleNewPlot pushes a new grid into that array in place, so its identity does not change when a panel is added. Assisted-by: Claude/opus-5 --- .../pages/visualization/VisualizationPlot.tsx | 158 +++++++++++------- frontend/src/tests/perf/BASELINE.md | 27 +++ 2 files changed, 121 insertions(+), 64 deletions(-) diff --git a/frontend/src/renderer/pages/visualization/VisualizationPlot.tsx b/frontend/src/renderer/pages/visualization/VisualizationPlot.tsx index 609cf327..27812330 100644 --- a/frontend/src/renderer/pages/visualization/VisualizationPlot.tsx +++ b/frontend/src/renderer/pages/visualization/VisualizationPlot.tsx @@ -10,11 +10,22 @@ interface VisualizationPlotProps { height?: string; } +/** Minimum grid size, kept identical to what the `data-grid` prop below asks for. */ +const minHeightOf = (plotData: DataGridPlot) => + plotData.coordinates.length > 0 ? 12 : 8; +const minWidthOf = (plotData: DataGridPlot) => + plotData.coordinates.length > 0 ? 6 : 4; + export const VisualizationPlot = ({ extended, height, }: VisualizationPlotProps) => { - const { active, updatedConfiguration } = useIbexStore(); + // Subscribe to the configuration, not to `active.dataPlot`: `handleNewPlot` + // (utils/plot.ts) pushes a new grid into that array in place, so its identity + // does not change when a panel is added and a narrower selector would never + // fire. Every writer does replace `active` itself. + const active = useIbexStore((state) => state.active); + const dataPlot = active?.dataPlot ?? []; const scrollAreaRef = useRef(null); const [dragEnabled, setDragEnabled] = useState(true); @@ -48,37 +59,58 @@ export const VisualizationPlot = ({ /** * Handle update grid layout + * + * react-grid-layout reports the whole layout whenever any of it changes - + * including when a panel only toggles `static` on entering edit mode. Grids + * that did not move keep their identity so the memoized panels are not + * re-rendered, and a report that changes nothing writes nothing at all + * (which also stops it flagging the configuration as unsaved). */ - const handleUpdateLayout = useCallback( - (updatedLayouts: Layout[]) => { - const updatedDataPlot: DataGridPlot[] = active.dataPlot.map( - (item: DataGridPlot) => { - const findUpdatedLayout = updatedLayouts.find( - (layout) => layout.i === item.i, - ); - - if (findUpdatedLayout) { - return { - ...item, - ...findUpdatedLayout, - minH: 12, - minW: 6, - }; - } + const handleUpdateLayout = useCallback((updatedLayouts: Layout[]) => { + const { active, updatedConfiguration } = useIbexStore.getState(); + let changed = false; + + const updatedDataPlot: DataGridPlot[] = active.dataPlot.map( + (item: DataGridPlot) => { + const findUpdatedLayout = updatedLayouts.find( + (layout) => layout.i === item.i, + ); + if (!findUpdatedLayout) return item; + + const minH = minHeightOf(item); + const minW = minWidthOf(item); + if ( + item.x === findUpdatedLayout.x && + item.y === findUpdatedLayout.y && + item.w === findUpdatedLayout.w && + item.h === findUpdatedLayout.h && + item.static === findUpdatedLayout.static && + item.minH === minH && + item.minW === minW + ) { return item; - }, - ); - - const newActive: Configuration = { - ...active, - saved: false, - dataPlot: updatedDataPlot, - }; - - updatedConfiguration(newActive); - }, - [active], - ); + } + + changed = true; + return { + ...item, + ...findUpdatedLayout, + minH, + minW, + }; + }, + ); + + if (!changed) return; + + const newActive: Configuration = { + ...active, + saved: false, + dataPlot: updatedDataPlot, + }; + + updatedConfiguration(newActive); + }, []); /* * Scroll to the bottom of the scroll area when new data is added or removed @@ -91,9 +123,9 @@ export const VisualizationPlot = ({ behavior: 'smooth', }); } - }, [active.dataPlot.length]); + }, [dataPlot.length]); - return active.dataPlot.length > 0 ? ( + return dataPlot.length > 0 ? ( <> handleUpdateLayout(layout)} > - {active.dataPlot.map((plotData: DataGridPlot) => { - return ( - 0 ? 12 : 8, - minW: plotData.coordinates.length > 0 ? 6 : 4, - }} - style={{ - width: '100%', - height: '100%', - display: 'flex', - flexDirection: 'column', - boxSizing: 'border-box', - }} - > - - - ); - })} + {dataPlot.map((plotData: DataGridPlot) => ( + + + + ))} diff --git a/frontend/src/tests/perf/BASELINE.md b/frontend/src/tests/perf/BASELINE.md index 865f0c8a..1030a867 100644 --- a/frontend/src/tests/perf/BASELINE.md +++ b/frontend/src/tests/perf/BASELINE.md @@ -96,3 +96,30 @@ components no longer subscribe to the store and the panel is memoized, but the configuration object is still replaced wholesale on every write, so `VisualizationPlot` — which does subscribe — re-renders and rebuilds the grid, and react-grid-layout clones its children on the way through. + +## After fixing the layout write cycle (stage 4) + +| Scenario | requests | redraws | renders | ms | +| -------------------------- | -------- | ------- | ------- | ---- | +| toggle edit mode (UI flag) | 0 | **1** | **4** | 1015 | +| coordinate slider, 2 steps | 0 | 6 | 28 | 1583 | +| metadata panel, first open | 2 | 3 | 6 | 1024 | +| metadata panel, revisit | 0 | 9 | 44 | 1687 | +| idle (no interaction) | 0 | 0 | 0 | 2126 | + +**All four guards pass.** Toggling one panel's edit flag now redraws only the +panel that was toggled; the untouched heatmap redraws 0 times, down from 6 at +the original baseline. Component renders for that scenario: 42 → 4. + +The cause was not the store after all. Entering edit mode sets `static` on the +grid, which changes the layout react-grid-layout derives from its children, so +RGL reports `onLayoutChange` — and `VisualizationPlot.handleUpdateLayout` +rebuilt *every* grid object from that report, which defeated the `memo` added in +the previous stage. It now keeps the identity of grids that did not move and +writes nothing at all when the report changes nothing (which also stops the +configuration being flagged unsaved by a no-op). + +Note for the store split: `VisualizationPlot` subscribes to `active`, not to +`active.dataPlot`, because `handleNewPlot` (`utils/plot.ts:235`) pushes a new +grid into that array **in place**. A narrower selector never fires when a panel +is added, and memoizing the RGL children on it renders an empty canvas. From e81ec585b226507ca42579d2b318d0f866c5d639 Mon Sep 17 00:00:00 2001 From: Olivier Hoenen Date: Fri, 18 Sep 2026 15:14:37 +0200 Subject: [PATCH 06/10] fix(ids_info): stop node_info building a tree nobody asked for node_info passed `show_error_bars` positionally into get_node_info, whose second parameter is `recursive`. With the "see error bars" preference on, every call therefore walked and serialized the metadata of the entire subtree - which NodeInfoResponse then discarded, since NodeInfoChildModel has no `children` field - and the error bar filter never ran at all. The filter being skipped had no visible effect (the recursive branch returns every child, which is what the flag asks for), so the cost was the only symptom: on the disruption fixture, node_info on summary:0 takes 0.20 s with the flag on against 0.13 s with it off, while on equilibrium:0 the difference is lost in the filled-path scan that dominates there. - Pass the flag by keyword. - Forward it through _jsonify_metadata's recursive branch, which dropped it. - Guard the delegation in a test: the response looks identical either way, so assert on the arguments the endpoint hands the service. Assisted-by: Claude/opus-5 --- .../ibex/data_source/imas_python_source.py | 2 +- backend/ibex/endpoints/ids_info.py | 2 +- backend/tests/test_ids_info_endpoints.py | 45 +++++++++++++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/backend/ibex/data_source/imas_python_source.py b/backend/ibex/data_source/imas_python_source.py index c102ffa0..01bd9823 100644 --- a/backend/ibex/data_source/imas_python_source.py +++ b/backend/ibex/data_source/imas_python_source.py @@ -172,7 +172,7 @@ def _jsonify_metadata(self, metadata: IDSMetadata, recursive: bool = False, show result["is_geometry_node"] = self._is_geometry_node(metadata) if recursive: - result["children"] = [self._jsonify_metadata(child, recursive) for child in metadata] + result["children"] = [self._jsonify_metadata(child, recursive, show_error_bars) for child in metadata] else: result["children"] = [ { diff --git a/backend/ibex/endpoints/ids_info.py b/backend/ibex/endpoints/ids_info.py index d5110641..d40f40a9 100644 --- a/backend/ibex/endpoints/ids_info.py +++ b/backend/ibex/endpoints/ids_info.py @@ -44,7 +44,7 @@ def node_info(uri: str, show_error_bars: bool = False) -> dict: :return: JSON response """ - return ibex_service.get_node_info(uri.strip(), show_error_bars) + return ibex_service.get_node_info(uri.strip(), show_error_bars=show_error_bars) @router.get( diff --git a/backend/tests/test_ids_info_endpoints.py b/backend/tests/test_ids_info_endpoints.py index bfdd5492..40c22f33 100644 --- a/backend/tests/test_ids_info_endpoints.py +++ b/backend/tests/test_ids_info_endpoints.py @@ -4,6 +4,8 @@ from packaging.version import Version from pytest_unordered import unordered +from ibex.endpoints import ids_info + def test_node_info_coordinates(entry_path): test_dict = { @@ -247,3 +249,46 @@ def test_show_error_bars_option(entry_path): assert "r0_error_upper" in [child["name"] for child in response.json()["children"]], ( "Error bars filtering failed. 'r0_error_upper' nodes was not returned, but it should be." ) + + +def test_node_info_never_requests_the_recursive_tree(entry_path, monkeypatch): + """`show_error_bars` used to be passed positionally into `get_node_info`, whose second + parameter is `recursive`. Switching the option on therefore built the metadata of the whole + subtree - which the response model then discarded - and never applied the error bar filter. + The response looks the same either way, so guard the delegation itself. + """ + calls = [] + + def spy(uri, recursive=False, show_error_bars=False): + calls.append({"uri": uri, "recursive": recursive, "show_error_bars": show_error_bars}) + return { + "name": "vacuum_toroidal_field", + "type": "structure", + "ndim": 0, + "shape": [], + "is_geometry_node": False, + "children": [], + "coordinates": [], + } + + monkeypatch.setattr(ids_info.ibex_service, "get_node_info", spy) + + for show_error_bars in (True, False): + parameters = { + "uri": f"imas:hdf5?path={entry_path}#core_profiles/vacuum_toroidal_field", + "show_error_bars": show_error_bars, + } + assert pytest.test_client.get("/ids_info/node_info", params=parameters).status_code == 200 + + assert calls == [ + { + "uri": f"imas:hdf5?path={entry_path}#core_profiles/vacuum_toroidal_field", + "recursive": False, + "show_error_bars": True, + }, + { + "uri": f"imas:hdf5?path={entry_path}#core_profiles/vacuum_toroidal_field", + "recursive": False, + "show_error_bars": False, + }, + ] From 1b687088953a0f28e42a5143a2877b0fe1a46d41 Mon Sep 17 00:00:00 2001 From: Olivier Hoenen Date: Fri, 18 Sep 2026 15:26:43 +0200 Subject: [PATCH 07/10] perf(plot): derive the Plotly layout instead of assembling it in effects react-plotly.js compares `layout` by reference, so each of the fifteen effects that called setLayoutPlot handed it a new identity and cost a redraw as a panel appeared: seven in SimplePlotly, five in usePlotLayout, six in Heatmap2D. Each component now derives its whole layout in one useMemo: - usePlotLayout returns a memoized {xaxis, yaxis, yaxis2} fragment (grid display and axis types) instead of taking a setter. Its rule that string x data forces a category axis, and that a category axis returns to linear once the data is numeric, is unchanged - but it no longer assigns to itemDataGrid.xAxisData in place. The configured type is persisted and read back by the customization panel, so it is written through updatedConfiguration, guarded so it writes only on the two transitions the effect handled. - SimplePlotly derives title, height, width, the axis titles and the whole y2 block. Its `dataEntries` state existed only to gate those effects and is gone; the titles derive from itemDataGrid.plot directly. - Heatmap2D derives the same, plus the 1:1 ratio (which no longer needs a state mirror of forceXyRatio nor a structuredClone of the layout to patch two fields) and the category y axis that init3DAxis used to push in. - What the user does with the mode bar cannot be derived, so it stays in state and is merged last: rebuilding the layout never discards a zoom or pan. Also fixes a stale width: the effect depended on [width] but read layoutPlotWidth, which also depends on how many coordinate sliders are shown. Redraws when a panel appears: 9 -> 6 on a metadata revisit, 3 -> 2 on first open, i.e. 19 -> 6 against the original baseline. The four benchmark guards and the 20 e2e specs pass, and the rendered layouts were checked against Plotly's _fullLayout (axis titles, types, grid, forced ratio, y2 side). Assisted-by: Claude/opus-5 --- .../renderer/components/plot/Heatmap2D.tsx | 191 +++++------- .../renderer/components/plot/SimplePlotly.tsx | 272 +++++++----------- .../components/plot/hooks/usePlotLayout.ts | 149 +++++----- frontend/src/tests/perf/BASELINE.md | 23 ++ 4 files changed, 279 insertions(+), 356 deletions(-) diff --git a/frontend/src/renderer/components/plot/Heatmap2D.tsx b/frontend/src/renderer/components/plot/Heatmap2D.tsx index f4ae33a7..f39b6721 100644 --- a/frontend/src/renderer/components/plot/Heatmap2D.tsx +++ b/frontend/src/renderer/components/plot/Heatmap2D.tsx @@ -65,67 +65,16 @@ export const Heatmap2D = ({ const [y, setY] = useState([]); const [z, setZ] = useState<(number | string)[][]>([]); const plotRef = useRef(null); - const [shouldForceRatio, setShouldForceRatio] = useState(false); - const [layoutPlot, setLayoutPlot] = useState>({ - autosize: true, - scene: { - xaxis: { title: { text: xAxis?.name || '' } }, - yaxis: { title: { text: yAxis?.name || '' } }, - zaxis: { title: { text: zAxis?.name || '' } }, - }, - xaxis: { - exponentformat: 'power', - showexponent: 'all', - separatethousands: true, - scaleanchor: null, - scaleratio: null, - zeroline: false, - showgrid: itemDataGrid.displayGrid, - }, - yaxis: { - exponentformat: 'power', - showexponent: 'all', - separatethousands: true, - zeroline: false, - showgrid: itemDataGrid.displayGrid, - }, - modebar: { - orientation: 'v', - }, - legend: { - x: 1.3, - y: 1, - groupclick: 'togglegroup', - tracegroupgap: 0, - }, - }); const selectedPlot = itemDataGrid.plot[parseInt(plotIndex)]; - // Custom hook used for trigger some useEffects to update the layout - usePlotLayout({ - itemDataGrid, - setLayoutPlot, - }); + // Axis types and grid display, derived rather than pushed into the layout by + // effects: Plotly compares `layout` by reference, so every push was a redraw. + const axisLayout = usePlotLayout({ itemDataGrid }); + // What the user changed with the mode bar (zoom, pan, autorange); merged last + // so rebuilding the layout never discards it. + const [userRelayout, setUserRelayout] = useState>({}); const [title, setTitle] = useState(itemDataGrid.title); const layoutPlotWidth = showSliders ? width * 0.8 : width; - /** - * Rule to determine if we have to force ratio. - * The value is initialized once at grid creation and only changed via the customization switch. - */ - useEffect(() => { - setShouldForceRatio(itemDataGrid.forceXyRatio); - }, [itemDataGrid.forceXyRatio]); - - /** - * Update layout to force ratio or not - */ - useEffect(() => { - const updatedLayoutPlot = structuredClone(layoutPlot); - updatedLayoutPlot.xaxis.scaleanchor = shouldForceRatio ? 'y' : null; - updatedLayoutPlot.xaxis.scaleratio = shouldForceRatio ? 1 : null; - setLayoutPlot(updatedLayoutPlot); - }, [shouldForceRatio]); - /** * Update the editable title when layout title change */ @@ -149,11 +98,6 @@ export const Heatmap2D = ({ return; } - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - title: { text: title }, - })); - const updatedDataPlot: DataGridPlot[] = active.dataPlot.map( (item: DataGridPlot) => { if (item.i === itemDataGrid.i) { @@ -175,12 +119,12 @@ export const Heatmap2D = ({ updatedConfiguration(newActive); }, [title]); - const handleRelayout = (newLayout: Partial) => { - setLayoutPlot((prevLayout) => ({ - ...prevLayout, + const handleRelayout = useCallback((newLayout: Partial) => { + setUserRelayout((previous) => ({ + ...previous, ...newLayout, // update the layout with new values })); - }; + }, []); const init3DAxis = useCallback(async () => { // Transpose data matrix to orign values @@ -216,17 +160,6 @@ export const Heatmap2D = ({ name: yCoord.name, unit: yCoord.unit, }; - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - yaxis: { - ...prevLayout.yaxis, - type: - typeof getFirstArrayValueFromShape(yCoord.data, yCoord.shape)[0] === - 'string' - ? 'category' - : 'linear', - }, - })); setYAxis(yAxisAtHeatmap); }, [itemDataGrid.plot, itemDataGrid.coordinates, plotIndex]); @@ -236,53 +169,83 @@ export const Heatmap2D = ({ init3DAxis(); }, [itemDataGrid.plot, itemDataGrid.coordinates, plotIndex]); - /* Update the layout of the plot */ - useEffect(() => { - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - title: { text: itemDataGrid.title }, - height: height, - width: layoutPlotWidth - 75, - })); - }, [itemDataGrid, width, height]); - /** - * Update the layout xAxis + * The whole Plotly layout, derived in one go: the axis titles from the axes + * `init3DAxis` resolved, the axis types and the grid from `usePlotLayout`, + * the 1:1 ratio from the grid's own rule, and the user's mode bar changes + * merged last. */ - useEffect(() => { + const layoutPlot = useMemo>(() => { const XTitle = xAxis?.name ? `${xAxis?.name} ${(xAxis?.unit && '[' + xAxis.unit + ']') || ''}` : ''; - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - xaxis: { - ...prevLayout.xaxis, - title: { - ...prevLayout.xaxis?.title, - text: XTitle, - }, - }, - })); - }, [xAxis]); - - /** - * Update the layout yAxis - */ - useEffect(() => { const YTitle = yAxis?.name ? `${yAxis?.name} ${(yAxis?.unit && '[' + yAxis.unit + ']') || ''}` : ''; - setLayoutPlot((prevLayout) => ({ - ...prevLayout, + + // A category y axis whenever the second coordinate holds strings - what + // init3DAxis used to push into the layout once it had resolved the axes. + const yCoord = itemDataGrid.coordinates?.find( + (coord) => coord.axeIndex === 1, + ); + const yTypeFromCoordinate = yCoord + ? typeof getFirstArrayValueFromShape(yCoord.data, yCoord.shape)[0] === + 'string' + ? 'category' + : 'linear' + : undefined; + + return { + autosize: true, + title: { text: itemDataGrid.title }, + height: height, + width: layoutPlotWidth - 75, + scene: { + xaxis: { title: { text: '' } }, + yaxis: { title: { text: '' } }, + zaxis: { title: { text: '' } }, + }, + xaxis: { + exponentformat: 'power', + showexponent: 'all', + separatethousands: true, + zeroline: false, + ...axisLayout.xaxis, + scaleanchor: itemDataGrid.forceXyRatio ? 'y' : null, + scaleratio: itemDataGrid.forceXyRatio ? 1 : null, + title: { text: XTitle }, + }, yaxis: { - ...prevLayout.yaxis, - title: { - ...prevLayout.yaxis?.title, - text: YTitle, - }, + exponentformat: 'power', + showexponent: 'all', + separatethousands: true, + zeroline: false, + ...axisLayout.yaxis, + ...(yTypeFromCoordinate ? { type: yTypeFromCoordinate } : {}), + title: { text: YTitle }, }, - })); - }, [yAxis]); + modebar: { + orientation: 'v', + }, + legend: { + x: 1.3, + y: 1, + groupclick: 'togglegroup', + tracegroupgap: 0, + }, + ...userRelayout, + }; + }, [ + xAxis, + yAxis, + itemDataGrid.title, + itemDataGrid.forceXyRatio, + itemDataGrid.coordinates, + height, + layoutPlotWidth, + axisLayout, + userRelayout, + ]); useEffect(() => { if (data3D && selectedPlot) { diff --git a/frontend/src/renderer/components/plot/SimplePlotly.tsx b/frontend/src/renderer/components/plot/SimplePlotly.tsx index 678ab0e7..23a08757 100644 --- a/frontend/src/renderer/components/plot/SimplePlotly.tsx +++ b/frontend/src/renderer/components/plot/SimplePlotly.tsx @@ -1,5 +1,5 @@ import { Center, Grid, Group, Select, Text } from '@mantine/core'; -import { Layout, AxisType } from 'plotly.js'; +import { Layout } from 'plotly.js'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import Plot from 'react-plotly.js'; import { Configuration, Coordinates, DataGridPlot } from 'src/renderer/types'; @@ -54,97 +54,26 @@ export const SimplePlotly = ({ }, [itemDataGrid.plot, itemDataGrid.coordinates]); const coordsUsedInAxes: 1 | 2 = 1; const SELECT_AXIS_HEIGHT = 40; // Height of the select axis component - const [layoutPlot, setLayoutPlot] = useState>({ - xaxis: { - scaleanchor: null, - scaleratio: null, - title: { - font: { - family: 'Courier New, monospace', - size: 16, - color: '#7f7f7f', - }, - }, - rangemode: 'normal', - showline: true, - zeroline: false, - type: - (itemDataGrid?.xAxisData?.type as AxisType) || - (itemDataGrid.plot.length > 0 && itemDataGrid.plot[0].x?.length > 0) - ? typeof itemDataGrid.plot[0]?.x[0] === 'string' - ? 'category' - : 'linear' - : 'linear', - exponentformat: 'power', - showexponent: 'all', - separatethousands: true, - showgrid: itemDataGrid.displayGrid, - }, - yaxis: { - title: { - font: { - family: 'Courier New, monospace', - size: 16, - color: '#7f7f7f', - }, - }, - rangemode: 'normal', - showline: true, - zeroline: false, - type: (itemDataGrid?.yAxisData?.type as AxisType) || 'linear', - exponentformat: 'power', - showexponent: 'all', - separatethousands: true, - showgrid: itemDataGrid.displayGrid, - }, - yaxis2: { - type: (itemDataGrid?.y2AxisData?.type as AxisType) || 'linear', - exponentformat: 'power', - showexponent: 'all', - separatethousands: true, - showgrid: itemDataGrid.displayGrid, - }, - modebar: { - orientation: 'v', - }, - legend: { - x: 1.1, - y: 1, - orientation: 'v', - traceorder: 'normal', - }, - plot_bgcolor: '#c7c7c7', - dragmode: 'zoom', - }); - // Custom hook used for trigger some useEffects to update the layout - usePlotLayout({ - itemDataGrid, - setLayoutPlot, - }); + // Plotly compares `layout` by reference, so the layout is derived in one + // memo instead of being assembled by a dozen effects that each produced a new + // identity - and so a new redraw - on mount and on every change. + const axisLayout = usePlotLayout({ itemDataGrid }); const [title, setTitle] = useState(itemDataGrid.title); - const [dataEntries, setDataEntries] = useState([]); + // What the user did with the mode bar (zoom, pan, autorange). It cannot be + // derived, and it is merged last so rebuilding the layout never undoes it. + const [userRelayout, setUserRelayout] = useState>({}); const plotDivRef = useRef(null); const layoutPlotWidth = showSliders ? width * (itemDataGrid.coordinates?.length > 1 ? 0.8 : 1) : width; const customContainerRef = useRef(null); - useEffect(() => { - // Check data entries to update axes titles when needed - const newDataEntries = [ - ...new Set(itemDataGrid.plot.map((plot) => plot.nodeUri.split('#')[0])), - ]; - if (JSON.stringify(newDataEntries) !== JSON.stringify(dataEntries)) { - setDataEntries(newDataEntries); - } - }, [itemDataGrid.plot]); - - const handleRelayout = (relayout: Partial) => { - setLayoutPlot((prevLayout) => ({ - ...prevLayout, + const handleRelayout = useCallback((relayout: Partial) => { + setUserRelayout((previous) => ({ + ...previous, ...relayout, // update the layout with new values })); - }; + }, []); const isPlotInY2 = useCallback( (plotName: string) => { @@ -197,14 +126,10 @@ export const SimplePlotly = ({ }, [isPlotInY2]); /** - * Update the layout title & dataPlot configuration when editing title + * Persist an edited title on the grid. The layout picks the title up from the + * `title` state below, so nothing here touches the layout. */ useEffect(() => { - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - title: { text: title }, - })); - if (!itemDataGrid.isEditing || title === itemDataGrid.title) { return; } @@ -228,29 +153,13 @@ export const SimplePlotly = ({ }, [title]); /** - * Update the layout height + * The whole Plotly layout, derived in one go. + * + * Axis titles are built from the plot names and the axis descriptors; the + * axis types and the grid come from `usePlotLayout`; what the user changed + * with the mode bar is merged last so a rebuild never discards their zoom. */ - useEffect(() => { - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - height: height, - })); - }, [height]); - - /** - * Update the layout width - */ - useEffect(() => { - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - width: layoutPlotWidth - 75, - })); - }, [width]); - - /** - * Update the layout yAxis - */ - useEffect(() => { + const layoutPlot = useMemo>(() => { const coordsYNames = [ ...new Set( itemDataGrid.plot @@ -261,41 +170,11 @@ export const SimplePlotly = ({ const YTitle = itemDataGrid.yAxisData?.name ? `${coordsYNames.length > 1 ? coordsYNames[0] + ', ...' : coordsYNames[0]} ${(itemDataGrid.yAxisData?.unit && '[' + itemDataGrid.yAxisData.unit + ']') || ''}` : ''; - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - yaxis: { - ...prevLayout.yaxis, - title: { - ...prevLayout.yaxis.title, - text: YTitle, - }, - }, - })); - }, [itemDataGrid.yAxisData, dataEntries]); - /** - * Update the layout xAxis - */ - useEffect(() => { const XTitle = itemDataGrid.xAxisData?.name ? `${itemDataGrid.xAxisData?.name} ${(itemDataGrid.xAxisData?.unit && '[' + itemDataGrid.xAxisData.unit + ']') || ''}` : ''; - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - xaxis: { - ...prevLayout.xaxis, - title: { - ...prevLayout.xaxis.title, - text: XTitle, - }, - }, - })); - }, [itemDataGrid.xAxisData, dataEntries]); - /** - * Update the layout y2Axis - */ - useEffect(() => { const coordsY2Names = [ ...new Set( itemDataGrid.plot @@ -306,31 +185,94 @@ export const SimplePlotly = ({ const Y2Title = itemDataGrid.y2AxisData?.name ? `${coordsY2Names.length > 1 ? coordsY2Names[0] + ', ...' : coordsY2Names[0]} ${(itemDataGrid.y2AxisData?.unit && '[' + itemDataGrid.y2AxisData.unit + ']') || ''}` : ''; - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - yaxis2: - itemDataGrid.y2AxisData && itemDataGrid.y2AxisData !== undefined - ? { - ...prevLayout.yaxis2, - title: { - text: Y2Title, - font: { - family: 'Courier New, monospace', - size: 16, - color: 'rgb(148, 103, 189)', - }, + + return { + title: { text: title }, + height: height, + width: layoutPlotWidth - 75, + xaxis: { + scaleanchor: null, + scaleratio: null, + title: { + font: { + family: 'Courier New, monospace', + size: 16, + color: '#7f7f7f', + }, + text: XTitle, + }, + rangemode: 'normal', + showline: true, + zeroline: false, + exponentformat: 'power', + showexponent: 'all', + separatethousands: true, + ...axisLayout.xaxis, + }, + yaxis: { + title: { + font: { + family: 'Courier New, monospace', + size: 16, + color: '#7f7f7f', + }, + text: YTitle, + }, + rangemode: 'normal', + showline: true, + zeroline: false, + exponentformat: 'power', + showexponent: 'all', + separatethousands: true, + ...axisLayout.yaxis, + }, + yaxis2: itemDataGrid.y2AxisData + ? { + exponentformat: 'power', + showexponent: 'all', + separatethousands: true, + ...axisLayout.yaxis2, + title: { + text: Y2Title, + font: { + family: 'Courier New, monospace', + size: 16, + color: 'rgb(148, 103, 189)', }, - tickfont: { color: 'rgb(148, 103, 189)' }, - overlaying: 'y', - side: 'right', - rangemode: 'normal', - showline: false, - zeroline: false, - showgrid: false, - } - : {}, - })); - }, [itemDataGrid.y2AxisData, dataEntries]); + }, + tickfont: { color: 'rgb(148, 103, 189)' }, + overlaying: 'y', + side: 'right', + rangemode: 'normal', + showline: false, + zeroline: false, + showgrid: false, + } + : {}, + modebar: { + orientation: 'v', + }, + legend: { + x: 1.1, + y: 1, + orientation: 'v', + traceorder: 'normal', + }, + plot_bgcolor: '#c7c7c7', + dragmode: 'zoom', + ...userRelayout, + }; + }, [ + title, + height, + layoutPlotWidth, + axisLayout, + itemDataGrid.plot, + itemDataGrid.xAxisData, + itemDataGrid.yAxisData, + itemDataGrid.y2AxisData, + userRelayout, + ]); useEffect(() => { // Update title when itemDataGrid.title change (when selecting a plot with original plot title) diff --git a/frontend/src/renderer/components/plot/hooks/usePlotLayout.ts b/frontend/src/renderer/components/plot/hooks/usePlotLayout.ts index d893f884..3c42070a 100644 --- a/frontend/src/renderer/components/plot/hooks/usePlotLayout.ts +++ b/frontend/src/renderer/components/plot/hooks/usePlotLayout.ts @@ -1,90 +1,85 @@ -import { useEffect } from 'react'; -import { AxisType } from 'plotly.js'; -import { DataGridPlot } from 'src/renderer/types'; -import { Layout } from 'plotly.js'; +import { useEffect, useMemo } from 'react'; +import { AxisType, Layout } from 'plotly.js'; +import { Configuration, DataGridPlot } from 'src/renderer/types'; +import { useIbexStore } from '../../../stores'; interface UsePlotLayoutParams { itemDataGrid: DataGridPlot; - setLayoutPlot: (value: React.SetStateAction>) => void; } +/** Type of the values actually plotted on x, which decides a category axis. */ +const typeOfXData = (itemDataGrid: DataGridPlot): string | undefined => + itemDataGrid.plot[0]?.x ? typeof itemDataGrid.plot[0].x[0] : undefined; + +/** + * Axis type for x: string data forces a category axis, and an axis left on + * `category` goes back to `linear` as soon as the data is numeric again. + * Otherwise the configured type wins. + */ +const xAxisTypeOf = (itemDataGrid: DataGridPlot): AxisType => { + const configured = itemDataGrid.xAxisData?.type as AxisType | undefined; + const fromData = typeOfXData(itemDataGrid); + + if (fromData === 'string') return 'category'; + if (configured === 'category' && fromData === 'number') return 'linear'; + return configured || 'linear'; +}; + +/** + * The parts of a Plotly layout both plot components derive the same way. + * + * This used to be five `useEffect`s calling `setLayoutPlot`. Since + * react-plotly.js compares `layout` by reference, each of them was a separate + * redraw of the panel; they are all pure functions of `itemDataGrid`, so they + * are derived in one memo instead and the caller merges the result into its own + * layout. + */ export function usePlotLayout({ itemDataGrid, - setLayoutPlot, -}: UsePlotLayoutParams) { - // Grid - useEffect(() => { - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - xaxis: { - ...prevLayout.xaxis, - showgrid: itemDataGrid.displayGrid, - }, - yaxis: { - ...prevLayout.yaxis, - showgrid: itemDataGrid.displayGrid, - }, - })); - }, [itemDataGrid.displayGrid, setLayoutPlot]); +}: UsePlotLayoutParams): Partial { + const displayGrid = itemDataGrid.displayGrid; + const xType = xAxisTypeOf(itemDataGrid); + const yType = (itemDataGrid.yAxisData?.type as AxisType) || 'linear'; + const y2Type = (itemDataGrid.y2AxisData?.type as AxisType) || 'linear'; - // X axis + // Keep the configured x axis type in step with the data. It is persisted with + // the configuration and read back by the customization panel, so it cannot + // just live in the layout. This used to assign to `itemDataGrid.xAxisData` + // directly, i.e. mutate store state from an effect. useEffect(() => { - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - xaxis: { - ...prevLayout.xaxis, - type: - (itemDataGrid?.xAxisData?.type as AxisType) || prevLayout.xaxis?.type, - }, - })); - }, [itemDataGrid.xAxisData?.type, setLayoutPlot]); + // Only the two transitions the old effect handled: nothing is written when + // the configured type is simply absent. + const fromData = typeOfXData(itemDataGrid); + const configured = itemDataGrid.xAxisData?.type; + const forcedType = + fromData === 'string' + ? 'category' + : configured === 'category' && fromData === 'number' + ? 'linear' + : null; + if (!forcedType) return; - // Y axis - useEffect(() => { - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - yaxis: { - ...prevLayout.yaxis, - type: - (itemDataGrid?.yAxisData?.type as AxisType) || prevLayout.yaxis?.type, - }, - })); - }, [itemDataGrid.yAxisData?.type, setLayoutPlot]); + const { active, updatedConfiguration } = useIbexStore.getState(); + const current = active?.dataPlot.find((item) => item.i === itemDataGrid.i); + if (!current?.xAxisData || current.xAxisData.type === forcedType) return; - // Y2 axis - useEffect(() => { - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - yaxis2: { - ...prevLayout.yaxis2, - type: - (itemDataGrid?.y2AxisData?.type as AxisType) || - prevLayout.yaxis2?.type, - }, - })); - }, [itemDataGrid.y2AxisData?.type, setLayoutPlot]); + const newActive: Configuration = { + ...active, + dataPlot: active.dataPlot.map((item) => + item.i === itemDataGrid.i + ? { ...item, xAxisData: { ...item.xAxisData, type: forcedType } } + : item, + ), + }; + updatedConfiguration(newActive); + }, [itemDataGrid.i, xType]); - useEffect(() => { - const newTypeOfX = itemDataGrid.plot[0]?.x - ? typeof itemDataGrid.plot[0]?.x[0] - : undefined; - if (newTypeOfX === 'string') { - // Update x axis to category type if it become a string - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - xaxis: { ...prevLayout.xaxis, type: 'category' }, - })); - itemDataGrid.xAxisData.type = 'category'; - } else if ( - itemDataGrid.xAxisData?.type === 'category' && - newTypeOfX === 'number' - ) { - // Update x axis to linear type if it become a number - setLayoutPlot((prevLayout) => ({ - ...prevLayout, - xaxis: { ...prevLayout.xaxis, type: 'linear' }, - })); - itemDataGrid.xAxisData.type = 'linear'; - } - }, [itemDataGrid.xAxisData.name]); + return useMemo( + () => ({ + xaxis: { showgrid: displayGrid, type: xType }, + yaxis: { showgrid: displayGrid, type: yType }, + yaxis2: { showgrid: displayGrid, type: y2Type }, + }), + [displayGrid, xType, yType, y2Type], + ); } diff --git a/frontend/src/tests/perf/BASELINE.md b/frontend/src/tests/perf/BASELINE.md index 1030a867..0fdea161 100644 --- a/frontend/src/tests/perf/BASELINE.md +++ b/frontend/src/tests/perf/BASELINE.md @@ -123,3 +123,26 @@ Note for the store split: `VisualizationPlot` subscribes to `active`, not to `active.dataPlot`, because `handleNewPlot` (`utils/plot.ts:235`) pushes a new grid into that array **in place**. A narrower selector never fires when a panel is added, and memoizing the RGL children on it renders an empty canvas. + +## After deriving the Plotly layout (stage 5) + +| Scenario | requests | redraws | renders | ms | +| -------------------------- | -------- | ------- | ------- | ---- | +| toggle edit mode (UI flag) | 0 | 1 | 4 | 1023 | +| coordinate slider, 2 steps | 0 | 6 | 28 | 1570 | +| metadata panel, first open | 2 | **2** | **4** | 1012 | +| metadata panel, revisit | 0 | **6** | 40 | 1548 | +| idle (no interaction) | 0 | 0 | 0 | 2126 | + +The layout used to be assembled by fifteen effects (seven in `SimplePlotly`, +five in `usePlotLayout`, six in `Heatmap2D`), each calling `setLayoutPlot` and +so handing `react-plotly.js` a new `layout` identity — one redraw apiece as a +panel appeared. It is now a single `useMemo` per component, with the user's mode +bar changes kept in state and merged last so a rebuild never discards a zoom. + +Redraws when a panel appears: 9 → 6 on revisit, 3 → 2 on first open. Against the +original baseline, reopening the metadata panel costs 19 → 6 redraws and 72 → 40 +renders. + +The remaining 6 on the slider scenario are data-driven, not layout-driven: each +tick rebuilds the plot arrays. From d91d0f2502f8bd3d4b5864f7d11cecb3a9add995 Mon Sep 17 00:00:00 2001 From: Olivier Hoenen Date: Fri, 18 Sep 2026 15:37:23 +0200 Subject: [PATCH 08/10] perf(data): stop serializing and deep-copying data on the hot paths Two kinds of waste, both on the paths that run per interaction rather than per fetch. Round trips that were taken one at a time: - The geometry overlays fetched r and z (outline), r/z/width/height (rectangle) and r/z/length_alpha/length_beta/alpha/beta (oblique) one after the other, although the nodes are independent: up to six sequential round trips per overlay, now one Promise.all. - Error bands fetched the upper band, waited, then fetched the lower one; same fix, in both the interpolated and the plain branch. - The interpolation