Skip to content

perf: read the published run order by id instead of scanning it - #400

Merged
nGervasyuk merged 1 commit into
Visual-Regression-Tracker:masterfrom
nGervasyuk:fix/dialog-order-quadratic
Aug 23, 2026
Merged

perf: read the published run order by id instead of scanning it#400
nGervasyuk merged 1 commit into
Visual-Regression-Tracker:masterfrom
nGervasyuk:fix/dialog-order-quadratic

Conversation

@nGervasyuk

@nGervasyuk nGervasyuk commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Problem

Reviewing a build of ~10 000 screenshots, every interaction with the list froze the page for over a second: opening the build, switching to the card view, toggling Group variations, picking a tag filter.

The API was idle throughout (0% CPU, list served in 160 ms), so this is all in the browser. A CPU profile taken in Chromium on a 10 000 run build points at one place — 1422 ms of the 1858 ms spent on a single Group variations toggle:

1422 ms self — (anonymous) at src/components/TestDetailsDialog/index.tsx:46
      <- useMemo
      <- TestDetailsDialog src/components/TestDetailsDialog/index.tsx:31
      <- renderWithHooks / beginWork / performSyncWorkOnRoot

That memo turns the order the list publishes (filteredSortedTestRunIds) into the dialog's own run array by scanning that array: includes once per run to filter, then indexOf twice per comparison while sorting. With 10 000 runs the filter alone is 10⁸ comparisons, and the sort is far worse.

Two things make it hurt:

  • it recomputes on every list change, because the list republishes the order (a fresh array) whenever the filter, sort, grouping or view changes
  • it runs whether the dialog is open or not — the if (!selectedTestRun) return null guard sits below the hooks

Change

Index the published order in a Map once and read positions off it, so filtering and sorting are lookups rather than scans. The render also no longer scans for the current run's index a second time — that index is already memoised right below.

Measured with the same harness in Chromium, 10 000 runs, 500 screens × 20 locales:

Interaction Before After
open a build (table view) 1190 ms 544 ms
switch to the card view 755 ms 118 ms
toggle Group variations 618 ms 100 ms

The hotspot disappears from the profile entirely; what is left is idle time and groupTestRuns at ~9 ms.

Behaviour is unchanged — the dialog still walks the runs in the order the list publishes, which the existing arrow-navigation integration tests cover. Unit (19) and Playwright (84) suites pass.


Nothing to configure. Independent of the other open frontend PRs (#397, #398, #399, #400, #401) — verified that they all merge onto master in sequence without conflicts, in any order.

@pashidlos pashidlos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

The details dialog turns the order the list publishes into its own run array,
and it did so by scanning that array: once per run to filter, then twice per
comparison while sorting. On a build of ten thousand screenshots that is a
hundred million comparisons for the filter alone, and the sort makes it far
worse.

It also runs whether the dialog is open or not, and on every list change —
switching view, grouping variations, picking a tag filter — so the whole page
stalled for over a second on each interaction. Measured in Chromium on a
10 000 run build: 1.4 s of the 1.9 s spent on a Group variations toggle was
this one memo.

Index the published order in a Map and read positions off it:

  open a build       1190 ms -> 544 ms
  switch to cards     755 ms -> 118 ms
  group variations    618 ms -> 100 ms

The index of the run on screen is already memoised right below, so the render
no longer scans for it a second time.
@nGervasyuk
nGervasyuk force-pushed the fix/dialog-order-quadratic branch from 73b8b8f to 71d71aa Compare August 23, 2026 06:15
@sonarqubecloud

Copy link
Copy Markdown

@nGervasyuk
nGervasyuk merged commit 32abac8 into Visual-Regression-Tracker:master Aug 23, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants