diff --git a/.claude/rules/caching-strategy.md b/.claude/rules/caching-strategy.md index cce08a14a..4206f331a 100644 --- a/.claude/rules/caching-strategy.md +++ b/.claude/rules/caching-strategy.md @@ -33,6 +33,7 @@ trade-offs. - `primary_remote()` — git config, doesn't change - `default_branch()` — from git config or detection, doesn't change - `merge_base()` — keyed by (commit1, commit2) pair +- `ahead_behind` — keyed by (base_ref, branch_name), populated by `batch_ahead_behind()` **Not cached (intentionally):** - `is_dirty()` — changes as we stage/commit diff --git a/src/commands/list/collect.rs b/src/commands/list/collect.rs index 458aedd73..f6d580fc4 100644 --- a/src/commands/list/collect.rs +++ b/src/commands/list/collect.rs @@ -974,11 +974,14 @@ pub fn collect( .ok() .and_then(|s| s.parse().ok()) .unwrap_or(50); + // batch_ahead_behind populates the Repository cache with all counts let ahead_behind = repo.batch_ahead_behind(&default_branch); - if !ahead_behind.is_empty() { - options.branch_ahead_behind = ahead_behind; - options.skip_expensive_threshold = Some(threshold); - } + // Filter to stale branches (behind > threshold). The set indicates which + // branches should skip expensive tasks; counts come from the cache. + options.stale_branches = ahead_behind + .into_iter() + .filter_map(|(branch, (_, behind))| (behind > threshold).then_some(branch)) + .collect(); } // Note: URL template expansion is deferred to task spawning (in collect_worktree_progressive @@ -1023,7 +1026,7 @@ pub fn collect( let default_branch_clone = default_branch.clone(); let target_clone = integration_target.clone(); let expected_results_clone = expected_results.clone(); - // Move options into the worker thread (not cloned - it can be large with branch_ahead_behind) + // Move options into the worker thread (not cloned - contains stale_branches set) let main_path = main_worktree.path.clone(); // Prepare branch data if needed (before moving into closure) @@ -1081,7 +1084,6 @@ pub fn collect( &target_clone, &options, &expected_results_clone, - &tx_worker, )); } diff --git a/src/commands/list/collect_progressive_impl.rs b/src/commands/list/collect_progressive_impl.rs index b3eefb7e6..fccb0cf2d 100644 --- a/src/commands/list/collect_progressive_impl.rs +++ b/src/commands/list/collect_progressive_impl.rs @@ -35,8 +35,8 @@ use super::model::{ /// Options for controlling what data to collect. /// -/// Uses a skip set to control which tasks are spawned. Tasks not in the set -/// will be computed; tasks in the set will be skipped. +/// This is operation parameters for a single `wt list` invocation, not a cache. +/// For cached repo data, see Repository's global cache. #[derive(Clone, Default)] pub struct CollectOptions { /// Tasks to skip (not compute). Empty set means compute everything. @@ -50,15 +50,15 @@ pub struct CollectOptions { /// Expanded per-item in task spawning (post-skeleton) to minimize time-to-skeleton. pub url_template: Option, - /// Pre-fetched ahead/behind counts for branches (from batched `git for-each-ref`). - /// Used to skip expensive tasks for branches that are far behind the default branch. - /// The counts are (ahead, behind). If empty, all tasks run normally. - pub branch_ahead_behind: std::collections::HashMap, - - /// Threshold for skipping expensive tasks. Branches with `behind > threshold` - /// will skip merge-base-dependent tasks (HasFileChanges, IsAncestor, WouldMergeAdd, - /// BranchDiff, MergeTreeConflicts). AheadBehind uses batch data instead of skipping. - /// CommittedTreesMatch is cheap and kept for integration detection. + /// Branches to skip expensive tasks for (behind > threshold). + /// + /// Presence in set = skip expensive tasks for this branch (HasFileChanges, + /// IsAncestor, WouldMergeAdd, BranchDiff, MergeTreeConflicts). + /// + /// Built by filtering `batch_ahead_behind()` results on local branches only. + /// Remote-only branches are never in this set (they use individual git commands). + /// The threshold (default 50) is applied at construction time. Ahead/behind + /// counts are cached in Repository and looked up by AheadBehindTask. /// /// **Display implications:** When tasks are skipped: /// - BranchDiff column shows `…` instead of diff stats @@ -70,7 +70,7 @@ pub struct CollectOptions { /// /// TODO: Consider adding a visible indicator in Status column when integration /// checks are skipped, so users know the `⊂` symbol may be incomplete. - pub skip_expensive_threshold: Option, + pub stale_branches: std::collections::HashSet, } /// Context for task computation. Cloned and moved into spawned threads. @@ -195,19 +195,6 @@ const EXPENSIVE_TASKS: &[TaskKind] = &[ TaskKind::MergeTreeConflicts, // git merge-tree simulation ]; -/// Check if a branch should skip expensive tasks based on how far behind it is. -/// Returns Some((ahead, behind)) if should skip, None otherwise. -fn should_skip_expensive(branch: Option<&str>, options: &CollectOptions) -> Option<(usize, usize)> { - let threshold = options.skip_expensive_threshold?; - let branch = branch?; - let &(ahead, behind) = options.branch_ahead_behind.get(branch)?; - if behind > threshold { - Some((ahead, behind)) - } else { - None - } -} - /// Generate work items for a worktree. /// /// Returns a list of work items representing all tasks that should run for this @@ -259,20 +246,11 @@ pub fn work_items_for_worktree( item_url, }; - // Check if this branch is far behind and should skip expensive tasks. - // If so, we get the batch-computed (ahead, behind) to send immediately. - let batch_counts = should_skip_expensive(wt.branch.as_deref(), options); - - // If we have batch counts and AheadBehind isn't skipped, send result immediately - if let Some((ahead, behind)) = batch_counts - && !skip.contains(&TaskKind::AheadBehind) - { - expected_results.expect(item_idx, TaskKind::AheadBehind); - let _ = tx.send(Ok(TaskResult::AheadBehind { - item_idx, - counts: AheadBehind { ahead, behind }, - })); - } + // Check if this branch is stale and should skip expensive tasks. + let is_stale = wt + .branch + .as_deref() + .is_some_and(|b| options.stale_branches.contains(b)); let mut items = Vec::with_capacity(15); @@ -304,11 +282,8 @@ pub fn work_items_for_worktree( if skip.contains(&kind) { continue; } - // Skip AheadBehind if we already sent batch data - if batch_counts.is_some() && kind == TaskKind::AheadBehind { - continue; - } - if batch_counts.is_some() && EXPENSIVE_TASKS.contains(&kind) { + // Skip expensive tasks for stale branches (far behind default branch) + if is_stale && EXPENSIVE_TASKS.contains(&kind) { continue; } add_item(kind); @@ -342,7 +317,6 @@ pub fn work_items_for_branch( target: &str, options: &CollectOptions, expected_results: &Arc, - tx: &Sender>, ) -> Vec { let skip = &options.skip_tasks; @@ -356,20 +330,8 @@ pub fn work_items_for_branch( item_url: None, // Branches without worktrees don't have URLs }; - // Check if this branch is far behind and should skip expensive tasks. - // If so, we get the batch-computed (ahead, behind) to send immediately. - let batch_counts = should_skip_expensive(Some(branch_name), options); - - // If we have batch counts and AheadBehind isn't skipped, send result immediately - if let Some((ahead, behind)) = batch_counts - && !skip.contains(&TaskKind::AheadBehind) - { - expected_results.expect(item_idx, TaskKind::AheadBehind); - let _ = tx.send(Ok(TaskResult::AheadBehind { - item_idx, - counts: AheadBehind { ahead, behind }, - })); - } + // Check if this branch is stale and should skip expensive tasks. + let is_stale = options.stale_branches.contains(branch_name); let mut items = Vec::with_capacity(11); @@ -397,11 +359,8 @@ pub fn work_items_for_branch( if skip.contains(&kind) { continue; } - // Skip AheadBehind if we already sent batch data - if batch_counts.is_some() && kind == TaskKind::AheadBehind { - continue; - } - if batch_counts.is_some() && EXPENSIVE_TASKS.contains(&kind) { + // Skip expensive tasks for stale branches (far behind default branch) + if is_stale && EXPENSIVE_TASKS.contains(&kind) { continue; } add_item(kind); @@ -447,9 +406,21 @@ impl Task for AheadBehindTask { fn compute(ctx: TaskContext) -> Result { let base = ctx.require_default_branch(Self::KIND)?; let repo = ctx.repo(); - let (ahead, behind) = repo - .ahead_behind(base, &ctx.commit_sha) - .map_err(|e| ctx.error(Self::KIND, e))?; + + // Check cache first (populated by batch_ahead_behind if it ran). + // Cache lookup has minor overhead (rev-parse for cache key + allocations), + // but saves the expensive ahead_behind computation on cache hit. + let (ahead, behind) = if let Some(branch) = ctx.branch.as_deref() { + if let Some(counts) = repo.get_cached_ahead_behind(base, branch) { + counts + } else { + repo.ahead_behind(base, &ctx.commit_sha) + .map_err(|e| ctx.error(Self::KIND, e))? + } + } else { + repo.ahead_behind(base, &ctx.commit_sha) + .map_err(|e| ctx.error(Self::KIND, e))? + }; Ok(TaskResult::AheadBehind { item_idx: ctx.item_idx, diff --git a/src/git/repository/mod.rs b/src/git/repository/mod.rs index 3511179c1..07c2ead54 100644 --- a/src/git/repository/mod.rs +++ b/src/git/repository/mod.rs @@ -80,6 +80,9 @@ struct RepoCache { project_config: OnceCell>, /// Merge-base cache: (commit1, commit2) -> merge_base_sha merge_base: DashMap<(String, String), String>, + /// Batch ahead/behind cache: (base_ref, branch_name) -> (ahead, behind) + /// Populated by batch_ahead_behind(), used by get_cached_ahead_behind() + ahead_behind: DashMap<(String, String), (usize, usize)>, // ========== Per-worktree values (keyed by path) ========== /// Worktree root paths: worktree_path -> canonicalized root @@ -1137,6 +1140,9 @@ impl Repository { /// Uses `git for-each-ref --format='%(ahead-behind:BASE)'` (git 2.36+) to get /// all counts in a single command. Returns a map from branch name to (ahead, behind). /// + /// Results are cached so subsequent lookups via `get_cached_ahead_behind()` avoid + /// running individual git commands (though cache access still has minor overhead). + /// /// On git < 2.36 or if the command fails, returns an empty map. pub fn batch_ahead_behind( &self, @@ -1156,7 +1162,10 @@ impl Repository { } }; - output + // Get cache (may fail if git_common_dir detection fails) + let cache = self.compute_git_common_dir().ok().map(get_cache); + + let results: std::collections::HashMap = output .lines() .filter_map(|line| { // Format: "branch-name ahead behind" @@ -1164,9 +1173,29 @@ impl Repository { let behind: usize = parts.next()?.parse().ok()?; let ahead: usize = parts.next()?.parse().ok()?; let branch = parts.next()?.to_string(); + // Cache each result for later lookup + if let Some(ref cache) = cache { + cache + .ahead_behind + .insert((base.to_string(), branch.clone()), (ahead, behind)); + } Some((branch, (ahead, behind))) }) - .collect() + .collect(); + + results + } + + /// Get cached ahead/behind counts for a branch. + /// + /// Returns cached results from a prior `batch_ahead_behind()` call, or None + /// if the branch wasn't in the batch or batch wasn't run. + pub fn get_cached_ahead_behind(&self, base: &str, branch: &str) -> Option<(usize, usize)> { + let cache = get_cache(self.compute_git_common_dir().ok()?); + cache + .ahead_behind + .get(&(base.to_string(), branch.to_string())) + .map(|r| *r) } /// List all local branches with their HEAD commit SHA.