perf(git): read the porcelain worktree mode instead of probing conflicted paths

Every porcelain-v2 `u` record already carries `mW`, the working-tree mode Git
stat'ed for that row: `000000` means the conflicted path is absent. Reading it
replaces the per-conflict `fs.access`, so the bounded-concurrency resolver, its
`= 8` cap, and the order/error-precedence invariant are unnecessary rather than
cheaper. `access()` stays only as a fallback for a malformed `mW`, so
`parseUnmergedEntry` keeps its signature and neither status-read.ts nor the
relay loop changes.

Also corrects two fixtures that encoded `mW=100644` for a file that does not
exist, which real Git never emits.
This commit is contained in:
Neil
2026-09-03 20:40:04 -07:00
parent 482f8aa4b4
commit 104f44222e
5 changed files with 90 additions and 260 deletions
+3 -27
View File
@@ -18,10 +18,7 @@ import { findExistingWorktreeSymlinkPaths } from '../worktree-symlink-detection'
import type { GetStatusOptions } from './get-status-options'
import { statusReadLeaseOwner } from './git-read-cache-invalidation'
import { detectConflictOperation } from './git-conflict-operation'
import {
parseUnmergedEntry,
resolveUnmergedStatusRecords
} from '../../../shared/git-status-conflict-entries'
import { parseUnmergedEntry } from '../../../shared/git-status-conflict-entries'
import { getEffectiveUpstreamStatusCacheKey } from './effective-upstream-status-cache'
import {
getShortBranchName,
@@ -179,37 +176,16 @@ async function runGetStatus(
// Why: git runs in the distro and answers in its namespace; the working-tree probes below run here.
const hostWorktreePath = resolveWorktreeHostPath(worktreePath, options) ?? worktreePath
// Why: a record only pushes one entry, so the cap can never break before index `limit`; prefetching
// exactly that prefix keeps the probe count identical to a serial read on a truncated status.
const resolvableUnmergedEnd = didHitLimit
? Math.min(parser.statusRecords.length, limit)
: parser.statusRecords.length
// Why: skip the prefetch entirely for the conflict-free poll so the common path allocates nothing extra.
const resolvedUnmerged =
parser.unmergedLines.length > 0
? await resolveUnmergedStatusRecords(
hostWorktreePath,
parser.statusRecords,
resolvableUnmergedEnd
)
: undefined
// Why: resolve deferred conflicts in Git's output order so the cap cannot hide
// an early conflict behind ordinary rows that appeared later in the stream.
for (const [index, record] of parser.statusRecords.entries()) {
for (const record of parser.statusRecords) {
if (didHitLimit && entries.length >= limit) {
break
}
if (record.type === 'entry') {
entries.push(record.entry)
} else {
const prefetched = resolvedUnmerged?.[index]
if (prefetched?.ok === false) {
throw prefetched.error
}
const unmergedEntry = prefetched
? prefetched.entry
: await parseUnmergedEntry(hostWorktreePath, record.line)
const unmergedEntry = await parseUnmergedEntry(hostWorktreePath, record.line)
if (unmergedEntry) {
entries.push(unmergedEntry)
}
+19 -12
View File
@@ -77,6 +77,13 @@ describe('getStatus', () => {
gitExecFileAsyncMock.mockResolvedValue({ stdout: '' })
})
/** `access` targets outside the git dir — i.e. working-tree probes, not conflict-marker reads. */
function conflictFileProbes(): string[] {
return accessMock.mock.calls
.map(([target]) => String(target).replaceAll('\\', '/'))
.filter((target) => !target.includes('/.git/'))
}
it('parses unmerged porcelain v2 entries into unresolved conflict rows', async () => {
readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n')
accessMock.mockImplementation(async (target: string) => {
@@ -104,11 +111,12 @@ describe('getStatus', () => {
])
})
it('maps deleted conflicts to deleted when the working tree file is absent', async () => {
// The 7th field of a `u` record is the working-tree mode; `000000` is how Git reports an absent path.
it('maps deleted conflicts to deleted from the porcelain working-tree mode', async () => {
readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n')
gitExecFileAsyncMock.mockResolvedValueOnce({
stdout:
'u UD N... 100644 100644 000000 100644 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb cccccccccccccccccccccccccccccccccccccccc src/deleted.ts\n'
'u UD N... 100644 100644 000000 000000 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb cccccccccccccccccccccccccccccccccccccccc src/deleted.ts\n'
})
const result = await getStatus('/repo')
@@ -120,10 +128,12 @@ describe('getStatus', () => {
conflictKind: 'deleted_by_them',
conflictStatus: 'unresolved'
})
expect(conflictFileProbes()).toEqual([])
})
it('falls back to modified when the working-tree probe fails for a non-absence reason', async () => {
it('never re-probes the working tree for a conflict row, whatever the filesystem would say', async () => {
readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n')
// Every probe fails ENOENT (beforeEach) or EIO — neither may reach the row's status.
accessMock.mockRejectedValue(Object.assign(new Error('EIO'), { code: 'EIO' }))
gitExecFileAsyncMock.mockResolvedValueOnce({
stdout:
@@ -134,19 +144,14 @@ describe('getStatus', () => {
expect(result.entries[0]?.status).toBe('modified')
expect(result.entries[0]?.conflictKind).toBe('added_by_us')
expect(conflictFileProbes()).toEqual([])
})
// Why both cases normalize separators: git reports the worktree in the WSL guest namespace, and
// the assertion is about which path is probed, not which separator this host's `path` emits.
it('probes the conflict working tree through the distro spelling on Windows', async () => {
it('resolves a WSL conflict row without crossing the 9p share', async () => {
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
readFileMock.mockResolvedValue('gitdir: /home/me/repo/.git/worktrees/feature\n')
accessMock.mockImplementation(async (target: string) => {
if (String(target).endsWith('new.ts')) {
return undefined
}
throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' })
})
gitExecFileAsyncMock.mockResolvedValueOnce({
stdout:
'u DU N... 100644 100644 100644 100644 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb cccccccccccccccccccccccccccccccccccccccc src/new.ts\n'
@@ -156,10 +161,12 @@ describe('getStatus', () => {
const result = await getStatus('/home/me/repo/feature', { wslDistro: 'Ubuntu' })
const probed = accessMock.mock.calls.map(([target]) => String(target).replaceAll('\\', '/'))
expect(probed).toContain('//wsl.localhost/Ubuntu/home/me/repo/feature/src/new.ts')
// No `\\wsl.localhost` round trip per conflict row: the porcelain `mW` field already answered.
expect(probed).not.toContain('//wsl.localhost/Ubuntu/home/me/repo/feature/src/new.ts')
expect(conflictFileProbes()).toEqual([])
expect(result.entries[0]?.status).toBe('modified')
expect(result.entries[0]?.conflictKind).toBe('deleted_by_us')
// The conflict-marker probes travel the same way.
// The conflict-marker probes still travel through the distro spelling.
expect(
probed.filter((target) =>
target.startsWith('//wsl.localhost/Ubuntu/home/me/repo/.git/worktrees/feature/')
+2 -1
View File
@@ -160,7 +160,8 @@ describe('relay/desktop unmerged-entry porcelain parity', () => {
const unmergedLines = [
'u UU N... 100644 100644 100644 100644 aa bb cc plain.ts',
'u UD N... 100644 100644 000000 100644 aa bb cc "present \\303\\251.ts"',
'u UD N... 100644 100644 000000 100644 aa bb cc "missing \\303\\251.ts"',
// mW=000000: real Git reports an absent working-tree path this way, and the file is not created below.
'u UD N... 100644 100644 000000 000000 aa bb cc "missing \\303\\251.ts"',
'u DD N... 100644 100644 000000 000000 aa bb cc both-gone.ts'
]
const git = vi.fn<GitExec>(async (args) => {
+47 -172
View File
@@ -1,10 +1,9 @@
/**
* `u` records for asymmetric conflict kinds each cost an `fs.access`. Resolving them concurrently
* must not move a row, change how many probes run, or change which error wins.
* Asymmetric `u` records used to cost one `fs.access` each — a 9p/network round trip per conflict on
* a WSL or remote worktree. Porcelain v2 already carries the answer in the worktree mode (`mW`), so
* the probe must not come back.
*/
import { beforeEach, describe, expect, it, vi } from 'vitest'
import type { StatusPorcelainRecord } from './git-status-porcelain-parser'
import type { GitStatusEntry } from './git-status-types'
import type * as NodeFsPromisesModule from 'node:fs/promises'
const { accessMock } = vi.hoisted(() => ({ accessMock: vi.fn() }))
@@ -14,199 +13,75 @@ vi.mock('node:fs/promises', async (importOriginal) => ({
access: accessMock
}))
/**
* `parseUnmergedEntry` only rethrows when reading `error.code` itself throws, so this is how a
* record is made to reject rather than degrade to 'modified'.
*/
function rejectionEscapingTheErrnoGuard(failure: Error): unknown {
return new Proxy(
{},
{
get: () => {
throw failure
}
}
)
}
const { parseUnmergedEntry, resolveUnmergedStatusRecords } =
await import('./git-status-conflict-entries')
const { parseUnmergedEntry } = await import('./git-status-conflict-entries')
const WORKTREE = '/repo'
function unmergedLine(xy: string, filePath: string): string {
return `u ${xy} N... 100644 100644 100644 100644 aaa bbb ccc ${filePath}`
/** `u <XY> <sub> <m1> <m2> <m3> <mW> <h1> <h2> <h3> <path>` — `mW` is the working-tree mode. */
function unmergedLine(xy: string, modeWorktree: string, filePath: string): string {
return `u ${xy} N... 100644 100644 100644 ${modeWorktree} aaa bbb ccc ${filePath}`
}
function entryRecord(path: string): StatusPorcelainRecord {
return { type: 'entry', entry: { path, status: 'modified', area: 'unstaged' } }
}
const ASYMMETRIC_KINDS = ['AU', 'UA', 'DU', 'UD'] as const
/** Mixes both conflict families: UU/AA return without I/O, AU/UA/DU/UD each probe the filesystem. */
const MIXED_RECORDS: StatusPorcelainRecord[] = [
{ type: 'unmerged', line: unmergedLine('UU', 'both-modified.ts') },
entryRecord('plain-one.ts'),
{ type: 'unmerged', line: unmergedLine('AU', 'added-by-us.ts') },
{ type: 'unmerged', line: unmergedLine('AA', 'both-added.ts') },
{ type: 'unmerged', line: unmergedLine('UA', 'added-by-them.ts') },
entryRecord('plain-two.ts'),
{ type: 'unmerged', line: unmergedLine('DU', 'deleted-by-us.ts') },
{ type: 'unmerged', line: unmergedLine('UD', 'deleted-by-them.ts') },
{ type: 'unmerged', line: unmergedLine('160000', 'submodule-ignored.ts') }
]
/** The pre-change consumption: one `await` per record, in Git's output order. */
async function collectSerially(
records: readonly StatusPorcelainRecord[]
): Promise<(GitStatusEntry | null)[]> {
const collected: (GitStatusEntry | null)[] = []
for (const record of records) {
collected.push(
record.type === 'entry' ? record.entry : await parseUnmergedEntry(WORKTREE, record.line)
)
}
return collected
}
async function collectConcurrently(
records: readonly StatusPorcelainRecord[]
): Promise<(GitStatusEntry | null)[]> {
const resolved = await resolveUnmergedStatusRecords(WORKTREE, records, records.length)
return records.map((record, index) => {
if (record.type === 'entry') {
return record.entry
}
const settled = resolved[index]
if (settled?.ok === false) {
throw settled.error
}
return settled?.entry ?? null
})
}
describe('resolveUnmergedStatusRecords', () => {
describe('parseUnmergedEntry', () => {
beforeEach(() => {
accessMock.mockReset()
accessMock.mockRejectedValue(new Error('fs.access must not be reached for well-formed records'))
})
it('produces the same rows, in the same order, as the serial read', async () => {
accessMock.mockImplementation((target: string) =>
target.includes('deleted-by')
? Promise.reject(Object.assign(new Error('x'), { code: 'ENOENT' }))
: Promise.resolve(undefined)
)
it('reads the working-tree mode instead of probing the filesystem', async () => {
for (const xy of ASYMMETRIC_KINDS) {
const absent = await parseUnmergedEntry(WORKTREE, unmergedLine(xy, '000000', 'gone.ts'))
const present = await parseUnmergedEntry(WORKTREE, unmergedLine(xy, '100644', 'here.ts'))
const serial = await collectSerially(MIXED_RECORDS)
const concurrent = await collectConcurrently(MIXED_RECORDS)
expect(absent?.status, xy).toBe('deleted')
expect(present?.status, xy).toBe('modified')
}
expect(concurrent).toEqual(serial)
expect(concurrent.map((entry) => entry?.path ?? null)).toEqual([
'both-modified.ts',
'plain-one.ts',
'added-by-us.ts',
'both-added.ts',
'added-by-them.ts',
'plain-two.ts',
'deleted-by-us.ts',
'deleted-by-them.ts',
null
])
expect(concurrent.map((entry) => entry?.status ?? null)).toEqual([
'modified',
'modified',
'modified',
'modified',
'modified',
'modified',
'deleted',
'deleted',
null
])
// The regression this replaces: one probe per asymmetric row, serialised across the status poll.
expect(accessMock).not.toHaveBeenCalled()
})
it('runs the same number of filesystem probes as the serial read', async () => {
accessMock.mockResolvedValue(undefined)
await collectSerially(MIXED_RECORDS)
const serialProbes = accessMock.mock.calls.length
it('treats a symlink left in place of the conflicted file as present', async () => {
const entry = await parseUnmergedEntry(WORKTREE, unmergedLine('UD', '120000', 'link.ts'))
accessMock.mockClear()
await collectConcurrently(MIXED_RECORDS)
// Only the four asymmetric kinds probe; UU/AA/submodule never touch the filesystem.
expect(serialProbes).toBe(4)
expect(accessMock.mock.calls.length).toBe(serialProbes)
expect(entry?.status).toBe('modified')
expect(accessMock).not.toHaveBeenCalled()
})
it('never exceeds the concurrency bound', async () => {
let inFlight = 0
let peak = 0
accessMock.mockImplementation(async () => {
inFlight += 1
peak = Math.max(peak, inFlight)
await new Promise((resolve) => setTimeout(resolve, 0))
inFlight -= 1
})
const records = Array.from({ length: 50 }, (_, index) => ({
type: 'unmerged' as const,
line: unmergedLine('AU', `conflict-${index}.ts`)
}))
it('resolves the symmetric kinds from XY alone, whatever the working-tree mode says', async () => {
const bothModified = await parseUnmergedEntry(WORKTREE, unmergedLine('UU', '000000', 'a.ts'))
const bothAdded = await parseUnmergedEntry(WORKTREE, unmergedLine('AA', '000000', 'b.ts'))
const bothDeleted = await parseUnmergedEntry(WORKTREE, unmergedLine('DD', '100644', 'c.ts'))
await resolveUnmergedStatusRecords(WORKTREE, records, records.length)
expect(peak).toBeGreaterThan(1)
expect(peak).toBeLessThanOrEqual(8)
expect(bothModified?.status).toBe('modified')
expect(bothAdded?.status).toBe('modified')
expect(bothDeleted?.status).toBe('deleted')
expect(accessMock).not.toHaveBeenCalled()
})
it('resolves only the requested prefix, leaving later records for the caller', async () => {
accessMock.mockResolvedValue(undefined)
it('drops submodule conflicts without probing', async () => {
const line = 'u UU S... 160000 160000 160000 160000 aa bb cc vendor/sub'
const resolved = await resolveUnmergedStatusRecords(WORKTREE, MIXED_RECORDS, 4)
expect(resolved[0]).toEqual({
ok: true,
entry: expect.objectContaining({ path: 'both-modified.ts' })
})
expect(resolved[1]).toBeUndefined()
expect(resolved[6]).toBeUndefined()
expect(accessMock.mock.calls.length).toBe(1)
expect(await parseUnmergedEntry(WORKTREE, line)).toBeNull()
expect(accessMock).not.toHaveBeenCalled()
})
it('surfaces the earliest failing record, exactly as the serial read did', async () => {
const firstFailure = new Error('first failure')
const secondFailure = new Error('second failure')
accessMock.mockImplementation((target: string) => {
if (target.endsWith('added-by-them.ts')) {
return Promise.reject(rejectionEscapingTheErrnoGuard(firstFailure))
}
if (target.endsWith('deleted-by-us.ts')) {
return Promise.reject(rejectionEscapingTheErrnoGuard(secondFailure))
}
return Promise.resolve(undefined)
})
it('keeps the working-tree probe as a fallback for a mode no real Git emits', async () => {
accessMock.mockRejectedValueOnce(Object.assign(new Error('nope'), { code: 'ENOENT' }))
const missing = await parseUnmergedEntry(WORKTREE, unmergedLine('UD', 'zzzzzz', 'weird-a.ts'))
await expect(collectSerially(MIXED_RECORDS)).rejects.toBe(firstFailure)
await expect(collectConcurrently(MIXED_RECORDS)).rejects.toBe(firstFailure)
})
accessMock.mockResolvedValueOnce(undefined)
const found = await parseUnmergedEntry(WORKTREE, unmergedLine('UD', '12345', 'weird-b.ts'))
it('does not surface a failure the serial read would never have reached', async () => {
const lateFailure = new Error('late failure')
accessMock.mockImplementation((target: string) =>
target.endsWith('deleted-by-them.ts')
? Promise.reject(rejectionEscapingTheErrnoGuard(lateFailure))
: Promise.resolve(undefined)
)
accessMock.mockRejectedValueOnce(Object.assign(new Error('denied'), { code: 'EACCES' }))
const unreadable = await parseUnmergedEntry(WORKTREE, unmergedLine('UD', '', 'weird-c.ts'))
// The caller stops before that record, so the captured rejection is simply never replayed.
const resolved = await resolveUnmergedStatusRecords(
WORKTREE,
MIXED_RECORDS,
MIXED_RECORDS.length
)
const consumedPrefix = MIXED_RECORDS.slice(0, 7).map((record, index) =>
record.type === 'entry' ? record.entry : resolved[index]
)
expect(consumedPrefix).not.toContainEqual({ ok: false, error: lateFailure })
expect(resolved[7]).toEqual({ ok: false, error: lateFailure })
expect(missing?.status).toBe('deleted')
expect(found?.status).toBe('modified')
// Why: an ambiguous fs failure keeps the row visible rather than falsely reading as 'deleted'.
expect(unreadable?.status).toBe('modified')
expect(accessMock).toHaveBeenCalledTimes(3)
})
})
+19 -48
View File
@@ -1,51 +1,9 @@
import { access } from 'node:fs/promises'
import * as path from 'node:path'
import type { GitConflictKind, GitFileStatus, GitStatusEntry } from './git-status-types'
import type { StatusPorcelainRecord } from './git-status-porcelain-parser'
import { decodeGitCQuotedPath } from './git-cquoted-path'
const UNMERGED_ENTRY_RESOLVE_CONCURRENCY = 8
/** A settled `parseUnmergedEntry` result, replayed at the record index it belongs to. */
export type ResolvedUnmergedEntry =
| { ok: true; entry: GitStatusEntry | null }
| { ok: false; error: unknown }
/**
* Resolve the deferred `u` records in `records[0, end)` with bounded concurrency, keyed by record
* index. Asymmetric conflict kinds each cost an `fs.access`, which is a 9p/network round trip on a
* WSL or remote worktree, and a big rebase makes hundreds of them — serialising those stalls every
* status poll for the life of the conflict.
*
* Ordering is untouched: results stay at their own index, so the caller still consumes Git's output
* order, and a rejection is replayed only if the caller actually reaches that record.
*/
export async function resolveUnmergedStatusRecords(
worktreePath: string,
records: readonly StatusPorcelainRecord[],
end: number
): Promise<(ResolvedUnmergedEntry | undefined)[]> {
const resolved: (ResolvedUnmergedEntry | undefined)[] = []
let nextIndex = 0
const resolveNext = async (): Promise<void> => {
while (nextIndex < end) {
const index = nextIndex
nextIndex += 1
const record = records[index]
if (record?.type !== 'unmerged') {
continue
}
try {
resolved[index] = { ok: true, entry: await parseUnmergedEntry(worktreePath, record.line) }
} catch (error) {
resolved[index] = { ok: false, error }
}
}
}
const workerCount = Math.min(UNMERGED_ENTRY_RESOLVE_CONCURRENCY, end)
await Promise.all(Array.from({ length: workerCount }, () => resolveNext()))
return resolved
}
const OCTAL_FILE_MODE = /^[0-7]{6}$/
export async function parseUnmergedEntry(
worktreePath: string,
@@ -57,6 +15,7 @@ export async function parseUnmergedEntry(
const modeStage1 = parts[3]
const modeStage2 = parts[4]
const modeStage3 = parts[5]
const modeWorktree = parts[6]
const filePath = decodeGitCQuotedPath(parts.slice(10).join(' '))
if (!filePath) {
return null
@@ -76,7 +35,12 @@ export async function parseUnmergedEntry(
return {
path: filePath,
area: 'unstaged',
status: await getConflictCompatibilityStatus(worktreePath, filePath, conflictKind),
status: await getConflictCompatibilityStatus(
worktreePath,
filePath,
conflictKind,
modeWorktree
),
conflictKind,
conflictStatus: 'unresolved'
}
@@ -104,11 +68,12 @@ function parseConflictKind(xy: string): GitConflictKind | null {
}
// Why: `status` here is a rendering-compat choice for icon/color plumbing, not semantic; the conflict badge carries the real meaning.
// Why: for deleted_by_*/added_by_* variants Git's result depends on merge strategy, so check the filesystem.
// Why: for deleted_by_*/added_by_* variants Git's result depends on merge strategy, so ask whether the path is in the working tree.
async function getConflictCompatibilityStatus(
worktreePath: string,
filePath: string,
conflictKind: GitConflictKind
conflictKind: GitConflictKind,
modeWorktree: string
): Promise<GitFileStatus> {
if (conflictKind === 'both_modified' || conflictKind === 'both_added') {
return 'modified'
@@ -118,8 +83,14 @@ async function getConflictCompatibilityStatus(
return 'deleted'
}
// Why async: on a WSL worktree this path is a `\\wsl.localhost\...` share, and a sync probe
// per asymmetric conflict blocks the Electron main thread for a 9p round trip each.
// Why: `mW` is the worktree mode Git already stat'ed for this row — `000000` means absent. Reading
// it costs nothing and stays consistent with the rest of the snapshot, whereas a re-probe here is a
// 9p/network round trip per asymmetric conflict on a WSL or remote worktree.
if (OCTAL_FILE_MODE.test(modeWorktree)) {
return modeWorktree === '000000' ? 'deleted' : 'modified'
}
// Why: only reachable on output no real Git emits (truncated/malformed `u` record).
try {
await access(path.join(worktreePath, filePath))
return 'modified'