feat: add global merge-base cache for improved wt list performance #508
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nsiveness On large repos with many branches, `wt select` was slow because git operations (merge-tree, rev-list, diff) can take 400-1800ms each. This adds a thread-local command timeout that kills git commands exceeding 500ms. Architecture: - Thread-local `COMMAND_TIMEOUT` in shell_exec.rs, checked by `run()` - `set_command_timeout()` called per-worker in Rayon's thread pool - Repository and TaskContext stay clean (no timeout fields) - `wt list` passes None (no timeout), `wt select` passes 500ms Closes #461 Co-Authored-By: Claude <[email protected]>
Change the global cache from a single RepoCache to a HashMap keyed by `git_common_dir`. This enables test isolation - each test creates different repos that need their own cached values. The cache entries are wrapped in Arc to allow releasing the outer HashMap lock before accessing cached values, preventing deadlocks when cached methods call each other (e.g., default_branch -> detect_default_branch -> primary_remote). Trade-off documented: every Repository instance must compute `git rev-parse --git-common-dir` for the lookup. Tests require isolation, and the HashMap approach is the simplest solution. Related: #461 Co-Authored-By: Claude <[email protected]>
# Conflicts: # src/commands/list/collect_progressive_impl.rs
The cache refactor incorrectly used get_or_init with error-swallowing fallbacks instead of get_or_try_init with proper error propagation. This caused several issues: - Invalid project config was silently ignored instead of reported - Default branch detection failures returned "main" instead of erroring - is_bare, worktree_base, project_identifier panicked or silently failed Fixed by restoring the original get_or_try_init pattern with proper error propagation using anyhow context. Co-Authored-By: Claude <[email protected]>
Change get_cache from a method to a free function that takes the key directly. This more honestly reflects that it operates on global state rather than instance state. Co-Authored-By: Claude <[email protected]>
Replace RwLock<HashMap> with DashMap for cleaner, more idiomatic caching: - merge_base, worktree_roots, current_branches now use DashMap - entry().or_insert_with() separates caching logic from computation - Reduces ~15 lines per cached method to ~6 lines Co-Authored-By: Claude <[email protected]>
max-sixty
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
git_common_dirfor test isolationRepositoryinstances within a process, avoiding redundant git commands during parallelwt listoperationsTest plan
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]