mirror of
https://github.com/stablyai/orca.git
synced 2026-09-27 00:02:37 +00:00
perf(git): answer remote-URL questions from one subprocess, not one per remote (#18158)
Four copies of the same loop ran `git remote` and then a serial `git remote get-url <name>` per remote to answer "which remote has this URL". On a repo with 58 remotes that is 59 subprocesses -- measured at 1083 ms -- for one question, and worktree create asks it several times. `git remote -v` answers for every remote from one child, reporting the same insteadOf-expanded first fetch URL `get-url` prints. The batched `cat-file --batch-check` branch-conflict probe decides from stdout, but its WSL route was unfenced, so a login-shell fallback printed the distro banner onto the stream it parses. That broke the one-line-per-ref contract, made every batch undecided, and fell straight back to one `show-ref` per remote -- the cost the batch exists to remove. Measured at 58 remotes / 4346 branches, spawns and wall time: push-target remote scan 59 -> 1 (1083 ms -> 8 ms) branch-conflict probe 60 -> 3 (984 ms -> 43 ms) configured push target 123 -> 6 (2707 ms -> 157 ms)
This commit is contained in:
@@ -157,7 +157,11 @@ export async function probeAnyExactRefBatched(
|
||||
} catch {
|
||||
return { found: false, unknown: true }
|
||||
}
|
||||
const lines = stdout.split('\n').filter((line) => line.trim().length > 0)
|
||||
// Trim per line so a CRLF-translating host's `\r` does not become part of the type.
|
||||
const lines = stdout
|
||||
.split('\n')
|
||||
.map((line) => line.trim())
|
||||
.filter((line) => line.length > 0)
|
||||
// One line per input, in order; a short read means the batch never answered for the rest.
|
||||
if (lines.length !== safeRefs.length) {
|
||||
return { found: false, unknown: true }
|
||||
|
||||
@@ -193,6 +193,17 @@ describe('git remote operations', () => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url' && args[2] === 'pr-pynickle-orca') {
|
||||
return { stdout: 'https://github.com/pynickle/orca.git\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === '-v') {
|
||||
return {
|
||||
stdout: [
|
||||
'origin\thttps://github.com/stablyai/orca.git (fetch)',
|
||||
'origin\thttps://github.com/stablyai/orca.git (push)',
|
||||
'pr-pynickle-orca\thttps://github.com/pynickle/orca.git (fetch)',
|
||||
'pr-pynickle-orca\thttps://github.com/pynickle/orca.git (push)'
|
||||
].join('\n'),
|
||||
stderr: ''
|
||||
}
|
||||
}
|
||||
if (args[0] === 'remote') {
|
||||
return { stdout: 'origin\npr-pynickle-orca\n', stderr: '' }
|
||||
}
|
||||
@@ -207,6 +218,54 @@ describe('git remote operations', () => {
|
||||
)
|
||||
})
|
||||
|
||||
// Regression: normalizing a URL-valued push remote used to run `git remote` and then a
|
||||
// serial `git remote get-url` per remote -- 59 subprocesses on a 58-remote repo.
|
||||
it('normalizes a URL-valued push remote from one remote table read at 58 remotes', async () => {
|
||||
const remotes = [
|
||||
{ name: 'origin', url: 'https://github.com/stablyai/orca.git' },
|
||||
...Array.from({ length: 56 }, (_, index) => ({
|
||||
name: `pr-user${index}-orca`,
|
||||
url: `https://github.com/user${index}/orca.git`
|
||||
})),
|
||||
{ name: 'pr-pynickle-orca', url: 'https://github.com/pynickle/orca.git' }
|
||||
]
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'symbolic-ref') {
|
||||
return { stdout: 'imp/chinese-translation\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'config' && args.includes('branch.imp/chinese-translation.remote')) {
|
||||
return { stdout: 'https://github.com/pynickle/orca.git\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'config' && args.includes('branch.imp/chinese-translation.merge')) {
|
||||
return { stdout: 'refs/heads/imp/chinese-translation\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'config') {
|
||||
throw new Error(`config key is not set: ${args.join(' ')}`)
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === '-v') {
|
||||
return {
|
||||
stdout: remotes
|
||||
.flatMap(({ name, url }) => [`${name}\t${url} (fetch)`, `${name}\t${url} (push)`])
|
||||
.join('\n'),
|
||||
stderr: ''
|
||||
}
|
||||
}
|
||||
if (args[0] === 'remote') {
|
||||
throw new Error(`unexpected remote scan: ${args.join(' ')}`)
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
|
||||
await gitPush('/repo', false)
|
||||
|
||||
const remoteReads = gitExecFileAsyncMock.mock.calls.filter(([args]) => args[0] === 'remote')
|
||||
expect(remoteReads.map(([args]) => args)).toEqual([['remote', '-v']])
|
||||
expect(gitExecFileAsyncMock).toHaveBeenLastCalledWith(
|
||||
['push', '--set-upstream', 'pr-pynickle-orca', 'HEAD:imp/chinese-translation'],
|
||||
{ cwd: '/repo' }
|
||||
)
|
||||
})
|
||||
|
||||
it('uses an explicit push target even when it differs from the local branch name', async () => {
|
||||
gitExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: '', stderr: '' })
|
||||
|
||||
+15
-23
@@ -4,6 +4,7 @@ import {
|
||||
} from '../../shared/git-remote-error'
|
||||
import { resolveEffectiveGitUpstream } from '../../shared/git-effective-upstream'
|
||||
import { gitRefTargetsBranchOnRemote } from '../../shared/git-remote-branch-name'
|
||||
import { findGitRemoteNameByFetchUrl } from '../../shared/git-remote-url-index'
|
||||
import type { GitPushTarget } from '../../shared/worktree/types'
|
||||
import type { GitRuntimeOptions } from './git-runtime-options'
|
||||
import { gitOptionsForWorktree } from './git-runtime-options'
|
||||
@@ -84,6 +85,8 @@ type ConfiguredPushRemote = {
|
||||
branchRemote: string | null
|
||||
}
|
||||
|
||||
// One `git remote -v` instead of `git remote` plus a serial `git remote get-url`
|
||||
// per remote; both print the same insteadOf-expanded fetch URL.
|
||||
async function findRemoteNameForUrl(
|
||||
worktreePath: string,
|
||||
remoteUrl: string,
|
||||
@@ -91,30 +94,13 @@ async function findRemoteNameForUrl(
|
||||
): Promise<string | null> {
|
||||
try {
|
||||
const { stdout } = await gitExecFileAsync(
|
||||
['remote'],
|
||||
['remote', '-v'],
|
||||
gitOptionsForWorktree(worktreePath, options)
|
||||
)
|
||||
const remotes = stdout
|
||||
.split(/\r?\n/)
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean)
|
||||
for (const remoteName of remotes) {
|
||||
try {
|
||||
const { stdout: urlStdout } = await gitExecFileAsync(
|
||||
['remote', 'get-url', remoteName],
|
||||
gitOptionsForWorktree(worktreePath, options)
|
||||
)
|
||||
if (urlStdout.trim() === remoteUrl) {
|
||||
return remoteName
|
||||
}
|
||||
} catch {
|
||||
// Ignore a remote that disappeared or has no fetch URL.
|
||||
}
|
||||
}
|
||||
return findGitRemoteNameByFetchUrl(stdout, (candidateUrl) => candidateUrl === remoteUrl)
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
async function normalizePushRemote(
|
||||
@@ -141,11 +127,17 @@ async function getConfiguredPushRemote(
|
||||
if (!remote) {
|
||||
return null
|
||||
}
|
||||
const normalizedRemote = await normalizePushRemote(worktreePath, remote, options)
|
||||
// The two usually name the same URL; resolving it twice reads the remote table twice.
|
||||
if (!branchRemote) {
|
||||
return { remote: normalizedRemote, branchRemote: null }
|
||||
}
|
||||
return {
|
||||
remote: await normalizePushRemote(worktreePath, remote, options),
|
||||
branchRemote: branchRemote
|
||||
? await normalizePushRemote(worktreePath, branchRemote, options)
|
||||
: null
|
||||
remote: normalizedRemote,
|
||||
branchRemote:
|
||||
branchRemote === remote
|
||||
? normalizedRemote
|
||||
: await normalizePushRemote(worktreePath, branchRemote, options)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
// Why: the batched `cat-file --batch-check` conflict probe decides from stdout, so a
|
||||
// WSL login-shell fallback that prints the distro banner onto that stream desynchronizes
|
||||
// the one-line-per-ref contract. Every batch then came back undecided and fell through to
|
||||
// one `show-ref` subprocess per remote -- the cost the batch exists to remove. These tests
|
||||
// pin the fence request and the resulting subprocess count at 58 remotes.
|
||||
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { gitExecFileAsyncMock } = vi.hoisted(() => ({ gitExecFileAsyncMock: vi.fn() }))
|
||||
|
||||
vi.mock('./runner', () => ({ gitExecFileAsync: gitExecFileAsyncMock }))
|
||||
|
||||
import { getBranchConflictKind } from './repo-branch-conflict'
|
||||
|
||||
const REMOTES = Array.from({ length: 58 }, (_, index) => `r${index}`)
|
||||
const BRANCH = 'user/feature'
|
||||
const WSL_BANNER =
|
||||
'Welcome to Ubuntu 24.04.1 LTS (GNU/Linux 5.15.167.4-microsoft-standard-WSL2 x86_64)\n' +
|
||||
'To run a command as administrator (user "root"), use "sudo <command>".\n'
|
||||
|
||||
type GitExecOptions = { stdin?: string; captureWslLoginShellOutput?: boolean }
|
||||
|
||||
/**
|
||||
* Stand-in for a WSL-routed runner: the login shell prepends its banner to stdout unless
|
||||
* the caller asked for the fenced form, which slices the payload back out.
|
||||
*/
|
||||
function installLoginShellRunner(): { argv: string[][] } {
|
||||
const argv: string[][] = []
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[], options: GitExecOptions = {}) => {
|
||||
argv.push(args)
|
||||
if (args[0] === 'rev-parse') {
|
||||
throw new Error('local branch is absent')
|
||||
}
|
||||
if (args[0] === 'remote') {
|
||||
return { stdout: `${WSL_BANNER}${REMOTES.join('\n')}\n`, stderr: '' }
|
||||
}
|
||||
if (args[0] === 'show-ref') {
|
||||
throw Object.assign(new Error('missing ref'), { code: 1, stderr: '' })
|
||||
}
|
||||
if (args[0] === 'cat-file') {
|
||||
const payload = `${(options.stdin ?? '')
|
||||
.split('\n')
|
||||
.filter(Boolean)
|
||||
.map((ref) => `${ref} missing`)
|
||||
.join('\n')}\n`
|
||||
return {
|
||||
stdout: options.captureWslLoginShellOutput ? payload : `${WSL_BANNER}${payload}`,
|
||||
stderr: ''
|
||||
}
|
||||
}
|
||||
throw new Error(`unexpected git command: ${args.join(' ')}`)
|
||||
})
|
||||
return { argv }
|
||||
}
|
||||
|
||||
function countSubcommand(argv: readonly string[][], subcommand: string): number {
|
||||
return argv.filter((args) => args[0] === subcommand).length
|
||||
}
|
||||
|
||||
describe('getBranchConflictKind batched remote probe', () => {
|
||||
beforeEach(() => {
|
||||
gitExecFileAsyncMock.mockReset()
|
||||
})
|
||||
|
||||
it('asks the WSL login shell to fence the batch payload it parses', async () => {
|
||||
installLoginShellRunner()
|
||||
|
||||
await getBranchConflictKind('/repo', BRANCH)
|
||||
|
||||
const batchCall = gitExecFileAsyncMock.mock.calls.find(([args]) => args[0] === 'cat-file')
|
||||
expect(batchCall?.[1]).toMatchObject({ captureWslLoginShellOutput: true })
|
||||
})
|
||||
|
||||
it('answers from one batched subprocess instead of one show-ref per remote', async () => {
|
||||
const { argv } = installLoginShellRunner()
|
||||
|
||||
await expect(getBranchConflictKind('/repo', BRANCH)).resolves.toBeNull()
|
||||
|
||||
expect(countSubcommand(argv, 'cat-file')).toBe(1)
|
||||
expect(countSubcommand(argv, 'show-ref')).toBe(0)
|
||||
})
|
||||
|
||||
it('still falls back to per-ref probes when the batch itself fails', async () => {
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'rev-parse') {
|
||||
throw new Error('local branch is absent')
|
||||
}
|
||||
if (args[0] === 'remote') {
|
||||
return { stdout: `${REMOTES.join('\n')}\n`, stderr: '' }
|
||||
}
|
||||
if (args[0] === 'cat-file') {
|
||||
throw new Error('cat-file is unavailable on this host')
|
||||
}
|
||||
if (args[0] === 'show-ref') {
|
||||
return { stdout: '', stderr: '' }
|
||||
}
|
||||
throw new Error(`unexpected git command: ${args.join(' ')}`)
|
||||
})
|
||||
|
||||
await expect(getBranchConflictKind('/repo', BRANCH)).resolves.toBe('remote')
|
||||
})
|
||||
})
|
||||
@@ -147,10 +147,12 @@ export function getBranchConflictKind(
|
||||
const execOptions = gitExecOptions(path, options)
|
||||
const runLocalGit = (
|
||||
argv: string[],
|
||||
commandOptions?: ExactRefProbeExecOptions & { stdin?: string }
|
||||
commandOptions?: ExactRefProbeExecOptions & { stdin?: string },
|
||||
captureWslLoginShellOutput = false
|
||||
): Promise<{ stdout: string }> =>
|
||||
gitExecFileAsync(argv, {
|
||||
...execOptions,
|
||||
...(captureWslLoginShellOutput ? { captureWslLoginShellOutput: true } : {}),
|
||||
...(commandOptions?.maxBuffer === undefined ? {} : { maxBuffer: commandOptions.maxBuffer }),
|
||||
...(commandOptions?.timeoutMs === undefined ? {} : { timeout: commandOptions.timeoutMs }),
|
||||
...(commandOptions?.stdin === undefined ? {} : { stdin: commandOptions.stdin })
|
||||
@@ -160,7 +162,13 @@ export function getBranchConflictKind(
|
||||
branchName,
|
||||
allowedBaseRef,
|
||||
{},
|
||||
(argv, commandOptions) => runLocalGit(argv, commandOptions)
|
||||
// Why fenced: the batch decides from stdout, and a WSL login-shell fallback writes
|
||||
// the distro's rc/motd banner to that same stream. The extra lines break the
|
||||
// one-line-per-ref contract, so every batch came back undecided and fell through to
|
||||
// one `show-ref` subprocess per remote -- the exact cost the batch exists to remove.
|
||||
// `show-ref --verify --quiet` prints nothing and is read by exit code, so it needs
|
||||
// no fence; the capture wrapper preserves the payload's exit status either way.
|
||||
(argv, commandOptions) => runLocalGit(argv, commandOptions, true)
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -363,6 +363,16 @@ describe('getUpstreamStatus', () => {
|
||||
if (args[0] === 'remote' && args[1] === 'get-url' && args[2] === 'pr-pynickle-orca') {
|
||||
return Promise.resolve({ stdout: 'https://github.com/pynickle/orca.git\n' })
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === '-v') {
|
||||
return Promise.resolve({
|
||||
stdout: [
|
||||
'origin\thttps://github.com/stablyai/orca.git (fetch)',
|
||||
'origin\thttps://github.com/stablyai/orca.git (push)',
|
||||
'pr-pynickle-orca\thttps://github.com/pynickle/orca.git (fetch)',
|
||||
'pr-pynickle-orca\thttps://github.com/pynickle/orca.git (push)'
|
||||
].join('\n')
|
||||
})
|
||||
}
|
||||
if (args[0] === 'remote') {
|
||||
return Promise.resolve({ stdout: 'origin\npr-pynickle-orca\n' })
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user