Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions benches/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,9 @@ because its ~15 GiB fixture must never build on a hosted CI runner):

| Variant | Expected | What it measures |
|---------|----------|------------------|
| `prune_e2e/dry_run_probe_cold` | ~160 ms | full parallel scan, probes re-run (`.git/wt/cache/` cleared; git's own caches stay warm — the "first prune after fetching main" shape) |
| `prune_e2e/dry_run_warm` | ~90 ms | steady-state re-scan, probes hit sha_cache |
| `prune_e2e/live` | ~620 ms | probe-cold scan + serial removal of the 8 candidates (~60 ms each, under the scan write lock) |
| `prune_e2e/dry_run_probe_cold` | ~150 ms | full parallel scan, probes re-run (`.git/wt/cache/` cleared; git's own caches stay warm — the "first prune after fetching main" shape) |
| `prune_e2e/dry_run_warm` | ~60 ms | steady-state re-scan, probes hit sha_cache |
| `prune_e2e/live` | ~600 ms | probe-cold scan + serial removal of the 8 candidates (worktree candidates ~100 ms each — mostly the fsmonitor-daemon stop — branch-only ~25 ms, under the scan write lock, reusing scan-time plans) |
| `prune_real_repo/dry_run_warm` | ~0.25–0.8 s | steady-state scan of 72 items (36 worktrees + 36 branches) at 331k-commit scale |
| `prune_real_repo/dry_run_probe_cold` | ~0.6–1 s | the same 72-item scan with probes re-running at real cost (statuses stay stat-warm) |
| `first_output/remove` | ~86 ms | single-target validation up to first output (`benches/time_to_first_output.rs`) |
Expand All @@ -232,10 +232,12 @@ alone; a live one consumes the candidates). Expected one-shots on the
`git status` at ~4.5 s per fresh worktree; the probes are `merge-base
--is-ancestor` ~40 ms and `merge-tree --write-tree` ~130 ms (vs 4–25 ms
synthetic, where shallow history walks bottom out at subprocess-spawn cost)
- **live ~12 s wall** — all 24 removals serialize under the scan write lock
inside the `prune-scan` window: each of the 12 worktree candidates takes
~0.5–1.7 s (pre-remove re-checks plus drain waits), branch-only candidates
~50 ms
- **live ~12 s wall** (measured before the removal chain was trimmed to one
clean check + daemon stop + rename + CAS delete; re-measure on the next
fixture rebuild) — all 24 removals serialize under the scan write lock
inside the `prune-scan` window: each of the 12 worktree candidates took
~0.5–1.7 s, dominated by re-validation statuses running after the fsmonitor
daemon stop, branch-only candidates ~50 ms

This is the "prune takes many seconds" experience users report: worktree
count × stat-cold statuses bounds the scan, and removals extend it serially.
Expand Down
7 changes: 4 additions & 3 deletions benches/prune.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
// parallel on the rayon pool. Dominated by the merge-tree/merge-base
// probes, whose results persist in `.git/wt/cache/` (sha_cache), so the
// first scan after new commits is cold and later scans are warm.
// 2. The removals — each integrated candidate runs the full removal chain
// (pre-remove re-checks, fsmonitor stop, rename-to-trash, branch CAS
// delete) serially under the write side of the scan lock.
// 2. The removals — each integrated candidate runs the removal chain
// (final clean check, fsmonitor stop, rename-to-trash, branch CAS
// delete) serially under the write side of the scan lock, reusing the
// removal plan its scan check computed.
//
// The fixture is `wt_perf::create_prune_repo_at`: squash-merged candidates
// (integrated by content — the expensive probe path, the post-PR-squash shape
Expand Down
9 changes: 9 additions & 0 deletions src/commands/picker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2808,6 +2808,7 @@ pub mod tests {
branch_name: Some("feature".to_string()),
deletion_mode: BranchDeletionMode::SafeDelete,
target_branch: Some("main".to_string()),
integration_reason: None,
force_worktree: false,
removed_commit: None,
};
Expand Down Expand Up @@ -2902,6 +2903,7 @@ pub mod tests {
branch_name: None,
deletion_mode: BranchDeletionMode::SafeDelete,
target_branch: Some("main".to_string()),
integration_reason: None,
force_worktree: false,
removed_commit: None,
};
Expand Down Expand Up @@ -2992,6 +2994,7 @@ pub mod tests {
branch_name: Some("feature".to_string()),
deletion_mode: BranchDeletionMode::SafeDelete,
target_branch: Some("main".to_string()),
integration_reason: None,
force_worktree: false,
removed_commit: None,
};
Expand Down Expand Up @@ -3679,6 +3682,7 @@ pub mod tests {
branch_name: Some("feature".to_string()),
deletion_mode: BranchDeletionMode::SafeDelete,
target_branch: Some("main".to_string()),
integration_reason: None,
force_worktree: false,
removed_commit: None,
};
Expand All @@ -3694,6 +3698,7 @@ pub mod tests {
branch_name: None,
deletion_mode: BranchDeletionMode::SafeDelete,
target_branch: Some("main".to_string()),
integration_reason: None,
force_worktree: false,
removed_commit: None,
};
Expand Down Expand Up @@ -4041,6 +4046,7 @@ pub mod tests {
branch_name: Some(branch.to_string()),
deletion_mode: mode,
target_branch: Some("main".to_string()),
integration_reason: None,
force_worktree: false,
removed_commit: None,
};
Expand Down Expand Up @@ -4138,6 +4144,7 @@ pub mod tests {
branch_name: Some("x".to_string()),
deletion_mode: BranchDeletionMode::SafeDelete,
target_branch: Some("main".to_string()),
integration_reason: None,
force_worktree: false,
removed_commit: None,
};
Expand Down Expand Up @@ -4196,6 +4203,7 @@ pub mod tests {
branch_name: Some("feature".to_string()),
deletion_mode: BranchDeletionMode::SafeDelete,
target_branch: Some("main".to_string()),
integration_reason: None,
force_worktree: false,
removed_commit: None,
};
Expand Down Expand Up @@ -4241,6 +4249,7 @@ pub mod tests {
branch_name: Some("feature".to_string()),
deletion_mode: BranchDeletionMode::SafeDelete,
target_branch: Some("main".to_string()),
integration_reason: None,
force_worktree: false,
removed_commit: None,
};
Expand Down
15 changes: 14 additions & 1 deletion src/commands/repository_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,12 +287,24 @@ impl RepositoryCliExt for Repository {
primary_path
};

// Resolve target branch for integration reason display
// Resolve target branch and integration verdict for display and
// retention prediction. The actual branch deletion re-decides against
// fresh refs (`delete_branch_if_safe`'s CAS), so this is display-only.
let default_branch = self.default_branch();
let target_branch = match (&default_branch, &branch_name) {
(Some(db), Some(bn)) if db == bn => None,
_ => default_branch,
};
let (integration_reason, target_branch) = match compute_integration_reason(
self,
snapshot,
branch_name.as_deref(),
target_branch.as_deref(),
deletion_mode,
) {
(reason, Some(effective_target)) => (reason, Some(effective_target)),
(reason, None) => (reason, target_branch),
};

// Capture commit SHA before removal for post-remove hook template variables.
// This ensures {{ commit }} references the removed worktree's state.
Expand All @@ -312,6 +324,7 @@ impl RepositoryCliExt for Repository {
branch_name,
deletion_mode,
target_branch,
integration_reason,
force_worktree,
removed_commit,
})
Expand Down
114 changes: 77 additions & 37 deletions src/commands/step/prune.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use worktrunk::trace::Span;
use super::super::hook_plan::{ApprovedHookPlan, HookPlan, HookPlanBuilder};
use super::super::hooks::HookAnnouncer;
use super::super::repository_ext::{RemoveTarget, RepositoryCliExt};
use super::super::worktree::RemoveResult;
use crate::output::{BackgroundFallbackMode, handle_remove_output};

/// A candidate worktree or branch selected for removal.
Expand Down Expand Up @@ -77,6 +78,10 @@ impl Candidate {
}
}

/// The current-worktree candidate held back until every other removal ran
/// (its removal cd's the shell to the primary), with its scan-time plan.
type DeferredCurrent = (Candidate, Option<RemoveResult>);

#[derive(Clone, Copy)]
enum CandidateKind {
Current,
Expand Down Expand Up @@ -188,7 +193,17 @@ struct RemovalContext<'a> {

/// Try to remove a candidate immediately. Returns Ok(true) if removed,
/// Ok(false) if skipped (preparation error), Err on execution error.
fn try_remove(candidate: &Candidate, ctx: &RemovalContext<'_>) -> anyhow::Result<bool> {
///
/// `plan` is the scan-time `prepare_worktree_removal` result from
/// [`check_one`]. `Prunable` candidates arrive plan-less and prepare here,
/// under the write lock, because preparing them prunes stale worktree
/// metadata. Scan-time plans may be stale by execution; the pre-rename
/// `ensure_clean` and the branch-delete CAS re-validate what matters.
fn try_remove(
candidate: &Candidate,
plan: Option<RemoveResult>,
ctx: &RemovalContext<'_>,
) -> anyhow::Result<bool> {
let _span = Span::new(format!("prune-remove:{}", candidate.label));
// The guard protects `()` — there is no shared state to corrupt, so a
// poisoned lock is meaningless here. Recover the guard rather than
Expand All @@ -201,21 +216,23 @@ fn try_remove(candidate: &Candidate, ctx: &RemovalContext<'_>) -> anyhow::Result
return Ok(true);
}

let target = candidate.remove_target()?;
let plan = match ctx.repo.prepare_worktree_removal(
target,
BranchDeletionMode::SafeDelete,
false,
None,
Some(ctx.worktrees),
Some(ctx.snapshot),
) {
Ok(plan) => plan,
Err(_) => {
// prepare_worktree_removal is the gate: if the worktree can't
// be removed (dirty, locked, etc.), it's simply not selected.
return Ok(false);
}
let plan = match plan {
Some(plan) => plan,
None => match ctx.repo.prepare_worktree_removal(
candidate.remove_target()?,
BranchDeletionMode::SafeDelete,
false,
None,
Some(ctx.worktrees),
Some(ctx.snapshot),
) {
Ok(plan) => plan,
Err(_) => {
// prepare_worktree_removal is the gate: if the worktree can't
// be removed (dirty, locked, etc.), it's simply not selected.
return Ok(false);
}
},
};
let mut announcer = HookAnnouncer::new(ctx.repo, true);
// `SynchronousForNonCurrent`: prune keeps the rename-failure fallback's
Expand Down Expand Up @@ -255,9 +272,15 @@ struct SkippedApproval {
struct CheckOutcome {
effective_target: String,
reason: Option<IntegrationReason>,
/// Result of `prepare_worktree_removal` — the same gate `wt remove` uses.
/// Dirty, locked, and primary worktrees end up `false` and are filtered
/// silently, never reported as "younger than" or processed downstream.
/// Removal plan from `prepare_worktree_removal` — the same gate `wt
/// remove` uses, computed here on the parallel scan so `try_remove`
/// doesn't re-derive it (a `git status` per worktree) under the write
/// lock. `None` means not removable (dirty, locked, primary — filtered
/// silently, never reported as "younger than") — except for `Prunable`
/// items, which are always removable but plan in `try_remove`: preparing
/// them calls `prune_worktrees()`, a mutation that must stay serialized.
plan: Option<RemoveResult>,
/// Whether the item passed the removability gate (see `plan`).
removable: bool,
/// `Some(_)` if `min_age` is set and the age could be resolved; the
/// caller compares against `min_age_duration` to decide on the skip.
Expand All @@ -284,12 +307,23 @@ fn check_one(
return Ok(CheckOutcome {
effective_target,
reason,
plan: None,
removable: false,
age: None,
});
}
let removable = match &item.source {
CheckSource::Prunable { .. } | CheckSource::Orphan => true,
let plan = match &item.source {
CheckSource::Prunable { .. } => None,
CheckSource::Orphan => repo
.prepare_worktree_removal(
RemoveTarget::Branch(&item.integration_ref),
BranchDeletionMode::SafeDelete,
false,
None,
Some(worktrees),
Some(snapshot),
)
.ok(),
CheckSource::Linked { wt_idx } => {
let wt = &worktrees[*wt_idx];
let target = match &wt.branch {
Expand All @@ -304,9 +338,13 @@ fn check_one(
Some(worktrees),
Some(snapshot),
)
.is_ok()
.ok()
}
};
let removable = match &item.source {
CheckSource::Prunable { .. } => true,
CheckSource::Orphan | CheckSource::Linked { .. } => plan.is_some(),
};
let age = if min_age_duration > Duration::ZERO {
match &item.source {
CheckSource::Linked { wt_idx } => worktree_age(repo, &worktrees[*wt_idx], now_secs)?,
Expand All @@ -319,6 +357,7 @@ fn check_one(
Ok(CheckOutcome {
effective_target,
reason,
plan,
removable,
age,
})
Expand Down Expand Up @@ -401,6 +440,7 @@ fn gather_check_items(
// Track branches seen via worktree entries so we don't double-count
// in the orphan branch scan below.
let mut seen_branches: HashSet<String> = HashSet::new();
let is_bare = repo.is_bare().context("checking whether repo is bare")?;

for (idx, wt) in worktrees.iter().enumerate() {
if let Some(branch) = &wt.branch {
Expand Down Expand Up @@ -435,13 +475,12 @@ fn gather_check_items(
continue;
}

// Skip main worktree (non-linked); in bare repos all are linked,
// so the default-branch check above is the primary guard.
let wt_tree = repo.worktree_at(&wt.path);
if !wt_tree
.is_linked()
.context("checking whether worktree is linked")?
{
// Skip the main worktree: `git worktree list` puts it first (a
// documented guarantee), so no per-worktree `git rev-parse` probe is
// needed. Bare repos have no main worktree — `list_worktrees()`
// filters the bare entry, leaving only linked worktrees — so the
// default-branch check above is their primary guard.
if idx == 0 && !is_bare {
continue;
}

Expand Down Expand Up @@ -881,8 +920,8 @@ pub fn step_prune(
// `try_remove` immediately for positives. The current worktree is the one
// exception: its removal cd's to the primary, so defer it until last.
let scan_span = Span::new("prune-scan");
let (removed, deferred_current) =
std::thread::scope(|s| -> anyhow::Result<(Vec<Candidate>, Option<Candidate>)> {
let (removed, deferred_current) = std::thread::scope(
|s| -> anyhow::Result<(Vec<Candidate>, Option<DeferredCurrent>)> {
let (tx, rx) = chan::unbounded::<(usize, anyhow::Result<CheckOutcome>)>();
// Pre-shadow with references so `move` on s.spawn moves only `tx`
// (so it's dropped when the spawn ends and `rx` can terminate),
Expand Down Expand Up @@ -915,7 +954,7 @@ pub fn step_prune(
});

let mut removed: Vec<Candidate> = Vec::new();
let mut deferred_current: Option<Candidate> = None;
let mut deferred_current: Option<DeferredCurrent> = None;
for (idx, outcome) in &rx {
let outcome = outcome.context("checking branch integration")?;
let Some(_reason) = outcome.reason else {
Expand Down Expand Up @@ -972,21 +1011,22 @@ pub fn step_prune(
kind,
};
if matches!(candidate.kind, CandidateKind::Current) {
deferred_current = Some(candidate);
} else if try_remove(&candidate, &removal_ctx)
deferred_current = Some((candidate, outcome.plan));
} else if try_remove(&candidate, outcome.plan, &removal_ctx)
.with_context(|| candidate.removal_context())?
{
removed.push(candidate);
}
}
Ok((removed, deferred_current))
})?;
},
)?;
drop(scan_span);

let mut removed = removed;
// Remove deferred current worktree last (cd-to-primary happens here)
if let Some(current) = deferred_current
&& try_remove(&current, &removal_ctx).with_context(|| current.removal_context())?
if let Some((current, plan)) = deferred_current
&& try_remove(&current, plan, &removal_ctx).with_context(|| current.removal_context())?
{
removed.push(current);
}
Expand Down
Loading
Loading