perf: debounce team watcher rebuilds during dir-event bursts
A team launch creates many directories/files in quick succession (worktrees, inboxes, session logs), and each addDir/unlinkDir event triggered a full TeamTaskWatchRegistry reconcile that tore down and recreated the entire chokidar watcher (re-opening a kqueue fd per watched file on macOS). Profiling a 6-member mixed-team launch showed kqueue churn (kevent) as a top native cost and watcher rebuild as the top remaining main-thread JS cost after the transcript fix. Debounce the event-driven reconcile (250ms) so a burst collapses into one rebuild. collectTargets re-reads the current directory state and emitExistingFilesForNewTargets backfills files created before the rebuild, so no change is missed; requestReconcile, startup, and the periodic 30s reconcile stay immediate. Adds a test asserting a burst of addDir events yields a single rebuild.
This commit is contained in:
parent
e1475deede
commit
aa9a1bba8c
2 changed files with 73 additions and 3 deletions
|
|
@ -32,6 +32,12 @@ export interface TeamTaskWatchRegistryOptions {
|
||||||
|
|
||||||
const RECONCILE_INTERVAL_MS = 30_000;
|
const RECONCILE_INTERVAL_MS = 30_000;
|
||||||
|
|
||||||
|
// Coalesce bursts of directory add/remove events (e.g. a team launch creating
|
||||||
|
// many dirs/files) into a single target reconcile + watcher rebuild. collectTargets
|
||||||
|
// re-reads the current directory state, so a trailing reconcile still sees every
|
||||||
|
// change; this only avoids rebuilding the whole watcher once per event in a burst.
|
||||||
|
const RECONCILE_DEBOUNCE_MS = 250;
|
||||||
|
|
||||||
// Keep this list aligned with FileWatcher.processTeamsChange().
|
// Keep this list aligned with FileWatcher.processTeamsChange().
|
||||||
// If a new team artifact should produce TeamChangeEvent, add it here too.
|
// If a new team artifact should produce TeamChangeEvent, add it here too.
|
||||||
const TEAM_ROOT_FILES = new Set([
|
const TEAM_ROOT_FILES = new Set([
|
||||||
|
|
@ -70,6 +76,7 @@ export class TeamTaskWatchRegistry {
|
||||||
private generation = 0;
|
private generation = 0;
|
||||||
private reconcileInProgress = false;
|
private reconcileInProgress = false;
|
||||||
private reconcileAgain = false;
|
private reconcileAgain = false;
|
||||||
|
private reconcileDebounceTimer: NodeJS.Timeout | null = null;
|
||||||
|
|
||||||
constructor(private readonly options: TeamTaskWatchRegistryOptions) {}
|
constructor(private readonly options: TeamTaskWatchRegistryOptions) {}
|
||||||
|
|
||||||
|
|
@ -101,6 +108,26 @@ export class TeamTaskWatchRegistry {
|
||||||
await this.reconcileTargets();
|
await this.reconcileTargets();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Debounced target reconcile for high-frequency directory events. Bursts of
|
||||||
|
* add/remove dir events (notably while a team launch creates many dirs/files)
|
||||||
|
* collapse into a single rebuild after a short window instead of tearing down
|
||||||
|
* and recreating the whole watcher once per event. Correctness is preserved:
|
||||||
|
* collectTargets re-reads the current directory state, so the trailing reconcile
|
||||||
|
* still sees every change, and emitExistingFilesForNewTargets backfills files
|
||||||
|
* created before the rebuild.
|
||||||
|
*/
|
||||||
|
private scheduleReconcile(): void {
|
||||||
|
if (this.closed || this.reconcileDebounceTimer) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
this.reconcileDebounceTimer = setTimeout(() => {
|
||||||
|
this.reconcileDebounceTimer = null;
|
||||||
|
void this.reconcileTargets();
|
||||||
|
}, RECONCILE_DEBOUNCE_MS);
|
||||||
|
this.reconcileDebounceTimer.unref?.();
|
||||||
|
}
|
||||||
|
|
||||||
async close(): Promise<void> {
|
async close(): Promise<void> {
|
||||||
this.closed = true;
|
this.closed = true;
|
||||||
this.generation += 1;
|
this.generation += 1;
|
||||||
|
|
@ -109,6 +136,10 @@ export class TeamTaskWatchRegistry {
|
||||||
clearInterval(this.reconcileTimer);
|
clearInterval(this.reconcileTimer);
|
||||||
this.reconcileTimer = null;
|
this.reconcileTimer = null;
|
||||||
}
|
}
|
||||||
|
if (this.reconcileDebounceTimer) {
|
||||||
|
clearTimeout(this.reconcileDebounceTimer);
|
||||||
|
this.reconcileDebounceTimer = null;
|
||||||
|
}
|
||||||
|
|
||||||
const watcher = this.watcher;
|
const watcher = this.watcher;
|
||||||
this.watcher = null;
|
this.watcher = null;
|
||||||
|
|
@ -197,9 +228,10 @@ export class TeamTaskWatchRegistry {
|
||||||
}
|
}
|
||||||
|
|
||||||
// addDir/unlinkDir can make the watch target set stale immediately.
|
// addDir/unlinkDir can make the watch target set stale immediately.
|
||||||
// Periodic reconciliation is the backup path if the directory event is missed.
|
// Debounced so a burst of dir events (e.g. a team launch) coalesces into one
|
||||||
|
// rebuild; periodic reconciliation is the backup path if an event is missed.
|
||||||
if (this.shouldReconcile(eventType, relativePath)) {
|
if (this.shouldReconcile(eventType, relativePath)) {
|
||||||
void this.reconcileTargets();
|
this.scheduleReconcile();
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!this.shouldEmit(eventType, relativePath)) {
|
if (!this.shouldEmit(eventType, relativePath)) {
|
||||||
|
|
|
||||||
|
|
@ -6,7 +6,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||||
type MockChokidarWatcher = {
|
type MockChokidarWatcher = {
|
||||||
targets: string[];
|
targets: string[];
|
||||||
options: unknown;
|
options: unknown;
|
||||||
|
handlers: Map<string, Array<(...args: unknown[]) => void>>;
|
||||||
on: (event: string, handler: (...args: unknown[]) => void) => MockChokidarWatcher;
|
on: (event: string, handler: (...args: unknown[]) => void) => MockChokidarWatcher;
|
||||||
|
emit: (event: string, ...args: unknown[]) => void;
|
||||||
close: ReturnType<typeof vi.fn>;
|
close: ReturnType<typeof vi.fn>;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
@ -16,9 +18,18 @@ const chokidarMock = vi.hoisted(() => {
|
||||||
const watcher = {
|
const watcher = {
|
||||||
targets: (Array.isArray(targets) ? targets : [targets]).map((t) => String(t)),
|
targets: (Array.isArray(targets) ? targets : [targets]).map((t) => String(t)),
|
||||||
options,
|
options,
|
||||||
|
handlers: new Map<string, Array<(...args: unknown[]) => void>>(),
|
||||||
close: vi.fn().mockResolvedValue(undefined),
|
close: vi.fn().mockResolvedValue(undefined),
|
||||||
|
emit(event: string, ...args: unknown[]) {
|
||||||
|
for (const h of watcher.handlers.get(event) ?? []) h(...args);
|
||||||
|
},
|
||||||
} as MockChokidarWatcher;
|
} as MockChokidarWatcher;
|
||||||
watcher.on = vi.fn(() => watcher);
|
watcher.on = vi.fn((event: string, handler: (...args: unknown[]) => void) => {
|
||||||
|
const hs = watcher.handlers.get(event) ?? [];
|
||||||
|
hs.push(handler);
|
||||||
|
watcher.handlers.set(event, hs);
|
||||||
|
return watcher;
|
||||||
|
});
|
||||||
instances.push(watcher);
|
instances.push(watcher);
|
||||||
return watcher;
|
return watcher;
|
||||||
};
|
};
|
||||||
|
|
@ -156,4 +167,31 @@ describe('TeamTaskWatchRegistry scoping', () => {
|
||||||
|
|
||||||
expect(targets).toContain(path.normalize(path.join(root, 'beta')));
|
expect(targets).toContain(path.normalize(path.join(root, 'beta')));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('coalesces a burst of addDir events into a single watcher rebuild', async () => {
|
||||||
|
const registry = new TeamTaskWatchRegistry({
|
||||||
|
kind: 'teams',
|
||||||
|
rootPath: root,
|
||||||
|
onChange: () => {},
|
||||||
|
onError: () => {},
|
||||||
|
});
|
||||||
|
await registry.start();
|
||||||
|
const instancesAfterStart = chokidarMock.instances.length;
|
||||||
|
const watcher = chokidarMock.instances.at(-1) as MockChokidarWatcher;
|
||||||
|
|
||||||
|
// A new team dir appears, then a burst of addDir events fire for it.
|
||||||
|
fs.mkdirSync(path.join(root, 'delta', 'inboxes'), { recursive: true });
|
||||||
|
for (let i = 0; i < 4; i += 1) {
|
||||||
|
watcher.emit('addDir', path.join(root, 'delta'));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Wait past the debounce window for the single coalesced reconcile to run.
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 400));
|
||||||
|
const finalTargets = latestTargets();
|
||||||
|
await registry.close();
|
||||||
|
|
||||||
|
// Exactly one rebuild despite 4 addDir events, and it picked up the new dir.
|
||||||
|
expect(chokidarMock.instances.length).toBe(instancesAfterStart + 1);
|
||||||
|
expect(finalTargets).toContain(path.normalize(path.join(root, 'delta')));
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue