perf: respect ignore rules in Swift module discovery#2
Merged
Conversation
discover_swift_packages_recursive hand-rolled a recursive directory walk that skipped only hidden dirs and node_modules/build/DerivedData/Pods. It did not skip Cargo's target/ nor honor .gitignore, so indexing a Rust project stat'd its way through the entire build-artifact tree (60G, 15k+ dirs here). Because module discovery runs in an untimed gap between the "discovered" and "extracting" phase prints, this showed up as a multi-second freeze that the per-phase timings did not explain. Replace the hand-rolled walk with ignore::WalkBuilder — the same ignore-aware walker discover.rs already uses for source files — honoring .gitignore (require_git(false) so it also applies in non-git checkouts), skipping hidden dirs, and explicitly pruning build trees including target/. Package registration semantics are preserved (Sources/Tests registration; a package nested inside another is registered once via lexical-sort ancestor dedup). Indexing this project drops from ~13-21s to 0.6s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves Swift module discovery performance by replacing a custom recursive filesystem walk with an ignore-aware walker, preventing Swift discovery from scanning large ignored/build artifact trees in non-Swift projects.
Changes:
- Uses
ignore::WalkBuilderto honor.gitignore, hidden directories, and explicit build-directory pruning. - Refactors Swift package and test target registration into helper functions.
- Adds regression tests for ignored directories, build artifact pruning, and nested package handling.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
grapha-swift/src/module_discovery.rs |
Reworks Swift package discovery and adds regression tests. |
grapha-swift/Cargo.toml |
Adds the ignore dependency. |
Cargo.lock |
Records ignore as a grapha-swift dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+31
to
+35
| .filter_entry(|entry| { | ||
| !entry | ||
| .file_name() | ||
| .to_str() | ||
| .is_some_and(|name| PRUNED_DIRS.contains(&name)) |
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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
Indexing a Rust project (this repo) froze for ~13–21s with no phase in the progress output accounting for the time. Root cause:
grapha-swift'sdiscover_swift_packages_recursivehand-rolled a recursive directory walk that skipped only hidden dirs andnode_modules/build/DerivedData/Pods— it did not skip Cargo'starget/nor honor.gitignore. Swift module discovery runs even for pure-Rust projects (the Swift plugin is always registered), so itstat/read_dir'd through the entire build-artifact tree (here: ~60 GB, 15,419 directories).It hid from the timings because it runs inside
discover_modules, in the untimed gap between the "discovered files" and "extracting" phase prints — so per-phase numbers summed to ~0.6s while wall time was 13–21s. A profiler sample confirmed ~92% of runtime was indiscover_swift_packages_recursive→stat/getdirentriesinsidetarget/.Fix
Replace the hand-rolled walk with
ignore::WalkBuilder— the same ignore-aware walkergrapha/src/discover.rsalready uses for source-file discovery, so the two are now consistent. It:.gitignore(require_git(false)so it also applies to non-git checkouts),node_modules/build/DerivedData/Pods/target) as belt-and-suspenders,Result
Indexing this project drops from ~13–21s to 0.6s.
Test plan
cargo test -p grapha-swift— 116 tests pass, incl. 3 new regression tests:prunes_build_artifact_directories(Package.swift undertarget/is ignored)skips_gitignored_directoriesnested_package_is_registered_oncecargo clippy -p grapha-swift --all-targets— cleangrapha index .on this repo verified 0.6s with identical node/edge counts🤖 Generated with Claude Code