Skip to content

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

Open
svarlamov wants to merge 3 commits intomainfrom
devin/1771599590-hooks-rebase-perf
Open

perf: skip managed repo lookup for intermediate post-checkout during rebase#567
svarlamov wants to merge 3 commits intomainfrom
devin/1771599590-hooks-rebase-perf

Conversation

@svarlamov
Copy link
Member

@svarlamov svarlamov commented Feb 20, 2026

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 rebasehook_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


Open with Devin

…rebase

During --rebase-merges, git fires post-checkout for every intermediate
checkout operation. In hooks mode each invocation spawns a new process,
reads the state JSON, opens the repository, and runs managed hook logic
that is effectively a no-op during rebase (post-rewrite handles
attribution at the end).

This adds two optimizations:

1. In hook_requires_managed_repo_lookup, skip the expensive repository
   open for post-checkout events during rebase unless this is a terminal
   event (abort or pull-rebase fallback).

2. In handle_git_hook_invocation, when we already determined there is no
   forward target and no repo was opened, return immediately instead of
   re-reading the state file in execute_forwarded_hook.

Co-Authored-By: Sasha Varlamov <sasha@sashavarlamov.com>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@git-ai-cloud
Copy link

git-ai-cloud bot commented Feb 20, 2026

No AI authorship found for these commits. Please install git-ai to start tracking AI generated code in your commits.

@git-ai-cloud-dev
Copy link

No AI authorship found for these commits. Please install git-ai to start tracking AI generated code in your commits.

@CLAassistant
Copy link

CLAassistant commented Feb 20, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ svarlamov
❌ devin-ai-integration[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

devin-ai-integration[bot]

This comment was marked as resolved.

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@svarlamov
Copy link
Member Author

devin fix the lints

Co-Authored-By: Sasha Varlamov <sasha@sashavarlamov.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants