⚡ Bolt: [performance improvement] eliminate redundant path allocations in DAG traversals#168
Conversation
…s in DAG traversals Replaces redundant `v.to_path_buf()` allocations with zero-allocation `&Path` lookups inside the high-frequency dependency loops of Tarjan's Strongly Connected Components DFS algorithm (`tarjan_dfs`). 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 GuideOptimizes the Tarjan DFS invalidation algorithm by avoiding repeated PathBuf allocations during HashMap/HashSet lookups, instead allocating once per node and using borrowed &Path for map access, and documents this performance pattern in the Bolt guide. Class diagram for Tarjan DFS invalidation structuresclassDiagram
class InvalidationDetector {
+tarjan_dfs(v: &Path, state: &mut TarjanState, sccs: &mut Vec~Vec~PathBuf~~)
+graph: DependencyGraph
}
class TarjanState {
+indices: HashMap~PathBuf, usize~
+lowlinks: HashMap~PathBuf, usize~
+stack: Vec~PathBuf~
+on_stack: HashSet~PathBuf~
+index_counter: usize
}
class DependencyGraph {
+get_dependencies(node: &Path): Vec~&Path~
}
InvalidationDetector --> TarjanState
InvalidationDetector --> DependencyGraph
TarjanState "1" o-- "*" PathBuf
DependencyGraph ..> Path
InvalidationDetector ..> Path
TarjanState ..> usize
TarjanState ..> HashMap
TarjanState ..> HashSet
TarjanState ..> Vec
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR improves performance in the incremental invalidation cycle-detection path by eliminating redundant PathBuf allocations during Tarjan SCC DFS map lookups, leveraging borrowed &Path lookups (Borrow<Path>) against RapidMap<PathBuf, _>.
Changes:
- Allocate a single
PathBufper visited node intarjan_dfs, reusing it for initial inserts/pushes. - Replace repeated
v.to_path_buf()allocations inget/get_mutcalls with borrowed&Pathlookups. - Document the performance learning in
.jules/bolt.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/flow/src/incremental/invalidation.rs | Removes redundant PathBuf allocations in Tarjan DFS by using borrowed key lookups and reusing a single per-node buffer. |
| .jules/bolt.md | Adds an entry documenting the allocation-avoidance pattern for DFS lookup paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
💡 What: Eliminated repetitive O(E) heap allocations for
PathBuflookups insidetarjan_dfsby relying on Rust'sBorrowtrait for.get()and.get_mut()lookups using the borrowed&Pathreference.🎯 Why: Creating an owned
PathBufpurely to look up a value inside aHashMapis extremely inefficient during graph traversals as it allocates memory repeatedly for the same string value across every edge visited.📊 Impact: Reduces redundant heap allocations in
tarjan_dfsfrom O(Edges) to strictly O(Nodes), lowering execution time and memory fragmentation for larger dependency graphs.🔬 Measurement: Verify functionality and absence of regressions using
cargo test -p thread-flow --test invalidation_testsandcargo clippy.PR created automatically by Jules for task 7373454954958409554 started by @bashandbone
Summary by Sourcery
Optimize Tarjan SCC traversal to reduce redundant path allocations during invalidation analysis.
Enhancements:
Documentation: