Skip to content

perf(server): Git status reads a branch's recorded base with one git process, not three - #28778

Open
only21mil wants to merge 1 commit into
pingdotgg:mainfrom
only21mil:fix/audit-m109
Open

only21mil wants to merge 1 commit into
pingdotgg:mainfrom
only21mil:fix/audit-m109

Conversation

@only21mil

Copy link
Copy Markdown
Contributor

Problem

Git status works out which base each non-default branch compares against on every refresh. For a branch with no usable t3-merge-base-ref, the lookup ran up to three git config --get processes: t3-merge-base-ref, then t3-merge-base, then the legacy gh-merge-base. Status with branch changes resolves the base twice, once for the ahead count and once for the change totals. That adds up to six config processes per refresh for branches created outside T3, or by older versions of it. They also take slots from the driver's limit of eight concurrent git processes.

Change

readBranchBaseConfig reads all three keys with one git config -z --get-regexp '^branch\.<branch>\.(t3-merge-base-ref|t3-merge-base|gh-merge-base)$'. The branch name is regex-escaped, so feature/a.b+ cannot match feature/aXbb. resolveBaseBranchForNoUpstream and readBranchBase both use it, and the precedence is unchanged:

  • A usable t3-merge-base-ref still wins.
  • t3-merge-base still wins over gh-merge-base.
  • A repeated key keeps its last value, as --get does.
  • An empty or valueless key still counts as missing.
  • Includes, -c and GIT_CONFIG_* overrides apply exactly as with --get.

Branches whose t3-merge-base-ref resolves already took one process, so they are unchanged.

Scope and approval

A focused performance change with no change in behavior or output, so it needs no prior issue or discussion. Config writes, key names and branch-base rules stay as they were. GitManager.resolvePrBase (PR creation, not polled) still reads t3-merge-base-ref on its own; I left it alone so the service contract stays the same.

No screen changes. Status returns the same branchChanges, aheadCount and aheadOfDefaultCount as before (measured below).

Verification

Measurement: statusDetailsLocal(cwd, { includeBranchChanges: true }) on one fixture repository, with a 200-file commit, an origin remote and one worktree per case. Both driver builds ran in the same process with a counting process spawner, alternating call by call over 400 pairs after 20 warm-up calls each. Linux, git 2.53.

Branch Git processes per status (config) Median before → after Median paired saving Pairs faster
no recorded base 17 (6) → 13 (2) 51.9 → 39.8 ms 11.1 ms 89%
only gh-merge-base 17 (6) → 13 (2) 47.3 → 36.7 ms 10.8 ms 88%
resolvable t3-merge-base-ref 13 (2) → 13 (2) 36.0 → 35.9 ms −0.06 ms 50%

The host was heavily loaded during the run, which inflates the cost of each process. On the same host, a plain shell loop timed the three git config --get calls at about 2.4 ms against 0.8 ms for one --get-regexp. The status results were identical before and after in every case.

  • New test "reads a recorded branch base as git config --get would" in GitVcsDriverCore.test.ts. It covers a missing key, a legacy-only key, a preferred key over legacy, a repeated key (last wins), an empty last value falling back to legacy, and a branch name with regex characters next to a branch it would wrongly match. With the escaping removed it fails with expected 'regex-match' to equal null.
  • With baseRef forced to null, the existing tests "fetches the selected worktree base for Changes" and "publishes a branch tracking its base under its own name, not the base" fail, so the preferred-ref path is already covered.
  • vp test run src/vcs/GitVcsDriverCore.test.ts src/git/GitManager.test.ts: 331/331 passed.
  • vp run check:changed 1d626d02d9: format, lint, knip, typecheck and tests passed.
  • The escaped pattern matched only its own branch with Apple Git 2.54 on macOS, for r]s, q{1}, u{2,3}v, a.b+, x(y), d$e and p|q.

Not checked: timing on macOS or Windows, where each process costs more; a run in a desktop or web client, since nothing on screen changes.

Prior PRs

None found for this change. Searches for t3-merge-base-ref, gh-merge-base, readBranchBase, get-regexp and resolveBaseBranchForNoUpstream across open and closed PRs found no same-fix PR. Related merged work on the same status path:

Open PRs #26125, #24841, #24047, #22249 and #21875 touch this file in other hunks.

Generated with Claude Opus 5.5 and GPT-6.1 Sol in T3 Code Nightly

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Nov 1, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 7ac63b6

Macroscope's review found this PR approvable — Branch-base configuration is read with one Git process instead of up to three, preserving existing precedence and fallback behavior. The change is confined to local configuration reads, with no change to product defaults or configuration writes.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 3fd2b85 and 7ac63b6.

Files selected for processing: 2
  • apps/server/src/vcs/GitVcsDriverCore.test.ts
  • apps/server/src/vcs/GitVcsDriverCore.ts

Limit details: You’ve used all 10 included reviews currently available.

Walkthrough

Branch base configuration reads now match branch-specific Git keys and select current or legacy base values. Base resolution uses the configured ref and branch base from the same read operation.

Changes: Branch base lookup

Layer / File(s) Summary
Read branch-specific base configuration
apps/server/src/vcs/GitVcsDriverCore.ts, apps/server/src/vcs/GitVcsDriverCore.test.ts
The reader matches branch-specific configuration keys and selects the last value for repeated entries. It prefers t3-merge-base and falls back to gh-merge-base. The regression test covers exact branch matching and fallback behavior.
Use configuration during base resolution
apps/server/src/vcs/GitVcsDriverCore.ts
Base resolution obtains the configured ref and branch base together, then uses the returned branch base.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: juliusmarminge, stbenjam

Merge Risk: Minimal · up to 7ac63

The lookup consolidates Git config reads, with no merge-blocking issue identified.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main performance change: Git status now reads a branch's recorded base with one Git process instead of three.
Description check Passed The description includes all required sections and provides specific problem context, implementation details, scope rationale, verification results, test coverage, benchmark data, and limitations.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

@onlydole

onlydole commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

macOS numbers for the "Not checked" line. These come from 111 minutes of server.trace on 0.1.7-nightly.20261102.4093, macOS 27.2, Apple silicon.

  • 2,752 GitVcsDriver.readConfigValue spans, median 10.8 ms, p90 17 ms.
  • 255 of them (9%) came from the path this PR changes, resolveBaseBranchForNoUpstream and readBranchBase, at a median of 10.2 ms each.
  • The other 91% came from PR lookup and status: rememberPrLookupGitState 661, resolveRemoteRepositoryContext 582, isUnpublishedBranch 558, resolveBranchHeadContext 454 and resolveHostingProvider 235.

The trace records git.args_count but not the keys, so I split the calls by their parent span. The durations are span time and may include waiting for one of the driver's eight git slots. Even so, a config process costs closer to 10 ms here than the 0.8 ms measured on Linux, so each spawn this PR removes saves more on macOS.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants