Skip to content

perf(search): memoize virtual row to cut flushSync reconcile cost#1921

Open
sanjams2 wants to merge 1 commit intohyperdxio:mainfrom
sanjams2:perf/memoize-virtual-row
Open

perf(search): memoize virtual row to cut flushSync reconcile cost#1921
sanjams2 wants to merge 1 commit intohyperdxio:mainfrom
sanjams2:perf/memoize-virtual-row

Conversation

@sanjams2
Copy link

== Motivation ==

Scroll on the search results table produces 90-450ms long tasks per tick

== Details ==

@tanstack/react-virtual calls flushSync(rerender) on every scroll event while isScrolling is true (see the onChange adapter in the react-virtual source). With overscan: 30 we keep ~90 rows mounted, and the inline items.map() render means React has no way to skip unchanged rows - it reconciles all 90 synchronously on every scroll notification, even when only 2-3 actually entered or left the window.

The comparator keys on row.id (tanstack-table's stable identifier) rather than object identity, since getRowModel() may return fresh row references even when data is unchanged. isHighlighted and isExpanded are computed per-row in the parent so that selecting one row doesn't bust memo on the other 89.

== Testing ==

  • npx jest DBRowTable.test.tsx - 15/15 pass
  • DevTools Performance profile on search page: hot path was scroll -> maybeNotify -> notify -> onChange -> flushSync -> React reconciler; memoizing the row lets React bail out on unchanged ones

== Motivation ==

Scroll on the search results table produces 90-450ms long tasks per tick

== Details ==

@tanstack/react-virtual calls `flushSync(rerender)` on every scroll event
while `isScrolling` is true (see the `onChange` adapter in the react-virtual
source). With `overscan: 30` we keep ~90 rows mounted, and the inline
`items.map()` render means React has no way to skip unchanged rows - it
reconciles all 90 synchronously on every scroll notification, even when
only 2-3 actually entered or left the window.

The comparator keys on `row.id` (tanstack-table's stable identifier)
rather than object identity, since `getRowModel()` may return fresh row
references even when data is unchanged. `isHighlighted` and `isExpanded`
are computed per-row in the parent so that selecting one row doesn't
bust memo on the other 89.

== Testing ==

- `npx jest DBRowTable.test.tsx` - 15/15 pass
- DevTools Performance profile on search page: hot path was `scroll -> maybeNotify -> notify -> onChange -> flushSync -> React reconciler`; memoizing the row lets React bail out on unchanged ones
@changeset-bot
Copy link

changeset-bot bot commented Mar 16, 2026

⚠️ No Changeset found

Latest commit: f8741c9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Mar 16, 2026

@sanjams2 is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions
Copy link
Contributor

PR Review

  • ⚠️ Column resizing will silently break: The custom memo comparator only checks columnsLength but not column widths. When a user resizes a column, row.id and columnsLength stay the same, so all 90 memoized rows skip re-render — displaying stale widths. The comparator needs to include column sizes: either pass a columnSizing snapshot as a prop (e.g., columnSizingHash) and compare it, or add prev.row === next.row (object identity) to catch TanStack Table rebuilding row objects on state change.

  • ⚠️ renderRowDetails excluded from comparator: If the parent passes a new renderRowDetails reference (e.g., due to re-render of a parent component), expanded rows won't update. Either add prev.renderRowDetails === next.renderRowDetails to the comparator or wrap renderRowDetails in useCallback at the call site.

  • onRowExpandClick and onToggleWrap are properly stabilized with useCallback — no stale closure risk there.

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.

1 participant