perf(devtools): stop the panel freezing on caches with thousands of queries - #11150
Open
cloudfluffy wants to merge 16 commits into
Open
perf(devtools): stop the panel freezing on caches with thousands of queries#11150cloudfluffy wants to merge 16 commits into
cloudfluffy wants to merge 16 commits into
Conversation
Give query/mutation rows a fixed height driven by --tsqd-font-size and render the key hash on a single line with ellipsis truncation. The full key remains available via the row aria-label, a new title tooltip, and the details pane. Predictable per-row geometry is a prerequisite for windowing the list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 639057ed-0ed9-4329-8008-71458714184f
Render the query pane through a new windowed VirtualList so only the rows in (and near) the scroll viewport are mounted. Because each QueryRow owns five query-cache subscriptions, windowing bounds both the DOM node count and the module-level subscription map that the global cache handler walks on every event, instead of scaling with the number of cached queries. VirtualList seeds its viewport from a bounded default (never the item count), resolves ResizeObserver from the element's own document for Picture-in-Picture, clamps the scroll offset so a shrinking list never blanks the viewport, and always keeps the selected row mounted. Row offsets are arithmetic given the fixed row height derived from --tsqd-font-size. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 639057ed-0ed9-4329-8008-71458714184f
Render the mutation pane through the same windowed VirtualList as the query pane, bounding mounted MutationRow components and their mutation- cache subscriptions to the visible window. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 639057ed-0ed9-4329-8008-71458714184f
Each MutationRow previously resolved its own mutation by scanning the whole mutation cache with getAll().find() in three subscriptions, so a mutation-cache event cost O(rows x cache size). Read the row's own stable mutation instance directly instead; the subscription still fires on cache changes but the read is O(1). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 639057ed-0ed9-4329-8008-71458714184f
Add tests for the windowed list: bounded row count for large caches, a bounded initial render even when no resize measurement is delivered, spacer sizing to the full list height, scroll-driven row recycling, filter-shrink after deep scroll not blanking the viewport, view remount re-initialization, the selected row staying mounted exactly once when scrolled out of range, the query-key title attribute, and a mutation row rendered from its own state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 639057ed-0ed9-4329-8008-71458714184f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 639057ed-0ed9-4329-8008-71458714184f
The subscriber registries for the query and mutation caches are shared by every devtools instance on the page, but a panel's teardown cleared them entirely rather than removing only its own entries. With two panels mounted at once - during the picture-in-picture transition, or when the standalone panel is used alongside the floating one - tearing one down unsubscribed the other, leaving its list, status counts and details pane frozen. Each subscription already deletes its own entry on disposal, so the registry-wide clear was redundant as well as harmful.
The virtualized lists rebuilt every mounted row on each scroll and each cache event. The window is recomputed into freshly allocated row wrappers, so the keyed list's item signal always notified, and because the row was rendered by calling the row renderer inside a child position that read that signal, every notification tore down and reconstructed the row - its DOM, its styles and its five cache subscriptions. Rows now receive an accessor and read the item through a prop, so a row is built once and updated in place. This is the shape the lists used before they were virtualized.
Each status badge backed its count with an independent cache subscription, and every one of those subscriptions walked the whole cache and allocated a full-size array on each cache event - five passes for queries, four for mutations. Each set of counts now comes from one pass. The individual counts are exposed as memos so a badge still only re-renders when its own count changes, as it did when each count had its own equality-checked signal. The mutation tally carries a bucket for the gray status that no badge displays, because an idle mutation resolves to it and the tally has to stay exhaustive.
While a query was selected, the details pane derived seven values through seven separate subscriptions, and each located the query by scanning the whole cache and allocating a full-size array on every cache event. The cache is keyed by query hash, so each of those lookups is now a direct retrieval - the same correction the query rows already had. The mutation cache has no keyed access, so the mutation pane keeps its scan but derives all three of its values from one shared lookup rather than three. That lookup's equality check stays disabled: a mutation's object identity does not change across status transitions, so the pane would otherwise freeze on its first rendered state.
A mutation-cache event scheduled every subscriber on its own microtask, so each signal write triggered an independent downstream update. The fan-out now runs inside a single microtask with the writes batched together, so one event produces one update. The batch has to sit inside the microtask rather than around the loop: a batch around the loop would exit before any deferred setter ran. Iterating the registry inside the tick also means subscriptions disposed in the meantime are skipped rather than written to after disposal.
…omponent
`stylesFactory` builds around sixty CSS-in-JS templates, and twelve components
in the panel plus six in the JSON explorer each called it from inside their own
per-instance memo. Two of those components are the query and mutation rows, and
the explorer is recursive - one instance per node of the rendered object - so
with a large cache the whole stylesheet was recompiled for every row the
virtualized list mounted and for every node the details pane expanded.
The compiled result depends only on the theme and the `css` instance, so it is
now memoized on both. Memoizing on the `css` instance only helps if that
instance is stable, and it was not: every call site built its own with
`css.bind({ target })`, which returns a new function object each call, so a
cache keyed on it would never hit for a panel mounted in a shadow root - which
is how the devtools are normally embedded. The bound function is therefore
cached per shadow root as well.
Both helpers live in `utils.tsx` so that the panel and the explorer, which each
keep their own stylesheet, can share them.
…e unchanged The list of queries to render was rebuilt into a fresh array on every cache event. Most events leave both the membership and the order of that list untouched - a query's data changing, or a fetch settling, cannot reorder anything, and half of all events are observer notifications - but the new array was still a new value, so every downstream consumer recomputed and the virtualizer rediffed. The memo now holds the previous array whenever the recomputed one contains the same queries in the same order, so consumers only rerun when the list has genuinely changed. The sorted array is always freshly allocated before it is sorted in place, so a retained array is never mutated afterwards.
Two devtools subscribers - the query count and the status tallies - inspect every query in the cache each time they run, and they ran once per cache event. A stream of arriving, updating and expiring queries therefore cost a full-cache pass per event, and each `setQueryData` emits two events. With several thousand queries that is enough to stall the page for as long as the panel is open. These two now coalesce. A subscriber that has been idle for a window runs immediately, so an isolated change is never delayed; events arriving inside a window fold into a single trailing pass. Per-row subscribers are already filtered to one query and are unchanged. Three details of the window matter. It is measured in time rather than microtasks, because cache events usually arrive in separate tasks - one per network response - and a microtask window would close between every pair of them, coalescing nothing. It is stamped when a pass finishes rather than when it starts, because a start-to-start window shorter than the pass leaves every event outside it, so nothing coalesces and the passes run back to back. And it widens to a multiple of the last pass, which bounds the share of the main thread the subscriber can take however large the cache becomes. A pass being in flight is tracked separately from a trailing pass being armed. An event raised from inside a pass has nothing in flight that can reflect it, so it is remembered and a trailing pass is armed once the pass unwinds, rather than being folded into a pass that has already read the cache. The clock is `performance.now()`; `Date.now()` can step backwards on a clock correction, which would hold a subscriber closed for the size of the step. At eight thousand queries with a mixed stream of arrivals, updates and removals these three changes together take the panel from blocking around 90% of wall clock, with a longest block of 142 ms, to blocking under half with a longest block of around 32 ms.
…on test Collapses the per-fix changesets into the single entry this pull request should produce, matching how the devtools changelog is written elsewhere - one entry per pull request rather than one per commit. The entry also records that the coalescing window widens with the measured cost of a pass, which is the one user-visible timing change in this work. Also replaces an assertion in the row-recycling test that reduced to `reused.length === reused.length` and so passed whatever the virtualizer did. The scroll in that test advances the window by exactly one row, so the exact count is assertable. Drops an `overscan` prop on the internal virtual list that no call site ever set.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughDevtools now virtualizes query and mutation lists, reduces cache traversal and notification work, preserves stable list identity, batches updates, and caches target-aware stylesheet generation. Tests cover virtualization, cache state transitions, panel teardown, and large-cache behavior. ChangesDevtools performance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant QueryCache
participant DevtoolsBatcher
participant VirtualList
participant DevtoolsView
QueryCache->>DevtoolsBatcher: emit cache events
DevtoolsBatcher->>DevtoolsBatcher: coalesce and batch updates
DevtoolsBatcher->>DevtoolsView: publish stable lists and status counts
DevtoolsView->>VirtualList: render visible rows and overscan
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/query-devtools/src/__tests__/Devtools.test.tsx (2)
344-388: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTighten the assertion and the name in these two tests.
Two points:
- Line 368:
expect(keysInOrder()).not.toEqual(before)passes for any label change, not only a reorder. Asserting the expected order, for exampleexpect(keysInOrder()[0]).toMatch(/\["a"\]/), states the intent and does not depend on unrelated label content.- Line 371: the test is named "reflects a change that does reorder the list", but the body removes a query and changes the list length. A name such as "reflects a removal from the list" describes what it verifies.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/query-devtools/src/__tests__/Devtools.test.tsx` around lines 344 - 388, The time-based sort test should assert the expected reordered first key directly, using the established `keysInOrder()` and `["a"]` label pattern instead of only checking that the labels differ. Rename the second test to describe removal from the list, while preserving its existing removal behavior and assertion.
1820-1833: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis test asserts reuse before the list recomputes.
queryCountis coalesced, sosetQueryData(['q-0'], 'updated')at line 1827 does not recompute thequeriesmemo within the same task. At the assertion the row window has not been rebuilt, so the test passes without exercising row reuse across a list recomputation.Advance the timers past the coalescing window before reading
after, and make the testasync.♻️ Proposed change
- it('reuses row elements across a cache event', () => { + it('reuses row elements across a cache event', async () => { seedQueries(200) const rendered = renderDevtools({ initialIsOpen: true }) const before = [...rendered.container.querySelectorAll('.tsqd-query-row')] before.forEach((row, i) => row.setAttribute('data-row-id', String(i))) queryClient.setQueryData(['q-0'], 'updated') + // The list recomputes only after the coalescing window closes. + await vi.advanceTimersByTimeAsync(50) const after = [...rendered.container.querySelectorAll('.tsqd-query-row')]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/query-devtools/src/__tests__/Devtools.test.tsx` around lines 1820 - 1833, Make the “reuses row elements across a cache event” test asynchronous and advance timers past the queryCount coalescing window after setQueryData, awaiting the resulting update before collecting after. Keep the existing row identity assertions unchanged so they validate reuse across the recomputed list.packages/query-devtools/src/Devtools.tsx (3)
2896-2908: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winThe mutation fan-out still runs once per event, not once per burst.
queueMicrotaskis called for every mutation-cache event with no guard. A burst of N events in one task queues N microtasks, and each one iterates the wholemutationCacheMap.MutationStatusCountwalks the entire mutation cache in its callback, so the cost is still O(events x cache size).The query cache side solves this with the new coalescing state. A minimal equivalent here is a single pending flag that collapses a burst into one fan-out.
♻️ Proposed fix to collapse a burst into one fan-out
const setupMutationCacheSubscription = () => { const mutationCache = createMemo(() => { const client = useQueryDevtoolsContext().client return client.getMutationCache() }) + let flushScheduled = false const unsubscribe = mutationCache().subscribe(() => { - // One microtask around the whole fan-out, with the writes batched inside - // it, so a mutation-cache event produces a single downstream update rather - // than one per subscriber. The batch has to live inside the microtask: a - // batch around the loop would exit before any deferred setter ran. + // One microtask around the whole fan-out, with the writes batched inside + // it, so a burst of mutation-cache events produces a single downstream + // update rather than one per event or one per subscriber. The batch has to + // live inside the microtask: a batch around the loop would exit before any + // deferred setter ran. + if (flushScheduled) return + flushScheduled = true queueMicrotask(() => { + flushScheduled = false batch(() => { for (const [callback, setter] of mutationCacheMap.entries()) { setter(callback(mutationCache)) } }) }) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/query-devtools/src/Devtools.tsx` around lines 2896 - 2908, Update the mutationCache subscription around mutationCacheMap to add a pending/coalescing flag: schedule the microtask only when no fan-out is already pending, set the flag before queueing, and clear it when the microtask begins before batching callbacks. Preserve the existing batch and callback iteration while ensuring multiple events in one task trigger only one fan-out.
2003-2010: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
activeQueryandactiveQueryFreshare now identical subscriptions.Both callbacks resolve the same query by the same hash, and both pass
equalityCheck = false. They register two separate entries inqueryCacheMapand run two identical passes on every cache event. Consider keeping one accessor and using it for both the actions and theExplorervalue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/query-devtools/src/Devtools.tsx` around lines 2003 - 2010, Remove the duplicate active-query subscription by retaining a single accessor for the selected query hash and reusing it for both the action handlers and the Explorer value. Update references to activeQueryFresh or activeQuery as needed so all consumers share the same subscription without changing the lookup or equality behavior.
966-989: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider re-reading the row font size when the panel resizes.
rowFontSizerefreshes only on windowfocus. The CSS row height usescalc(var(--tsqd-font-size) * 1.5)and tracks the variable live. If the inherited font size changes while the window keeps focus, for example after a page-level font-size change, the CSS row height and the JSrowHeightdiverge until the next focus event. Rows then overlap or leave gaps, because offsets are computed from the stale value.
ContentViewalready runs inside a panel that observes resizes. Re-reading the variable on those resizes closes the gap at low cost.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/query-devtools/src/Devtools.tsx` around lines 966 - 989, Update the rowFontSize measurement flow in the onMount callback to also invoke readRowFontSize when the observed panel resizes, reusing ContentView’s existing resize observation mechanism. Preserve the current initial read, window-focus refresh, and rowHeight calculation while ensuring JS offsets stay synchronized with the live --tsqd-font-size value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/query-devtools/src/Devtools.tsx`:
- Around line 3643-3652: Update the .tsqd-query-hash markup and styles so the
hash is wrapped in an inner block-level element that owns min-width: 0,
white-space: nowrap, overflow: hidden, and text-overflow: ellipsis; keep the
outer flex container’s layout and other styling unchanged.
---
Nitpick comments:
In `@packages/query-devtools/src/__tests__/Devtools.test.tsx`:
- Around line 344-388: The time-based sort test should assert the expected
reordered first key directly, using the established `keysInOrder()` and `["a"]`
label pattern instead of only checking that the labels differ. Rename the second
test to describe removal from the list, while preserving its existing removal
behavior and assertion.
- Around line 1820-1833: Make the “reuses row elements across a cache event”
test asynchronous and advance timers past the queryCount coalescing window after
setQueryData, awaiting the resulting update before collecting after. Keep the
existing row identity assertions unchanged so they validate reuse across the
recomputed list.
In `@packages/query-devtools/src/Devtools.tsx`:
- Around line 2896-2908: Update the mutationCache subscription around
mutationCacheMap to add a pending/coalescing flag: schedule the microtask only
when no fan-out is already pending, set the flag before queueing, and clear it
when the microtask begins before batching callbacks. Preserve the existing batch
and callback iteration while ensuring multiple events in one task trigger only
one fan-out.
- Around line 2003-2010: Remove the duplicate active-query subscription by
retaining a single accessor for the selected query hash and reusing it for both
the action handlers and the Explorer value. Update references to
activeQueryFresh or activeQuery as needed so all consumers share the same
subscription without changing the lookup or equality behavior.
- Around line 966-989: Update the rowFontSize measurement flow in the onMount
callback to also invoke readRowFontSize when the observed panel resizes, reusing
ContentView’s existing resize observation mechanism. Preserve the current
initial read, window-focus refresh, and rowHeight calculation while ensuring JS
offsets stay synchronized with the live --tsqd-font-size value.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e2b5b66b-fe1c-4a52-b8f5-70f11f26619c
📒 Files selected for processing (7)
.changeset/devtools-large-cache-performance.mdpackages/query-devtools/src/Devtools.tsxpackages/query-devtools/src/Explorer.tsxpackages/query-devtools/src/__tests__/Devtools.test.tsxpackages/query-devtools/src/__tests__/DevtoolsPanelComponent.test.tsxpackages/query-devtools/src/constants.tspackages/query-devtools/src/utils.tsx
`text-overflow` applies to block containers, and the query hash element is a flex container, so the declaration had no effect and a key too long for its row was cut off mid-character instead. Make the element a block and keep the text vertically centred with a line height equal to its minimum height.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
Closes #11149.
Opening the devtools against a
QueryClientholding thousands of queries freezes the page, and it stays frozen while the cache keeps changing. The panel renders one row per cached query and per cached mutation with no windowing, and several of its subscriptions walk the whole cache on every cache event, so both the mount cost and the per-event cost grow with the size of the cache.There were four separate causes.
Every query got its own row, all at once. The panel mounted one row per cached query and per cached mutation, and each row subscribed to the cache to keep itself current — thousands of DOM nodes, and thousands of subscriptions for the cache to notify on every change.
Both lists are now virtualized.
VirtualList(Devtools.tsx:680) mounts only the rows inside the scroll viewport, plus a small overscan. This needs a known row height, so rows are now a fixed height derived fromQUERY_ROW_HEIGHT_MULTIPLIER(constants.ts:20), which drives both the row CSS and the window arithmetic.Selecting a query and then scrolling its row out of view would unmount the row and tear down the details pane's subscription, so
pinnedKey(Devtools.tsx:684) keeps the selected row mounted.Parts of the panel searched the whole cache to find one query.
QueryDetails(Devtools.tsx:1981) andMutationDetails(2509) resolved their selected item by scanning the cache until they found a match, and the status badges —QueryStatusCount(1781) andMutationStatusCount(1826) — counted the entire cache five separate times, once per status. Every cache event re-ran all of that.The details panes now look their item up by key, and the badges tally all five statuses in a single pass.
MutationRow(1677) had a variant of the same problem: it already receives its mutation as a prop, but three of its subscriptions scanned the whole cache to find it bymutationIdon every mutation-cache event. They now readprops.mutation.statedirectly.The panel recalculated on every change, even when changes arrived in bursts. Nothing throttled the two subscribers that read the entire cache, and a single
setQueryDatafires two cache events, so every update in a burst cost two full passes.Those two now coalesce (
setupQueryCacheSubscription,Devtools.tsx:2787): a subscriber that has been idle runs immediately, and anything arriving while a window is open folds into one trailing pass. The window ismax(COALESCE_WINDOW_MS, lastPassMs * COALESCE_PASS_MULTIPLE)— widening it with the cost of the last pass bounds the share of the main thread the panel can take as the cache grows.Two smaller changes ride along: the query list keeps its array identity when its contents haven't changed (
sameAsPrevious,871-872), andsetupMutationCacheSubscription(2890) now batches its fan-out the way the query side already did.There is also a fix here that isn't about performance. The subscriber registries are shared by every panel on the page, but a panel's teardown cleared them entirely instead of removing its own entries. With two panels mounted — during the picture-in-picture transition, or the standalone panel alongside the floating one — tearing one down froze the other.
The panel's stylesheet was rebuilt for every component. Around sixty CSS-in-JS templates were recompiled from scratch each time a row mounted or a node in the JSON explorer expanded.
createStylesCache(utils.tsx:345) now compiles a stylesheet once per theme and shares it, andcssForTarget(utils.tsx:332) gives it a stable cache key by bindinggoober'scssonce per shadow root instead of once per component.Behaviour change
A single change to a quiet cache still updates the panel immediately, exactly as before. The difference only shows during a stream of changes: the query list and the status badges then update once per coalescing window instead of once per event.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Performance
Usability