perf: bound concurrent file work with a worker pool#58
Conversation
Walk directories synchronously and run content rewrites plus renames for regular files through a semaphore sized from GOMAXPROCS (4–32). Closes dolph#7 Co-authored-by: Cursor <cursoragent@cursor.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68a5e9b616
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| func (fr *findReplace) acquireWorker() { | ||
| fr.workers <- struct{}{} |
There was a problem hiding this comment.
Sending to fr.workers without initialization now blocks forever, so any findReplace created without workers (for example the existing BenchmarkNova initializer in find_replace_test.go) will hang when it reaches file processing. This regression was introduced by adding the semaphore field but relying on every call site to populate it; initializing lazily (or in a constructor) would avoid deadlocks for zero-value/legacy initializers.
Useful? React with 👍 / 👎.
| var wg sync.WaitGroup | ||
| for _, entry := range files { | ||
| childFile := NewFile(filepath.Join(f.Path, entry.Name())) | ||
| if entry.IsDir() { |
There was a problem hiding this comment.
Switching directory detection to entry.IsDir() changes traversal semantics for symlinked directories: those entries are now treated as regular files, so recursion never enters their targets and replacements are skipped in linked subtrees. The previous path (f.Info().IsDir()) followed symlinks via os.Stat, so this is a functional regression for repositories that include directory symlinks.
Useful? React with 👍 / 👎.
1 participant
Summary
GOMAXPROCS).Test plan
go test ./...Closes #7
Made with Cursor