From 444a9f2a63fced2da8831da2f67a6ef6b29e8b9d Mon Sep 17 00:00:00 2001 From: amiran-gorgazjan Date: Tue, 3 Mar 2026 11:02:19 +0100 Subject: [PATCH 1/4] Use in-memory queue instead of directly calling setTimeout to avoid performance issues. --- packages/db/src/collection/cleanup-queue.ts | 86 ++++++++++++++++ packages/db/src/collection/lifecycle.ts | 20 +--- packages/db/tests/cleanup-queue.test.ts | 107 ++++++++++++++++++++ 3 files changed, 198 insertions(+), 15 deletions(-) create mode 100644 packages/db/src/collection/cleanup-queue.ts create mode 100644 packages/db/tests/cleanup-queue.test.ts diff --git a/packages/db/src/collection/cleanup-queue.ts b/packages/db/src/collection/cleanup-queue.ts new file mode 100644 index 000000000..ffa191ecb --- /dev/null +++ b/packages/db/src/collection/cleanup-queue.ts @@ -0,0 +1,86 @@ +type CleanupTask = { + executeAt: number + callback: () => void +} + +export class CleanupQueue { + private static instance: CleanupQueue | null = null + + private tasks: Map = new Map() + + private timeoutId: ReturnType | null = null + private microtaskScheduled = false + + private constructor() {} + + public static getInstance(): CleanupQueue { + if (!CleanupQueue.instance) { + CleanupQueue.instance = new CleanupQueue() + } + return CleanupQueue.instance + } + + public schedule(key: unknown, gcTime: number, callback: () => void): void { + const executeAt = Date.now() + gcTime + this.tasks.set(key, { executeAt, callback }) + + if (!this.microtaskScheduled) { + this.microtaskScheduled = true + Promise.resolve().then(() => { + this.microtaskScheduled = false + this.updateTimeout() + }) + } + } + + public cancel(key: unknown): void { + this.tasks.delete(key) + } + + private updateTimeout(): void { + if (this.timeoutId !== null) { + clearTimeout(this.timeoutId) + this.timeoutId = null + } + + if (this.tasks.size === 0) { + return + } + + let earliestTime = Infinity + for (const task of this.tasks.values()) { + if (task.executeAt < earliestTime) { + earliestTime = task.executeAt + } + } + + const delay = Math.max(0, earliestTime - Date.now()) + this.timeoutId = setTimeout(() => this.process(), delay) + } + + private process(): void { + this.timeoutId = null + const now = Date.now() + + for (const [key, task] of this.tasks.entries()) { + if (now >= task.executeAt) { + this.tasks.delete(key) + task.callback() + } + } + + if (this.tasks.size > 0) { + this.updateTimeout() + } + } + + // Only used for testing to clean up state + public static resetInstance(): void { + if (CleanupQueue.instance) { + if (CleanupQueue.instance.timeoutId !== null) { + clearTimeout(CleanupQueue.instance.timeoutId) + } + CleanupQueue.instance = null + } + } +} diff --git a/packages/db/src/collection/lifecycle.ts b/packages/db/src/collection/lifecycle.ts index 2225b35aa..3010e3467 100644 --- a/packages/db/src/collection/lifecycle.ts +++ b/packages/db/src/collection/lifecycle.ts @@ -7,6 +7,7 @@ import { safeCancelIdleCallback, safeRequestIdleCallback, } from '../utils/browser-polyfills' +import { CleanupQueue } from './cleanup-queue' import type { IdleCallbackDeadline } from '../utils/browser-polyfills' import type { StandardSchemaV1 } from '@standard-schema/spec' import type { CollectionConfig, CollectionStatus } from '../types' @@ -34,7 +35,6 @@ export class CollectionLifecycleManager< public hasBeenReady = false public hasReceivedFirstCommit = false public onFirstReadyCallbacks: Array<() => void> = [] - public gcTimeoutId: ReturnType | null = null private idleCallbackId: number | null = null /** @@ -174,10 +174,6 @@ export class CollectionLifecycleManager< * Called when the collection becomes inactive (no subscribers) */ public startGCTimer(): void { - if (this.gcTimeoutId) { - clearTimeout(this.gcTimeoutId) - } - const gcTime = this.config.gcTime ?? 300000 // 5 minutes default // If gcTime is 0, negative, or non-finite (Infinity, -Infinity, NaN), GC is disabled. @@ -187,12 +183,12 @@ export class CollectionLifecycleManager< return } - this.gcTimeoutId = setTimeout(() => { + CleanupQueue.getInstance().schedule(this, gcTime, () => { if (this.changes.activeSubscribersCount === 0) { // Schedule cleanup during idle time to avoid blocking the UI thread this.scheduleIdleCleanup() } - }, gcTime) + }) } /** @@ -200,10 +196,7 @@ export class CollectionLifecycleManager< * Called when the collection becomes active again */ public cancelGCTimer(): void { - if (this.gcTimeoutId) { - clearTimeout(this.gcTimeoutId) - this.gcTimeoutId = null - } + CleanupQueue.getInstance().cancel(this) // Also cancel any pending idle cleanup if (this.idleCallbackId !== null) { safeCancelIdleCallback(this.idleCallbackId) @@ -258,10 +251,7 @@ export class CollectionLifecycleManager< this.changes.cleanup() this.indexes.cleanup() - if (this.gcTimeoutId) { - clearTimeout(this.gcTimeoutId) - this.gcTimeoutId = null - } + CleanupQueue.getInstance().cancel(this) this.hasBeenReady = false diff --git a/packages/db/tests/cleanup-queue.test.ts b/packages/db/tests/cleanup-queue.test.ts new file mode 100644 index 000000000..ecf5c5d94 --- /dev/null +++ b/packages/db/tests/cleanup-queue.test.ts @@ -0,0 +1,107 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { CleanupQueue } from '../src/collection/cleanup-queue' + +describe('CleanupQueue', () => { + beforeEach(() => { + vi.useFakeTimers() + CleanupQueue.resetInstance() + }) + + afterEach(() => { + vi.useRealTimers() + CleanupQueue.resetInstance() + }) + + it('batches setTimeout creations across multiple synchronous schedules', async () => { + const queue = CleanupQueue.getInstance() + const cb1 = vi.fn() + const cb2 = vi.fn() + + const spySetTimeout = vi.spyOn(global, 'setTimeout') + + queue.schedule('key1', 1000, cb1) + queue.schedule('key2', 1000, cb2) + + expect(spySetTimeout).not.toHaveBeenCalled() + + // Process microtasks + await Promise.resolve() + + // Should only create a single timeout for the earliest scheduled task + expect(spySetTimeout).toHaveBeenCalledTimes(1) + }) + + it('executes callbacks after delay', async () => { + const queue = CleanupQueue.getInstance() + const cb1 = vi.fn() + + queue.schedule('key1', 1000, cb1) + + await Promise.resolve() + + expect(cb1).not.toHaveBeenCalled() + + vi.advanceTimersByTime(500) + expect(cb1).not.toHaveBeenCalled() + + vi.advanceTimersByTime(500) + expect(cb1).toHaveBeenCalledTimes(1) + }) + + it('can cancel tasks before they run', async () => { + const queue = CleanupQueue.getInstance() + const cb1 = vi.fn() + + queue.schedule('key1', 1000, cb1) + + await Promise.resolve() + + queue.cancel('key1') + + vi.advanceTimersByTime(1000) + expect(cb1).not.toHaveBeenCalled() + }) + + it('schedules subsequent tasks properly if earlier tasks are cancelled', async () => { + const queue = CleanupQueue.getInstance() + const cb1 = vi.fn() + const cb2 = vi.fn() + + queue.schedule('key1', 1000, cb1) + queue.schedule('key2', 2000, cb2) + + await Promise.resolve() + + queue.cancel('key1') + + // At 1000ms, process will be called because of the original timeout, but no callbacks will trigger + vi.advanceTimersByTime(1000) + expect(cb1).not.toHaveBeenCalled() + expect(cb2).not.toHaveBeenCalled() + + // It should automatically schedule the next timeout for key2 + vi.advanceTimersByTime(1000) + expect(cb2).toHaveBeenCalledTimes(1) + }) + + it('processes multiple tasks that have expired at the same time', async () => { + const queue = CleanupQueue.getInstance() + const cb1 = vi.fn() + const cb2 = vi.fn() + const cb3 = vi.fn() + + queue.schedule('key1', 1000, cb1) + queue.schedule('key2', 1500, cb2) + queue.schedule('key3', 1500, cb3) + + await Promise.resolve() + + vi.advanceTimersByTime(1000) + expect(cb1).toHaveBeenCalledTimes(1) + expect(cb2).not.toHaveBeenCalled() + + vi.advanceTimersByTime(500) + expect(cb2).toHaveBeenCalledTimes(1) + expect(cb3).toHaveBeenCalledTimes(1) + }) +}) From 0110bf93d2fa8af3b924b2edc222323c45b47d75 Mon Sep 17 00:00:00 2001 From: amiran-gorgazjan Date: Wed, 4 Mar 2026 05:21:57 +0100 Subject: [PATCH 2/4] Fix failing tests and add missing failure case. Made-with: Cursor --- packages/db/src/collection/cleanup-queue.ts | 6 ++- packages/db/tests/cleanup-queue.test.ts | 24 +++++++++ .../db/tests/collection-lifecycle.test.ts | 50 ++++++++++--------- 3 files changed, 55 insertions(+), 25 deletions(-) diff --git a/packages/db/src/collection/cleanup-queue.ts b/packages/db/src/collection/cleanup-queue.ts index ffa191ecb..45dc1e8c6 100644 --- a/packages/db/src/collection/cleanup-queue.ts +++ b/packages/db/src/collection/cleanup-queue.ts @@ -65,7 +65,11 @@ export class CleanupQueue { for (const [key, task] of this.tasks.entries()) { if (now >= task.executeAt) { this.tasks.delete(key) - task.callback() + try { + task.callback() + } catch (error) { + console.error('Error in CleanupQueue task:', error) + } } } diff --git a/packages/db/tests/cleanup-queue.test.ts b/packages/db/tests/cleanup-queue.test.ts index ecf5c5d94..24fc00da7 100644 --- a/packages/db/tests/cleanup-queue.test.ts +++ b/packages/db/tests/cleanup-queue.test.ts @@ -104,4 +104,28 @@ describe('CleanupQueue', () => { expect(cb2).toHaveBeenCalledTimes(1) expect(cb3).toHaveBeenCalledTimes(1) }) + + it('continues processing tasks if one throws an error', async () => { + const queue = CleanupQueue.getInstance() + const cb1 = vi.fn().mockImplementation(() => { + throw new Error('Test error') + }) + const cb2 = vi.fn() + + const spyConsoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + + queue.schedule('key1', 1000, cb1) + queue.schedule('key2', 1000, cb2) + + await Promise.resolve() + + vi.advanceTimersByTime(1000) + + expect(cb1).toHaveBeenCalledTimes(1) + expect(spyConsoleError).toHaveBeenCalledWith('Error in CleanupQueue task:', expect.any(Error)) + // cb2 should still be called even though cb1 threw an error + expect(cb2).toHaveBeenCalledTimes(1) + + spyConsoleError.mockRestore() + }) }) diff --git a/packages/db/tests/collection-lifecycle.test.ts b/packages/db/tests/collection-lifecycle.test.ts index 8b70b3ddc..140d0a7e2 100644 --- a/packages/db/tests/collection-lifecycle.test.ts +++ b/packages/db/tests/collection-lifecycle.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { createCollection } from '../src/collection/index.js' +import { CleanupQueue } from '../src/collection/cleanup-queue.js' // Mock setTimeout and clearTimeout for testing GC behavior const originalSetTimeout = global.setTimeout @@ -10,6 +11,8 @@ describe(`Collection Lifecycle Management`, () => { let mockClearTimeout: ReturnType let timeoutCallbacks: Map void> let timeoutId = 1 + let scheduleSpy: ReturnType + let cancelSpy: ReturnType beforeEach(() => { timeoutCallbacks = new Map() @@ -31,22 +34,18 @@ describe(`Collection Lifecycle Management`, () => { global.setTimeout = mockSetTimeout as any global.clearTimeout = mockClearTimeout as any + + scheduleSpy = vi.spyOn(CleanupQueue.prototype, 'schedule').mockImplementation(() => {}) + cancelSpy = vi.spyOn(CleanupQueue.prototype, 'cancel').mockImplementation(() => {}) }) afterEach(() => { global.setTimeout = originalSetTimeout global.clearTimeout = originalClearTimeout vi.clearAllMocks() + CleanupQueue.resetInstance() }) - const triggerTimeout = (id: number) => { - const callback = timeoutCallbacks.get(id) - if (callback) { - callback() - timeoutCallbacks.delete(id) - } - } - const triggerAllTimeouts = () => { const callbacks = Array.from(timeoutCallbacks.entries()) callbacks.forEach(([id, callback]) => { @@ -247,10 +246,10 @@ describe(`Collection Lifecycle Management`, () => { expect(collection.subscriberCount).toBe(0) // Should start GC timer each time - expect(mockSetTimeout).toHaveBeenCalledWith(expect.any(Function), 1000) + expect(scheduleSpy).toHaveBeenCalledWith(expect.any(Object), 1000, expect.any(Function)) } - expect(mockSetTimeout).toHaveBeenCalledTimes(5) + expect(scheduleSpy).toHaveBeenCalledTimes(5) }) }) @@ -268,12 +267,12 @@ describe(`Collection Lifecycle Management`, () => { const subscription = collection.subscribeChanges(() => {}) // Should not have GC timer while there are subscribers - expect(mockSetTimeout).not.toHaveBeenCalled() + expect(scheduleSpy).not.toHaveBeenCalled() subscription.unsubscribe() // Should start GC timer when last subscriber is removed - expect(mockSetTimeout).toHaveBeenCalledWith(expect.any(Function), 5000) + expect(scheduleSpy).toHaveBeenCalledWith(expect.any(Object), 5000, expect.any(Function)) }) it(`should cancel GC timer when new subscriber is added`, () => { @@ -289,12 +288,11 @@ describe(`Collection Lifecycle Management`, () => { const subscription1 = collection.subscribeChanges(() => {}) subscription1.unsubscribe() - expect(mockSetTimeout).toHaveBeenCalledTimes(1) - const timerId = mockSetTimeout.mock.results[0]?.value + expect(scheduleSpy).toHaveBeenCalledTimes(1) // Add new subscriber should cancel GC timer const subscription2 = collection.subscribeChanges(() => {}) - expect(mockClearTimeout).toHaveBeenCalledWith(timerId) + expect(cancelSpy).toHaveBeenCalledWith(expect.any(Object)) subscription2.unsubscribe() }) @@ -315,9 +313,11 @@ describe(`Collection Lifecycle Management`, () => { expect(collection.status).toBe(`loading`) // Trigger GC timeout - this will schedule the idle cleanup - const gcTimerId = mockSetTimeout.mock.results[0]?.value - if (gcTimerId) { - triggerTimeout(gcTimerId) + const gcCallback = scheduleSpy.mock.calls[0]?.[2] as + | (() => void) + | undefined + if (gcCallback) { + gcCallback() } // Now trigger all remaining timeouts to handle the idle callback @@ -340,7 +340,7 @@ describe(`Collection Lifecycle Management`, () => { subscription.unsubscribe() // Should use default 5 minutes (300000ms) - expect(mockSetTimeout).toHaveBeenCalledWith(expect.any(Function), 300000) + expect(scheduleSpy).toHaveBeenCalledWith(expect.any(Object), 300000, expect.any(Function)) }) it(`should disable GC when gcTime is 0`, () => { @@ -357,7 +357,7 @@ describe(`Collection Lifecycle Management`, () => { subscription.unsubscribe() // Should not start any timer when GC is disabled - expect(mockSetTimeout).not.toHaveBeenCalled() + expect(scheduleSpy).not.toHaveBeenCalled() expect(collection.status).not.toBe(`cleaned-up`) }) @@ -377,7 +377,7 @@ describe(`Collection Lifecycle Management`, () => { // Should not start any timer when gcTime is Infinity // Note: Without this fix, setTimeout(fn, Infinity) would coerce to 0, // causing immediate GC instead of never collecting - expect(mockSetTimeout).not.toHaveBeenCalled() + expect(scheduleSpy).not.toHaveBeenCalled() expect(collection.status).not.toBe(`cleaned-up`) }) }) @@ -525,9 +525,11 @@ describe(`Collection Lifecycle Management`, () => { }) // Trigger GC timeout to schedule cleanup - const gcTimerId = mockSetTimeout.mock.results[0]?.value - if (gcTimerId) { - triggerTimeout(gcTimerId) + const gcCallback = scheduleSpy.mock.calls[0]?.[2] as + | (() => void) + | undefined + if (gcCallback) { + gcCallback() } // Trigger all remaining timeouts to handle the idle callback From 908d9e924f035e692ab90c3c074b362d9b44fd0f Mon Sep 17 00:00:00 2001 From: amiran-gorgazjan Date: Wed, 4 Mar 2026 05:41:07 +0100 Subject: [PATCH 3/4] Added changeset. --- .changeset/gc-cleanup-queue.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/gc-cleanup-queue.md diff --git a/.changeset/gc-cleanup-queue.md b/.changeset/gc-cleanup-queue.md new file mode 100644 index 000000000..3f822b5a9 --- /dev/null +++ b/.changeset/gc-cleanup-queue.md @@ -0,0 +1,5 @@ +--- +'@tanstack/db': patch +--- + +fix: Optimized unmount performance by batching cleanup tasks in a central queue. From f1bafc1d7e51d966e28b1c053f034f8aef512899 Mon Sep 17 00:00:00 2001 From: amiran-gorgazjan Date: Wed, 4 Mar 2026 06:07:55 +0100 Subject: [PATCH 4/4] Fix electric-db-collection test failure due to change in synchronicity. --- packages/electric-db-collection/tests/electric.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/electric-db-collection/tests/electric.test.ts b/packages/electric-db-collection/tests/electric.test.ts index 8ed73a863..0010b6d17 100644 --- a/packages/electric-db-collection/tests/electric.test.ts +++ b/packages/electric-db-collection/tests/electric.test.ts @@ -3929,7 +3929,7 @@ describe(`Electric Integration`, () => { }) describe(`syncMode configuration - GC and resync`, () => { - it(`should resync after garbage collection and new subscription`, () => { + it(`should resync after garbage collection and new subscription`, async () => { // Use fake timers for this test vi.useFakeTimers() @@ -3979,7 +3979,7 @@ describe(`Electric Integration`, () => { expect(testCollection.size).toBe(2) // Fast-forward time to trigger GC (past the 100ms gcTime) - vi.advanceTimersByTime(150) + await vi.advanceTimersByTimeAsync(150) // Collection should be cleaned up expect(testCollection.status).toBe(`cleaned-up`)