Improve subscription performance by 10-20 times #9827
Conversation
…ss caches - Improved performance by 5-10 times - Tests left intact (no breaking changes expected)
🦋 Changeset detectedLatest commit: e07ba50 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe PR introduces a centralized GCManager class that consolidates garbage-collection timing across multiple queries and mutations into a single coordinated scanning mechanism. Query, Mutation, and Removable classes are refactored to replace per-item timeout scheduling with mark-based GC tracking. QueryClient initializes and exposes the GCManager, and tests are updated to reflect timing adjustments and GC state inspection patterns. Changes
Sequence DiagramsequenceDiagram
participant Query
participant GCManager
participant Removable Item
Note over Query,Removable Item: New GC Flow
Query->>Query: Constructor: markForGc()
Query->>GCManager: trackEligibleItem(this)
GCManager->>GCManager: Schedule scan (microtask + timeout)
alt Item becomes observable
Query->>GCManager: clearGcMark()
GCManager->>GCManager: Untrack item, reschedule if needed
end
alt Scan interval fires
GCManager->>Removable Item: isEligibleForGc()
alt Item eligible
GCManager->>Removable Item: optionalRemove()
Removable Item-->>GCManager: true (removed)
GCManager->>GCManager: Untrack item
else Item not eligible
GCManager->>GCManager: Continue to next item
end
GCManager->>GCManager: Reschedule if items remain
end
Note over Query,Removable Item: Old GC Flow (replaced)
Query->>Query: scheduleGc() via timeoutManager
Note over Query: Per-item timeout ✗
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (15)
packages/react-query/src/__tests__/useQuery.test.tsx (3)
4024-4028: Prefer explicit microtask flush over advancing timers by 0msGCManager schedules via queueMicrotask. Replace advanceTimersByTimeAsync(0) with vi.runAllTicks() (or await Promise.resolve()) to avoid environment‑dependent flakiness.
- await vi.advanceTimersByTimeAsync(0) + await vi.runAllTicks()
4048-4063: Stabilize GC scheduling in the finite gcTime testAfter unmount, flush microtasks before advancing by gcTime to ensure the scan timeout is actually scheduled. Also consider restoring system time at the end (defensive) even though useRealTimers typically resets it.
- rendered.unmount() + rendered.unmount() + await vi.runAllTicks() expect(query!.gcMarkedAt).not.toBeNull() @@ - await vi.advanceTimersByTimeAsync(gcTime) + await vi.advanceTimersByTimeAsync(gcTime) + // Optionally: vi.setSystemTime(new Date())
4026-4028: Avoid asserting internal gcMarkedAt in app‑level testsgcMarkedAt is an internal detail; these assertions may become brittle. Prefer black‑box checks (presence/absence in cache after time) or expose a stable accessor solely in test builds if needed.
Also applies to: 4048-4052
packages/query-core/src/__tests__/mutationCache.test.tsx (1)
419-421: Replace magic 11ms with intent‑expressive timingMake the wait derive from the mutation’s 10ms work plus a microtask flush to reduce off‑by‑one flakiness.
- await vi.advanceTimersByTimeAsync(11) + await vi.advanceTimersByTimeAsync(10) + await vi.runAllTicks() + await vi.advanceTimersByTimeAsync(1)packages/query-core/src/queryClient.ts (1)
78-79: GC lifecycle and API surface: a couple of follow‑ups
- Disabling on server: forceDisable: isServer is sensible. Please verify Deno/edge runtimes where globalThis.Deno may exist even when a window‑like environment is present.
- Consider stopping scanning on unmount to avoid stray timeouts when the client is detached (without clearing tracked items). This prevents keeping the event loop alive unintentionally.
- getGcManager() is a new public method; confirm it’s intended as public API despite “no API changes” in the PR text.
unmount(): void { this.#mountCount-- if (this.#mountCount !== 0) return this.#unsubscribeFocus?.() this.#unsubscribeFocus = undefined this.#unsubscribeOnline?.() this.#unsubscribeOnline = undefined + // Stop background GC timers when the client is fully unmounted. + this.#gcManager.stopScanning() }Would you like me to add a test to assert no timers remain after unmount?
Also applies to: 102-111, 461-463
packages/query-core/src/query.ts (1)
181-197: Marking for GC in the constructor can hasten evictionMarking immediately means “warm” queries created without observers get GC‑scheduled right away. If that’s a change from prior behavior, consider gating on finite gcTime (and/or initialData presence) to avoid surprising early collection.
- this.markForGc() + if (this.options.gcTime !== Infinity) { + this.markForGc() + }Please confirm this doesn’t alter expectations for prefetch/setQueryData heavy apps.
packages/query-core/src/__tests__/gcManager.test.tsx (2)
41-49: Add a helper to flush microtasks explicitlyReplace repeated advanceTimersByTimeAsync(0) with a small flushMicrotasks helper to deterministically run queueMicrotask work and reduce timing flakiness.
+async function flushMicrotasks() { + await vi.runAllTicks() +} @@ - await vi.advanceTimersByTimeAsync(0) + await flushMicrotasks()Also applies to: 68-80
323-339: Add coverage for forceDisable pathInclude a test that GCManager constructed with { forceDisable: true } never schedules scans nor tracks items, even when items are marked.
I can draft the test if you want.
packages/query-core/src/mutation.ts (2)
112-114: Initial GC mark: consider deferring to next microtask to avoid churn when an observer is typically added immediately.Small perf nit: creating every Mutation with a GC mark often gets cleared right away by addObserver. Deferring the initial mark via queueMicrotask can avoid a track→untrack cycle in common paths.
- this.setOptions(config.options) - this.markForGc() + this.setOptions(config.options) + queueMicrotask(() => { + if (!this.#observers.length) this.markForGc() + })
386-389: Re-marking after state transitions may postpone GC; confirm intent.Calling markForGc() here resets gcMarkedAt, effectively extending the GC window if it was previously marked. If your intent is “start GC window when it first becomes safe,” this is fine. If you prefer “earliest of all safe times,” consider only marking if not already marked.
- if (this.isSafeToRemove()) { - this.markForGc() - } + if (this.isSafeToRemove() && this.gcMarkedAt === null) { + this.markForGc() + }packages/query-core/src/gcManager.ts (3)
98-106: Method name/doc mismatch: “isScanning” actually means “scan is scheduled.”isScanning() returns true while a timeout is pending, not while performing the scan (it’s set false before #performScan). Consider renaming to improve clarity or adjust the JSDoc.
- /** - * Check if a scan is scheduled (timeout is pending). - * - * @returns true if a timeout is scheduled to perform a scan - */ - isScanning(): boolean { - return this.#isScanning - } + /** + * Check if a scan is scheduled (timeout is pending). + * Returns true while there is a pending timeout for a future scan. + */ + isScanScheduled(): boolean { + return this.#isScanning + } # Update call sites accordingly (e.g., untrackEligibleItem)
114-126: Reschedule when an already-tracked item’s GC timestamp moves.If markForGc is called again on an already-tracked item (gcMarkedAt reset or gcTime extended), we don’t reschedule, so an earlier timeout can fire unnecessarily and do an early no-op scan. Low impact, but easy to improve.
trackEligibleItem(item: Removable): void { if (this.#forceDisable) { return } - if (this.#eligibleItems.has(item)) { - return - } - - this.#eligibleItems.add(item) - - this.#scheduleScan() + if (this.#eligibleItems.has(item)) { + // Item already tracked; ensure we recalc next scan time + this.#scheduleScan() + return + } + + this.#eligibleItems.add(item) + this.#scheduleScan() }
162-180: Iteration + deletion is fine; optionally iterate over a snapshot to reduce mutation surprises.Current Set deletion during for-of is safe. If you ever add more complex side effects, iterating a copied array can simplify reasoning.
- for (const item of this.#eligibleItems) { + for (const item of Array.from(this.#eligibleItems)) {packages/query-core/src/removable.ts (2)
41-49: markForGc semantics are correct; consider rescheduling when re-marked.Setting gcMarkedAt and delegating to GCManager is right. If gcMarkedAt is updated on an already-tracked item, you may want GCManager to reschedule (see suggested change in GCManager.trackEligibleItem).
111-117: updateGcTime “max” rule matches prior semantics; note extended windows post-mark.Using Math.max prevents shortening GC windows. If gcTime changes after marking, the next scan might fire early once; GCManager handles this, but rescheduling (as suggested) can avoid the extra scan.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.changeset/smart-crabs-flow.md(1 hunks)packages/query-core/src/__tests__/gcManager.test.tsx(1 hunks)packages/query-core/src/__tests__/mutationCache.test.tsx(1 hunks)packages/query-core/src/gcManager.ts(1 hunks)packages/query-core/src/mutation.ts(6 hunks)packages/query-core/src/query.ts(8 hunks)packages/query-core/src/queryClient.ts(7 hunks)packages/query-core/src/removable.ts(2 hunks)packages/react-query/src/__tests__/useMutation.test.tsx(1 hunks)packages/react-query/src/__tests__/useQuery.test.tsx(2 hunks)packages/solid-query/src/__tests__/useMutation.test.tsx(1 hunks)packages/solid-query/src/__tests__/useQuery.test.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
packages/solid-query/src/__tests__/useQuery.test.tsx (3)
packages/query-core/src/gcManager.ts (1)
item(162-180)packages/query-core/src/queryObserver.ts (1)
query(704-721)packages/solid-query/src/useQuery.ts (1)
useQuery(36-50)
packages/query-core/src/queryClient.ts (3)
packages/query-core/src/gcManager.ts (1)
GCManager(24-189)packages/query-core/src/utils.ts (1)
isServer(78-78)packages/query-core/src/index.ts (1)
isServer(30-30)
packages/query-core/src/query.ts (2)
packages/query-core/src/gcManager.ts (1)
GCManager(24-189)packages/query-core/src/types.ts (1)
QueryOptions(225-278)
packages/query-core/src/__tests__/gcManager.test.tsx (1)
packages/query-core/src/gcManager.ts (2)
GCManager(24-189)item(162-180)
packages/react-query/src/__tests__/useQuery.test.tsx (1)
packages/query-core/src/gcManager.ts (1)
item(162-180)
packages/query-core/src/removable.ts (2)
packages/query-core/src/utils.ts (1)
isValidTimeout(93-95)packages/query-core/src/gcManager.ts (1)
GCManager(24-189)
packages/query-core/src/mutation.ts (2)
packages/query-core/src/gcManager.ts (1)
GCManager(24-189)packages/query-core/src/mutationObserver.ts (1)
MutationObserver(23-211)
🔇 Additional comments (14)
packages/react-query/src/__tests__/useMutation.test.tsx (1)
905-905: LGTM: Timing adjustment accounts for GC batching.The +1ms adjustment correctly accounts for the GCManager's microtask-based batching overhead introduced in this PR, ensuring the mutation is fully garbage collected after unmounting.
packages/solid-query/src/__tests__/useMutation.test.tsx (1)
992-992: LGTM: Consistent timing adjustment for GC batching.The +1ms adjustment mirrors the react-query test change and correctly accounts for the GCManager's batching overhead.
packages/solid-query/src/__tests__/useQuery.test.tsx (3)
1-1: LGTM: Proper fake timer cleanup.The
afterEachhook ensures fake timers don't leak between tests, which is essential when some tests usevi.useFakeTimers().Also applies to: 41-43
3906-3931: LGTM: Improved GC test validation forInfinitygcTime.The test now validates GC behavior via state inspection (
gcMarkedAt === null) rather than implementation details (setTimeout spying). This is more robust and directly verifies that queries withgcTime: Infinityare never marked for garbage collection.
3933-3971: LGTM: Comprehensive GC lifecycle validation.The refactored test validates the complete GC flow:
- Query is initially unmarked (
gcMarkedAt === null)- On unmount, query is marked with a timestamp
- After
gcTimeelapses, query is garbage collectedThis approach directly tests the GC behavior with the centralized GCManager rather than relying on implementation details like setTimeout calls. The use of fake timers with explicit system time control makes the test deterministic and robust.
.changeset/smart-crabs-flow.md (1)
1-5: Changeset looks goodPatch bump with clear, minimal description of the internal GCManager change. No user‑facing API promises are made here.
packages/query-core/src/query.ts (2)
231-239: optionalRemove() is now public and returns booleanThis effectively expands the public surface. Ensure downstream typings (and docs) reflect this and that no external code relied on the previous protected/void shape.
361-390: Observer‑driven GC logic looks solidclearGcMark on addObserver, and markForGc on last‑observer removal/finally only when isSafeToRemove() keeps semantics tight; paused fetches won’t be GC’d. LGTM.
Also applies to: 620-626
packages/query-core/src/mutation.ts (3)
128-131: Accessor looks good.getGcManager() correctly delegates to the client and keeps GC wiring encapsulated.
136-139: Observer add/remove wiring to GC is correct.
- addObserver clears GC mark.
- removeObserver re-marks only when safe to remove.
Looks consistent with the new centralized GC semantics.If observers can be added/removed in quick succession, please ensure unit tests cover “remove last observer while status flips pending→success” so markForGc is hit once as expected.
Also applies to: 150-153
161-164: Remove safety predicate is minimal but correct.isSafeToRemove() guards against GC while pending and requires zero observers. Good.
packages/query-core/src/gcManager.ts (1)
35-80: Scheduling flow looks sound.Microtask batching followed by a single timeout, with clearing/re-scheduling, is correct and avoids timer storms.
packages/query-core/src/removable.ts (2)
60-64: clearGcMark correctly untracks and nulls the mark.Good guard via GCManager.untrackEligibleItem no-op if not tracked.
74-83: Eligibility and timestamp calculations look correct.
- Infinity is treated as “never GC.”
- Date arithmetic is straightforward and uses >= for inclusive eligibility.
Also applies to: 91-101
MrFlashAccount
changed the title
1 participant
🎯 Changes
This PR introduces a centralized
GCManagerthat consolidates individual timeouts across queries and mutations into a single dynamically-scheduled timeout, resulting in 10-20x performance improvements for garbage collection operations. This addresses scalability issues in applications with thousands of active queries and mutations.Demo: https://react-19-query-demo-git-tanstac-d7ec69-mrflashaccounts-projects.vercel.app/
Performance difference
Before:After:
Problem
Previously, each query and mutation scheduled its own individual timeout for garbage collection. In applications with hundreds or thousands of queries:
setTimeout, leading to hundreds or thousands of timers on the event loopTimeoutManagerdocs, thousands of timeouts can hit platform limitationsSolution
The new
GCManagerimplements a centralized timeout scheduling approach:gcMarkedAt) when they become eligible for GCGCManagerschedules a single timeout for when the nearest item becomes eligible for removalKey Design Decisions
queueMicrotaskto batch calculations and schedules a timeout for when the nearest item becomes eligible, ensuring we only scan when necessaryPerformance Impact
Benchmarking shows 10-20x performance improvements for GC operations, particularly noticeable when:
The centralized approach reduces:
Migration Notes
This change is fully backward compatible. No API changes are required for users. The improvement is transparent and automatic.
The only behavioral difference is internal: GC operations are now batched and more efficient, which may result in slightly different timing in edge cases, but the overall behavior (items being collected after their
gcTimeexpires) remains identical.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Release Notes
New Features
Tests