Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 129 additions & 0 deletions src/main/git-limiter.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import { describe, it, expect, beforeEach } from 'vitest'
import {
runGitRead,
gitLimiterStats,
resetGitLimiter,
MAX_CONCURRENT_GIT_READS
} from './git-limiter'

/** A promise plus its resolver, so a test can hold a git read open. */
function deferred(): { promise: Promise<void>; resolve: () => void } {
let resolve!: () => void
const promise = new Promise<void>((r) => {
resolve = r
})
return { promise, resolve }
}

/** Let the microtask queue drain so pending acquires settle. */
const tick = (): Promise<void> => new Promise((r) => setImmediate(r))

describe('git-limiter', () => {
beforeEach(() => {
resetGitLimiter()
})

it('runs up to the cap concurrently', async () => {
const gates = Array.from({ length: MAX_CONCURRENT_GIT_READS }, deferred)
let started = 0
const runs = gates.map((g) =>
runGitRead('interactive', async () => {
started++
await g.promise
})
)
await tick()

expect(started).toBe(MAX_CONCURRENT_GIT_READS)
gates.forEach((g) => g.resolve())
await Promise.all(runs)
})

it('queues work past the cap', async () => {
const gates = Array.from({ length: MAX_CONCURRENT_GIT_READS }, deferred)
let started = 0
const runs = gates.map((g) =>
runGitRead('interactive', async () => {
started++
await g.promise
})
)
const extra = runGitRead('interactive', async () => {
started++
})
await tick()

expect(started).toBe(MAX_CONCURRENT_GIT_READS)
expect(gitLimiterStats().interactiveQueued).toBe(1)

gates.forEach((g) => g.resolve())
await Promise.all([...runs, extra])
expect(started).toBe(MAX_CONCURRENT_GIT_READS + 1)
})

it('dequeues interactive work ahead of bulk work already waiting', async () => {
const gates = Array.from({ length: MAX_CONCURRENT_GIT_READS }, deferred)
const blockers = gates.map((g) => runGitRead('interactive', () => g.promise))
await tick()

const order: string[] = []
// The queued items hold their own permit open, so freeing one permit at a
// time makes the dequeue order directly observable.
const bulkGate = deferred()
const interactiveGate = deferred()
// Bulk enqueues first, interactive second — priority must still win.
const bulk = runGitRead('bulk', async () => {
order.push('bulk')
await bulkGate.promise
})
const interactive = runGitRead('interactive', async () => {
order.push('interactive')
await interactiveGate.promise
})
await tick()
expect(order).toEqual([])

gates[0].resolve()
await tick()
expect(order).toEqual(['interactive'])

gates[1].resolve()
await tick()
expect(order).toEqual(['interactive', 'bulk'])

gates.slice(2).forEach((g) => g.resolve())
bulkGate.resolve()
interactiveGate.resolve()
await Promise.all([...blockers, bulk, interactive])
})

it('releases the permit when the operation throws', async () => {
await expect(
runGitRead('interactive', async () => {
throw new Error('git exploded')
})
).rejects.toThrow('git exploded')

expect(gitLimiterStats().active).toBe(0)

// A subsequent read still gets a permit rather than hanging.
await expect(runGitRead('interactive', async () => 'ok')).resolves.toBe('ok')
})

it('drains bulk work once interactive work is done', async () => {
const done: number[] = []
const all = Array.from({ length: MAX_CONCURRENT_GIT_READS * 3 }, (_, i) =>
runGitRead(i % 2 === 0 ? 'bulk' : 'interactive', async () => {
done.push(i)
})
)
await Promise.all(all)

expect(done).toHaveLength(MAX_CONCURRENT_GIT_READS * 3)
expect(gitLimiterStats()).toEqual({
active: 0,
interactiveQueued: 0,
bulkQueued: 0
})
})
})
121 changes: 121 additions & 0 deletions src/main/git-limiter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
// Concurrency gate for read-only git subprocesses.
//
// Every panel read here is I/O-bound, not CPU-bound: a cold `git status
// --porcelain` in a large monorepo stats thousands of files at ~35% CPU while
// the main process sits idle. That makes the interesting number *not* total
// throughput but how long any one read waits behind the others.
//
// Measured on the reference monorepo (18 worktrees, `git status --porcelain`
// in each, warm cache), varying only the concurrency cap:
//
// cap=1 total 1626ms p50 49ms max 463ms
// cap=4 total 741ms p50 77ms max 495ms
// cap=8 total 718ms p50 155ms max 534ms
// cap=16 total 732ms p50 274ms max 619ms
// cap=64 total 766ms p50 220ms max 621ms
//
// Total wall time plateaus at cap=4 — past that, extra parallelism buys no
// throughput and only inflates per-call latency (p50 77ms → 274ms), because
// each read now shares the disk with 15 others instead of 3. So the cap is
// close to free, which is what makes the second half of this module possible.
//
// The second half is priority. A cap alone doesn't help an interactive read
// that lands behind a 66-worktree bulk sweep — it still waits for the queue to
// drain. Interactive work is dequeued ahead of bulk work, so a background scan
// yields to a panel the user is actually looking at. Strict priority is safe
// here because interactive load is inherently finite and short-lived (a bounded
// set of mounted panels, each firing a handful of reads per switch or per 30s
// poll), so bulk always drains once the burst passes.
//
// Writes deliberately do NOT go through this gate. Merges, fetches, and
// `worktree add` are user-initiated, rare, and long — queueing them behind a
// background sweep would be strictly worse, and they call read helpers
// internally, which under a shared cap is a deadlock waiting to happen.
//
// Cap re-checked against the thing that actually matters — one worktree
// switch, which issues ~15-20 reads, measured on the same monorepo both idle
// and while a full dirty sweep runs:
//
// cap quiet mean quiet p50 contended mean contended max
// 2 1881ms 1784ms 1876ms 2884ms
// 4 1675ms 1614ms 1702ms 3449ms
// 6 1623ms 1548ms 1677ms 3724ms
// 8 1625ms 1559ms 1770ms 3936ms
//
// 4 and 6 are within run-to-run noise of each other on the mean (repeat runs
// at cap=4 landed between 1602ms and 1675ms), and the tail under contention
// gets monotonically worse as the cap rises. 4 is chosen for that tail.

export type GitPriority = 'interactive' | 'bulk'

/** The throughput plateau from the table above. */
export const MAX_CONCURRENT_GIT_READS = 4

interface Waiter {
resolve: () => void
}

const queues: Record<GitPriority, Waiter[]> = {
interactive: [],
bulk: []
}

let active = 0

function next(): void {
const waiter = queues.interactive.shift() ?? queues.bulk.shift()
if (!waiter) return
active++
waiter.resolve()
}

function acquire(priority: GitPriority): Promise<void> {
if (active < MAX_CONCURRENT_GIT_READS) {
active++
return Promise.resolve()
}
return new Promise<void>((resolve) => {
queues[priority].push({ resolve })
})
}

function release(): void {
active--
next()
}

/** Run a read-only git operation under the concurrency gate. The permit is
* held only for the duration of `fn`, so a caller that runs several reads in
* sequence takes and returns a permit per read rather than holding one across
* the whole sequence — that's what keeps nested helpers deadlock-free. */
export async function runGitRead<T>(
priority: GitPriority,
fn: () => Promise<T>
): Promise<T> {
await acquire(priority)
try {
return await fn()
} finally {
release()
}
}

/** Test-only: observable queue depth. */
export function gitLimiterStats(): {
active: number
interactiveQueued: number
bulkQueued: number
} {
return {
active,
interactiveQueued: queues.interactive.length,
bulkQueued: queues.bulk.length
}
}

/** Test-only: drop all state between cases. */
export function resetGitLimiter(): void {
queues.interactive.length = 0
queues.bulk.length = 0
active = 0
}
4 changes: 2 additions & 2 deletions src/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1674,8 +1674,8 @@ function registerIpcHandlers(): void {
}
)

transport.onRequest('worktree:isDirty', async (_ctx, path: string) => {
const git = await isWorktreeDirty(path)
transport.onRequest('worktree:isDirty', async (_ctx, path: string, opts?: { bulk?: boolean }) => {
const git = await isWorktreeDirty(path, opts?.bulk ? 'bulk' : 'interactive')
const scratchpad = hasScratchpadNote(store.getSnapshot().state.scratchpad, path)
return { git, scratchpad }
})
Expand Down
110 changes: 110 additions & 0 deletions src/main/worktree-main-status-cache.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest'
/* These spawn real git. Under a full parallel suite run a handful of spawns
* can blow past vitest's 5s default, so every case here sets its own budget —
* a timeout in this file would otherwise read as a caching regression. */
import { execFileSync } from 'child_process'
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'fs'
import { tmpdir } from 'os'
import { join } from 'path'
import { getMainWorktreeStatus, invalidateMainWorktreeStatus } from './worktree'

function git(cwd: string, args: string[]): string {
return execFileSync('git', args, {
cwd,
env: {
...process.env,
GIT_AUTHOR_NAME: 't',
GIT_AUTHOR_EMAIL: 't@t',
GIT_COMMITTER_NAME: 't',
GIT_COMMITTER_EMAIL: 't@t'
}
}).toString()
}

const TIMEOUT = 60_000

describe('getMainWorktreeStatus caching', () => {
let repo: string

beforeEach(() => {
invalidateMainWorktreeStatus()
repo = mkdtempSync(join(tmpdir(), 'harness-mainstatus-'))
git(repo, ['init', '-q', '-b', 'main'])
git(repo, ['config', 'commit.gpgsign', 'false'])
writeFileSync(join(repo, 'f.txt'), 'base\n')
git(repo, ['add', 'f.txt'])
git(repo, ['commit', '-q', '-m', 'base'])
})

afterEach(() => {
invalidateMainWorktreeStatus()
rmSync(repo, { recursive: true, force: true })
})

// The switch-time shape: the panel asks directly while worktree:previewMerge
// asks internally, at the same instant. One underlying read, not two.
it('collapses concurrent callers onto a single read', async () => {
const [a, b] = await Promise.all([
getMainWorktreeStatus(repo),
getMainWorktreeStatus(repo)
])
expect(a).toBe(b)
}, TIMEOUT)

it('serves a later caller from cache within the TTL', async () => {
const first = await getMainWorktreeStatus(repo)
expect(await getMainWorktreeStatus(repo)).toBe(first)
}, TIMEOUT)

it('re-reads after an explicit invalidation', async () => {
const first = await getMainWorktreeStatus(repo)
invalidateMainWorktreeStatus(repo)
const second = await getMainWorktreeStatus(repo)
expect(second).not.toBe(first)
expect(second).toEqual(first)
}, TIMEOUT)

it('re-reads when forced, and picks up a change the cache would have hidden', async () => {
const clean = await getMainWorktreeStatus(repo)
expect(clean.isDirty).toBe(false)
expect(clean.ready).toBe(true)

writeFileSync(join(repo, 'f.txt'), 'dirty\n')
// Unforced within the TTL still reports the stale answer — that's the
// trade the TTL makes, and why the merge gate forces.
expect((await getMainWorktreeStatus(repo)).isDirty).toBe(false)

const forced = await getMainWorktreeStatus(repo, { force: true })
expect(forced.isDirty).toBe(true)
expect(forced.ready).toBe(false)
}, TIMEOUT)

it("keys by repo, so a second repo is not served the first one's answer", async () => {
const other = mkdtempSync(join(tmpdir(), 'harness-mainstatus-b-'))
try {
// `master` rather than an arbitrary name: getLocalBaseBranch only
// recognises main/master, and falls back to the literal 'main' for
// anything else — which would mask a key collision instead of exposing it.
git(other, ['init', '-q', '-b', 'master'])
git(other, ['config', 'commit.gpgsign', 'false'])
writeFileSync(join(other, 'g.txt'), 'x\n')
git(other, ['add', 'g.txt'])
git(other, ['commit', '-q', '-m', 'x'])

const a = await getMainWorktreeStatus(repo)
const b = await getMainWorktreeStatus(other)
expect(a.path).not.toBe(b.path)
expect(a.baseBranch).toBe('main')
expect(b.baseBranch).toBe('master')
} finally {
rmSync(other, { recursive: true, force: true })
}
}, TIMEOUT)

it('does not cache a failure', async () => {
const missing = join(tmpdir(), 'harness-mainstatus-does-not-exist')
await expect(getMainWorktreeStatus(missing)).rejects.toThrow()
// A cached rejection here would poison the repo for the whole TTL.
await expect(getMainWorktreeStatus(missing)).rejects.toThrow()
}, TIMEOUT)
})
Loading
Loading