⚡ Bolt: [performance improvement] Eliminate per-frame Map allocation in KnowledgeGraph - #188
servathadi wants to merge 2 commits into
Conversation
Co-authored-by: servathadi <116760885+servathadi@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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 942ab4be77
ℹ️ 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".
| source: nodeMap.get(edge.source)!, | ||
| target: nodeMap.get(edge.target)! | ||
| })).filter(e => e.source && e.target) | ||
| }, [initialNodes, edges, dimensions]) |
There was a problem hiding this comment.
Separate edge refreshes from node initialization
When a parent supplies a new edges array while retaining the same nodes, adding edges to this initialization effect reruns the random placement and zeroes every node's velocity. Previously an edge-only update changed the rendered links without discarding the settled layout, so dynamic graph updates now cause a visible jump and restart the simulation; resolve the edges in a separate effect against nodesRef.current instead.
Useful? React with 👍 / 👎.
|
⚖️ Loom PR Gate Intake: Received by Loom @ 2026-08-26T03:56:37Z. Coordinating Athena & Codex gate review. |
Co-authored-by: servathadi <116760885+servathadi@users.noreply.github.com>
💡 What: Pre-computes edge references (mapping string slugs to
GraphNodereferences) in auseEffectinstead of dynamically resolving them per frame in therequestAnimationFrameloop.🎯 Why: Instantiating
new Map()and performing O(E) string lookups inside a 60fpsrequestAnimationFrameloop caused unnecessary CPU usage and garbage collection (GC) pressure.📊 Impact: Eliminates 60 Map instantiations and hundreds of string lookups per second, reducing GC pauses and keeping the main thread free, ensuring a smoother 60fps render.
🔬 Measurement: Observe memory allocations and frame rate in Chrome DevTools Performance tab while viewing the
/exploreroute.PR created automatically by Jules for task 1422548245672864626 started by @servathadi