2036749 - git: skip restoring git checkout in cleanup/finalize when HEAD already on branch

Status: RESOLVED FIXED

(Conduit :: moz-phab, defect, P3)

Description

[Sylvestre Ledru [:Sylvestre]]

The two unconditional git checkout calls in git.py cleanup/finalize are defensive — but a normal submit never moves HEAD (amend/rebase go through _commit_tree + update-ref). Guard both behind _is_head_on_branch (one symbolic-ref HEAD read); only uplift_commits still triggers the real checkout. Saves ~20–60ms per call on large worktrees, ~300–500ms per submit on a Firefox-sized tree, and avoids spurious inotify events.

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 2036749 - git: skip restoring `git checkout` in cleanup/finalize when HEAD already on branch r=sheehan

Inspired by phlay (https://github.com/mystor/phlay), which advertises
"Never modifies working directory, running fast and preventing needless
rebuilds" as a perf win. The mechanism behind that claim in phlay is that
it rewrites commits via git commit-tree (Commit.recommit, lines 502-523
of phlay) and updates branch refs via git update-ref (process_args,
lines 729-734). The script invokes git checkout zero times.

moz-phab is already 90% there: amend_commit (git.py:615) and
_rebase_branch (git.py:741) both use _commit_tree (git.py:584),
which reuses the original tree_hash so the rewritten commits have an
identical tree. The remaining mtime-touching calls are two unconditional
git checkout invocations at git.py:164 (cleanup) and git.py:195
(finalize), defensive restorations from any code path that moved HEAD.
A normal submit never moves HEAD: amend/rebase rewrite via commit-tree +
update-ref. Only uplift_commits (git.py:709, git switch -c …)
actually moves HEAD off self.branch.

Guard both checkouts behind a _is_head_on_branch helper (one
symbolic-ref HEAD read). Uplift still restores correctly because the
guard sees HEAD on a different ref and falls through to the existing
checkout call.

Worktree mtimes were already preserved (preserved tree_hash means
git checkout <same-branch> doesn't rewrite tracked files), but the
checkout still stats every file, rewrites .git/index, and spawns a
subprocess. Bench (benchmarks/bench_cleanup_checkout.py, best-of-7,
warm cache) measures the saved cost per call:

files    BEFORE    GUARD    saved
  100     9.0ms    3.2ms     5.7ms
 1000    22.8ms    3.3ms    19.4ms
10000    20.0ms    2.5ms    17.5ms
50000    58.6ms    1.1ms    57.4ms

Submit calls into the guarded path twice (cleanup + finalize), so
realized savings double these numbers. Linear extrapolation to a
Firefox-sized worktree (~280k files) suggests ~300-500ms saved per
submit. As a side benefit, the skipped checkout also avoids any
spurious IN_OPEN inotify events on watched worktrees.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 2036749 - git: skip restoring `git checkout` in cleanup/finalize when HEAD already on branch r=sheehan

Inspired by phlay (https://github.com/mystor/phlay), which advertises
"Never modifies working directory, running fast and preventing needless
rebuilds" as a perf win. The mechanism behind that claim in phlay is that
it rewrites commits via git commit-tree (Commit.recommit, lines 502-523
of phlay) and updates branch refs via git update-ref (process_args,
lines 729-734). The script invokes git checkout zero times.

moz-phab is already 90% there: amend_commit and _rebase_branch both
use _commit_tree, which reuses the original tree_hash so the
rewritten commits have an identical tree. The remaining mtime-touching
calls are two unconditional git checkout invocations in cleanup and
finalize, defensive restorations from any code path that moved HEAD.
A normal submit never moves HEAD: amend/rebase rewrite via commit-tree +
update-ref. Only uplift_commits (git switch -c …) actually moves HEAD
off self.branch.

Guard both checkouts behind a _is_head_on_branch helper (one
symbolic-ref HEAD read). Uplift still restores correctly because the
guard sees HEAD on a different ref and falls through to the existing
checkout call. A HEAD that can't be resolved to a branch name at all
(detached HEAD, unexpected ref shape) also falls through.

Worktree mtimes were already preserved (preserved tree_hash means
git checkout <same-branch> doesn't rewrite tracked files), but the
checkout still stats every file, rewrites .git/index, and spawns a
subprocess. Measured locally against synthetic worktrees (best-of-7,
warm cache), the cost saved per guarded call:

files    BEFORE    GUARD    saved
  100     9.0ms    3.2ms     5.7ms
 1000    22.8ms    3.3ms    19.4ms
10000    20.0ms    2.5ms    17.5ms
50000    58.6ms    1.1ms    57.4ms

Submit calls into the guarded path three times (cleanup twice + finalize),
so realized savings multiply these numbers. Linear extrapolation to a
Firefox-sized worktree (~280k files) suggests ~300-500ms saved per
submit. As a side benefit, the skipped checkout also avoids any
spurious IN_OPEN inotify events on watched worktrees.

tests/test_git_call_counts.py is updated for the new counts: submit of
a single commit no longer spawns git checkout at all, spending three
cheap symbolic-ref reads instead. A new integration test pins the
intended behaviour by asserting submit leaves worktree file mtimes and
.git/index untouched (it fails without the guard), and unit tests cover
_is_head_on_branch plus both guarded call sites.