Skip to content

Commit 43ecbfa

Browse files
amiran-gorgazjanautofix-ci[bot]samwillis
authored
Use in-memory queue for faster GC registration (#1326)
* Use in-memory queue instead of directly calling setTimeout to avoid performance issues. * Fix failing tests and add missing failure case. Made-with: Cursor * Added changeset. * Fix electric-db-collection test failure due to change in synchronicity. * ci: apply automated fixes * docs(db): add JSDoc comments to cleanup queue Document the cleanup queue's batching behavior and single-timeout design to make the GC scheduling flow easier to understand. Made-with: Cursor --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Sam Willis <sam.willis@gmail.com>
1 parent 045fd50 commit 43ecbfa

File tree

6 files changed

+295
-41
lines changed

6 files changed

+295
-41
lines changed

.changeset/gc-cleanup-queue.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/db': patch
3+
---
4+
5+
fix: Optimized unmount performance by batching cleanup tasks in a central queue.
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
type CleanupTask = {
2+
executeAt: number
3+
callback: () => void
4+
}
5+
6+
/**
7+
* Batches many GC registrations behind a single shared timeout.
8+
*/
9+
export class CleanupQueue {
10+
private static instance: CleanupQueue | null = null
11+
12+
private tasks: Map<unknown, CleanupTask> = new Map()
13+
14+
private timeoutId: ReturnType<typeof setTimeout> | null = null
15+
private microtaskScheduled = false
16+
17+
private constructor() {}
18+
19+
public static getInstance(): CleanupQueue {
20+
if (!CleanupQueue.instance) {
21+
CleanupQueue.instance = new CleanupQueue()
22+
}
23+
return CleanupQueue.instance
24+
}
25+
26+
/**
27+
* Queues a cleanup task and defers timeout selection to a microtask so
28+
* multiple synchronous registrations can share one root timer.
29+
*/
30+
public schedule(key: unknown, gcTime: number, callback: () => void): void {
31+
const executeAt = Date.now() + gcTime
32+
this.tasks.set(key, { executeAt, callback })
33+
34+
if (!this.microtaskScheduled) {
35+
this.microtaskScheduled = true
36+
Promise.resolve().then(() => {
37+
this.microtaskScheduled = false
38+
this.updateTimeout()
39+
})
40+
}
41+
}
42+
43+
public cancel(key: unknown): void {
44+
this.tasks.delete(key)
45+
}
46+
47+
/**
48+
* Keeps only one active timeout: whichever task is due next.
49+
*/
50+
private updateTimeout(): void {
51+
if (this.timeoutId !== null) {
52+
clearTimeout(this.timeoutId)
53+
this.timeoutId = null
54+
}
55+
56+
if (this.tasks.size === 0) {
57+
return
58+
}
59+
60+
let earliestTime = Infinity
61+
for (const task of this.tasks.values()) {
62+
if (task.executeAt < earliestTime) {
63+
earliestTime = task.executeAt
64+
}
65+
}
66+
67+
const delay = Math.max(0, earliestTime - Date.now())
68+
this.timeoutId = setTimeout(() => this.process(), delay)
69+
}
70+
71+
/**
72+
* Runs every task whose deadline has passed, then schedules the next wakeup
73+
* if there is still pending work.
74+
*/
75+
private process(): void {
76+
this.timeoutId = null
77+
const now = Date.now()
78+
for (const [key, task] of this.tasks.entries()) {
79+
if (now >= task.executeAt) {
80+
this.tasks.delete(key)
81+
try {
82+
task.callback()
83+
} catch (error) {
84+
console.error('Error in CleanupQueue task:', error)
85+
}
86+
}
87+
}
88+
89+
if (this.tasks.size > 0) {
90+
this.updateTimeout()
91+
}
92+
}
93+
94+
/**
95+
* Resets the singleton instance for tests.
96+
*/
97+
public static resetInstance(): void {
98+
if (CleanupQueue.instance) {
99+
if (CleanupQueue.instance.timeoutId !== null) {
100+
clearTimeout(CleanupQueue.instance.timeoutId)
101+
}
102+
CleanupQueue.instance = null
103+
}
104+
}
105+
}

packages/db/src/collection/lifecycle.ts

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
safeCancelIdleCallback,
88
safeRequestIdleCallback,
99
} from '../utils/browser-polyfills'
10+
import { CleanupQueue } from './cleanup-queue'
1011
import type { IdleCallbackDeadline } from '../utils/browser-polyfills'
1112
import type { StandardSchemaV1 } from '@standard-schema/spec'
1213
import type { CollectionConfig, CollectionStatus } from '../types'
@@ -34,7 +35,6 @@ export class CollectionLifecycleManager<
3435
public hasBeenReady = false
3536
public hasReceivedFirstCommit = false
3637
public onFirstReadyCallbacks: Array<() => void> = []
37-
public gcTimeoutId: ReturnType<typeof setTimeout> | null = null
3838
private idleCallbackId: number | null = null
3939

4040
/**
@@ -174,10 +174,6 @@ export class CollectionLifecycleManager<
174174
* Called when the collection becomes inactive (no subscribers)
175175
*/
176176
public startGCTimer(): void {
177-
if (this.gcTimeoutId) {
178-
clearTimeout(this.gcTimeoutId)
179-
}
180-
181177
const gcTime = this.config.gcTime ?? 300000 // 5 minutes default
182178

183179
// If gcTime is 0, negative, or non-finite (Infinity, -Infinity, NaN), GC is disabled.
@@ -187,23 +183,20 @@ export class CollectionLifecycleManager<
187183
return
188184
}
189185

190-
this.gcTimeoutId = setTimeout(() => {
186+
CleanupQueue.getInstance().schedule(this, gcTime, () => {
191187
if (this.changes.activeSubscribersCount === 0) {
192188
// Schedule cleanup during idle time to avoid blocking the UI thread
193189
this.scheduleIdleCleanup()
194190
}
195-
}, gcTime)
191+
})
196192
}
197193

198194
/**
199195
* Cancel the garbage collection timer
200196
* Called when the collection becomes active again
201197
*/
202198
public cancelGCTimer(): void {
203-
if (this.gcTimeoutId) {
204-
clearTimeout(this.gcTimeoutId)
205-
this.gcTimeoutId = null
206-
}
199+
CleanupQueue.getInstance().cancel(this)
207200
// Also cancel any pending idle cleanup
208201
if (this.idleCallbackId !== null) {
209202
safeCancelIdleCallback(this.idleCallbackId)
@@ -258,10 +251,7 @@ export class CollectionLifecycleManager<
258251
this.changes.cleanup()
259252
this.indexes.cleanup()
260253

261-
if (this.gcTimeoutId) {
262-
clearTimeout(this.gcTimeoutId)
263-
this.gcTimeoutId = null
264-
}
254+
CleanupQueue.getInstance().cancel(this)
265255

266256
this.hasBeenReady = false
267257

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
2+
import { CleanupQueue } from '../src/collection/cleanup-queue'
3+
4+
describe('CleanupQueue', () => {
5+
beforeEach(() => {
6+
vi.useFakeTimers()
7+
CleanupQueue.resetInstance()
8+
})
9+
10+
afterEach(() => {
11+
vi.useRealTimers()
12+
CleanupQueue.resetInstance()
13+
})
14+
15+
it('batches setTimeout creations across multiple synchronous schedules', async () => {
16+
const queue = CleanupQueue.getInstance()
17+
const cb1 = vi.fn()
18+
const cb2 = vi.fn()
19+
20+
const spySetTimeout = vi.spyOn(global, 'setTimeout')
21+
22+
queue.schedule('key1', 1000, cb1)
23+
queue.schedule('key2', 1000, cb2)
24+
25+
expect(spySetTimeout).not.toHaveBeenCalled()
26+
27+
// Process microtasks
28+
await Promise.resolve()
29+
30+
// Should only create a single timeout for the earliest scheduled task
31+
expect(spySetTimeout).toHaveBeenCalledTimes(1)
32+
})
33+
34+
it('executes callbacks after delay', async () => {
35+
const queue = CleanupQueue.getInstance()
36+
const cb1 = vi.fn()
37+
38+
queue.schedule('key1', 1000, cb1)
39+
40+
await Promise.resolve()
41+
42+
expect(cb1).not.toHaveBeenCalled()
43+
44+
vi.advanceTimersByTime(500)
45+
expect(cb1).not.toHaveBeenCalled()
46+
47+
vi.advanceTimersByTime(500)
48+
expect(cb1).toHaveBeenCalledTimes(1)
49+
})
50+
51+
it('can cancel tasks before they run', async () => {
52+
const queue = CleanupQueue.getInstance()
53+
const cb1 = vi.fn()
54+
55+
queue.schedule('key1', 1000, cb1)
56+
57+
await Promise.resolve()
58+
59+
queue.cancel('key1')
60+
61+
vi.advanceTimersByTime(1000)
62+
expect(cb1).not.toHaveBeenCalled()
63+
})
64+
65+
it('schedules subsequent tasks properly if earlier tasks are cancelled', async () => {
66+
const queue = CleanupQueue.getInstance()
67+
const cb1 = vi.fn()
68+
const cb2 = vi.fn()
69+
70+
queue.schedule('key1', 1000, cb1)
71+
queue.schedule('key2', 2000, cb2)
72+
73+
await Promise.resolve()
74+
75+
queue.cancel('key1')
76+
77+
// At 1000ms, process will be called because of the original timeout, but no callbacks will trigger
78+
vi.advanceTimersByTime(1000)
79+
expect(cb1).not.toHaveBeenCalled()
80+
expect(cb2).not.toHaveBeenCalled()
81+
82+
// It should automatically schedule the next timeout for key2
83+
vi.advanceTimersByTime(1000)
84+
expect(cb2).toHaveBeenCalledTimes(1)
85+
})
86+
87+
it('processes multiple tasks that have expired at the same time', async () => {
88+
const queue = CleanupQueue.getInstance()
89+
const cb1 = vi.fn()
90+
const cb2 = vi.fn()
91+
const cb3 = vi.fn()
92+
93+
queue.schedule('key1', 1000, cb1)
94+
queue.schedule('key2', 1500, cb2)
95+
queue.schedule('key3', 1500, cb3)
96+
97+
await Promise.resolve()
98+
99+
vi.advanceTimersByTime(1000)
100+
expect(cb1).toHaveBeenCalledTimes(1)
101+
expect(cb2).not.toHaveBeenCalled()
102+
103+
vi.advanceTimersByTime(500)
104+
expect(cb2).toHaveBeenCalledTimes(1)
105+
expect(cb3).toHaveBeenCalledTimes(1)
106+
})
107+
108+
it('continues processing tasks if one throws an error', async () => {
109+
const queue = CleanupQueue.getInstance()
110+
const cb1 = vi.fn().mockImplementation(() => {
111+
throw new Error('Test error')
112+
})
113+
const cb2 = vi.fn()
114+
115+
const spyConsoleError = vi
116+
.spyOn(console, 'error')
117+
.mockImplementation(() => {})
118+
119+
queue.schedule('key1', 1000, cb1)
120+
queue.schedule('key2', 1000, cb2)
121+
122+
await Promise.resolve()
123+
124+
vi.advanceTimersByTime(1000)
125+
126+
expect(cb1).toHaveBeenCalledTimes(1)
127+
expect(spyConsoleError).toHaveBeenCalledWith(
128+
'Error in CleanupQueue task:',
129+
expect.any(Error),
130+
)
131+
// cb2 should still be called even though cb1 threw an error
132+
expect(cb2).toHaveBeenCalledTimes(1)
133+
134+
spyConsoleError.mockRestore()
135+
})
136+
})

0 commit comments

Comments
 (0)