⚡ Bolt: [performance improvement] Defer PathBuf allocations in Tarjan's SCC DFS#253
⚡ Bolt: [performance improvement] Defer PathBuf allocations in Tarjan's SCC DFS#253bashandbone wants to merge 1 commit into
Conversation
…'s SCC DFS Replaces `v.to_path_buf()` with a borrowed `&Path` reference `v` when looking up indices and lowlinks in `tarjan_dfs`. This eliminates heap allocations (O(E)) during graph traversals while searching for strongly connected components in the dependency invalidator graph. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReplaces repeated PathBuf allocations in the Tarjan SCC DFS with borrowed &Path lookups to reduce heap allocations and improve performance during graph invalidation computations. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull request overview
This PR optimizes the Tarjan SCC DFS used by InvalidationDetector by removing repeated PathBuf allocations during map lookups, relying instead on borrowed &Path queries against RapidMap<PathBuf, _>.
Changes:
- Replace
&v.to_path_buf()withvforindices/lowlinksget/get_mutlookups insidetarjan_dfs. - Reduce per-edge temporary heap allocations during SCC traversal to improve performance on large graphs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
💡 What: Replaced
v.to_path_buf()withv(borrowed&Path) instate.lowlinks.get_mutandstate.indices.getcalls during the recursivetarjan_dfsgraph traversal algorithm.🎯 Why: In recursive DFS traversal functions over a dependency graph, repeatedly calling
.to_path_buf()right before querying a map creates a substantial number of temporary heap allocations (one for every visited edge in SCC check). Using the borrowed reference eliminates O(E) unnecessary heap memory allocations insideRapidMap::get_mut()lookups.📊 Impact: Reduces memory overhead and time spent allocating strings/paths on the heap significantly when computing invalidations for very large and deeply nested dependency graphs.
🔬 Measurement: Run
cargo test -p thread-flow --test invalidation_teststo verify graph traversals still succeed, ensuring there are no regressions. Measure runtime impact withcargo test --test test_performance_large_graphor equivalent backend benchmarks.PR created automatically by Jules for task 7578039882070192528 started by @bashandbone
Summary by Sourcery
Enhancements: