⚡ Bolt: [performance improvement] Reduce O(E) allocation churn in graph construction#164
⚡ Bolt: [performance improvement] Reduce O(E) allocation churn in graph construction#164bashandbone wants to merge 1 commit intomainfrom
Conversation
…yGraph * Replaces `ensure_node`'s unconditional `.entry(file.to_path_buf())` with `.contains_key` to avoid allocating and cloning strings on every node lookup. * Updates `add_edge` to use `.get_mut` instead of `.entry` to skip `edge.from.clone()` and `edge.to.clone()` when adjacency lists already exist. * This dramatically reduces memory churn when adding elements to the dependency graph by making allocations O(V) instead of O(E). 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 dependency graph construction to avoid unnecessary hash map entry allocations and minorly simplifies lifetimes on rule-engine helpers, reducing allocation churn and improving performance during graph building. Flow diagram for optimized add_edge adjacency updatesflowchart TD
A["add_edge(edge)"] --> B["compute idx for new edge"]
B --> C["ensure_node(edge.from)"]
C --> D["ensure_node(edge.to)"]
D --> E{"forward_adj has key edge.from?"}
E --> F["get_mut(&edge.from).push(idx)"]:::fast
E --> G["insert(edge.from.clone(), vec![idx])"]:::alloc
F --> H{"reverse_adj has key edge.to?"}
G --> H
H --> I["get_mut(&edge.to).push(idx)"]:::fast
H --> J["insert(edge.to.clone(), vec![idx])"]:::alloc
I --> K["edges.push(edge)"]
J --> K
classDef fast fill:#bbf,stroke:#333,stroke-width:1px
classDef alloc fill:#fdd,stroke:#333,stroke-width:1px
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 targets allocation/performance hot paths by avoiding unconditional PathBuf cloning during dependency graph construction, and includes a small Rust signature cleanup in the rule engine.
Changes:
- Reworked
DependencyGraph::add_edgeadjacency updates to clonePathBufs only on first insertion into adjacency maps. - Reworked
DependencyGraph::ensure_nodeto avoid allocating aPathBufwhen the node already exists. - Removed unnecessary explicit lifetimes from internal helper functions in
check_var.rs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/rule-engine/src/check_var.rs | Simplifies internal helper signatures by removing unused lifetime parameters. |
| crates/flow/src/incremental/graph.rs | Reduces allocation churn by avoiding unconditional key cloning when populating nodes and adjacency maps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
💡 What
Replaced expensive
.entry().or_insert()and.entry().or_default()uses incrates/flow/src/incremental/graph.rswith.contains_key()and.get_mut()checks to eliminate redundantPathBufstring clones.🎯 Why
During graph construction (
ensure_nodeandadd_edge),entry(key.clone())unconditionally allocates and clones strings even if the key is already present in the HashMap. In large codebases with heavily connected dependency graphs, this results in significant O(E) memory churn.📊 Impact
Changes allocations from O(E) to O(V). It greatly reduces memory allocation pressure and improves speed during full-graph dependency updates and analysis cycles.
🔬 Measurement
Run
cargo bench -p thread-flow --bench graph_benchor profile the application graph builder. Graph insertion times should be tangibly lower andjemallocshould report significantly reduced allocation volume insideDependencyGraph::add_edge.PR created automatically by Jules for task 8275981448862509516 started by @bashandbone
Summary by Sourcery
Optimize graph construction performance and simplify rule-engine helper signatures.
Enhancements: