[PR #567] perf: skip managed repo lookup for intermediate post-checkout during rebase #573

New issue

Open

opened 2026-03-02 04:14:01 +03:00 by kerem · 0 comments

kerem commented

2026-03-02 04:14:01 +03:00

Owner

Original Pull Request: https://github.com/git-ai-project/git-ai/pull/567

State: open
Merged: No

perf: skip managed repo lookup for intermediate post-checkout during rebase

Summary

Fixes the rebase_merges / current_hooks margin check failure from PR #530 (27.255% > 25.0% threshold).

During --rebase-merges, git fires post-checkout for every intermediate checkout operation (topology recreation). In hooks mode, each invocation spawns a new process that:

Reads the state JSON (should_forward_repo_state_first)
Opens the git repository (find_hook_repository_from_context)
Runs managed hook logic that is effectively a no-op during rebase

Two optimizations:

Skip repo lookup for intermediate post-checkout during rebase — hook_requires_managed_repo_lookup now returns false for post-checkout when a rebase is in progress, unless it's a terminal event (rebase --abort or pull --rebase fallback). This matches wrapper-mode behavior where intermediate checkout events don't get individual checkout hook processing.
Avoid redundant state file re-read — When should_forward_repo_state_first(None) already returned None (no forward target) and no repo was opened, skip the call to execute_forwarded_hook which would re-read the same state file.

Local nasty benchmark result after fix: 6/6 margin checks passing (hooks rebase_merges dropped from ~27% to ~11% slowdown vs wrapper).

Review & Testing Checklist for Human

Verify skipping checkout_hooks::post_checkout_hook() for intermediate rebase checkouts is safe. The run_managed_hook("post-checkout") handler does real work including calling checkout_hooks::post_checkout_hook(). By skipping the repo lookup, this entire handler is skipped for non-terminal rebase checkouts. Confirm this is consistent with wrapper-mode behavior (where the wrapper handles rebase as one command, not per-checkout).
Verify the abort/pull guards are complete. The skip only fires when !is_rebase_abort_reflog_action() && !is_pull_reflog_action(). Are there any other terminal rebase events that arrive via post-checkout and need managed processing?
Verify the early-return optimization (cached_forward_dir.is_none() && repo.is_none()) doesn't break edge cases where GIT_DIR might not be set in the hook environment (e.g., worktrees, unusual git configurations). The assumption is that the env-based state file lookup is authoritative when repo is None.
Run the nasty benchmark a few times to confirm the fix consistently passes the 25% margin (CI only runs 1 repetition so variance is high).

Notes

The 6 test failures in the local cargo test run are pre-existing on main (snapshot version mismatches and a network error) — not introduced by this change.
Link to Devin run: https://app.devin.ai/sessions/2ae70595150c4b718441b1a3c599eeda
Requested by: @svarlamov

**Original Pull Request:** https://github.com/git-ai-project/git-ai/pull/567 **State:** open **Merged:** No --- # perf: skip managed repo lookup for intermediate post-checkout during rebase ## Summary Fixes the `rebase_merges / current_hooks` margin check failure from PR #530 (`27.255% > 25.0%` threshold). During `--rebase-merges`, git fires `post-checkout` for every intermediate checkout operation (topology recreation). In hooks mode, each invocation spawns a new process that: 1. Reads the state JSON (`should_forward_repo_state_first`) 2. Opens the git repository (`find_hook_repository_from_context`) 3. Runs managed hook logic that is effectively a no-op during rebase Two optimizations: 1. **Skip repo lookup for intermediate `post-checkout` during rebase** — `hook_requires_managed_repo_lookup` now returns `false` for `post-checkout` when a rebase is in progress, unless it's a terminal event (`rebase --abort` or `pull --rebase` fallback). This matches wrapper-mode behavior where intermediate checkout events don't get individual checkout hook processing. 2. **Avoid redundant state file re-read** — When `should_forward_repo_state_first(None)` already returned `None` (no forward target) and no repo was opened, skip the call to `execute_forwarded_hook` which would re-read the same state file. Local nasty benchmark result after fix: `6/6 margin checks passing` (hooks `rebase_merges` dropped from ~27% to ~11% slowdown vs wrapper). ## Review & Testing Checklist for Human - [ ] **Verify skipping `checkout_hooks::post_checkout_hook()` for intermediate rebase checkouts is safe.** The `run_managed_hook("post-checkout")` handler does real work including calling `checkout_hooks::post_checkout_hook()`. By skipping the repo lookup, this entire handler is skipped for non-terminal rebase checkouts. Confirm this is consistent with wrapper-mode behavior (where the wrapper handles rebase as one command, not per-checkout). - [ ] **Verify the abort/pull guards are complete.** The skip only fires when `!is_rebase_abort_reflog_action() && !is_pull_reflog_action()`. Are there any other terminal rebase events that arrive via `post-checkout` and need managed processing? - [ ] **Verify the early-return optimization (`cached_forward_dir.is_none() && repo.is_none()`)** doesn't break edge cases where `GIT_DIR` might not be set in the hook environment (e.g., worktrees, unusual git configurations). The assumption is that the env-based state file lookup is authoritative when `repo` is `None`. - [ ] **Run the nasty benchmark a few times** to confirm the fix consistently passes the 25% margin (CI only runs 1 repetition so variance is high). ### Notes - The 6 test failures in the local `cargo test` run are pre-existing on `main` (snapshot version mismatches and a network error) — not introduced by this change. - Link to Devin run: https://app.devin.ai/sessions/2ae70595150c4b718441b1a3c599eeda - Requested by: @svarlamov  --- <a href="https://app.devin.ai/review/git-ai-project/git-ai/pull/567" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a>