⚡ Bolt: Cache ColorMap to prevent repeated map allocations#78
⚡ Bolt: Cache ColorMap to prevent repeated map allocations#78
Conversation
In performance-sensitive areas with highly concurrent paths (e.g. statusline rendering with plugins executed in parallel), repeatedly allocating new large maps (like a color lookup map via colors.ColorMap()) causes measurable allocation overhead and garbage collection pressure. This commit caches the static color data map at the instance level in the StatusLine struct. Using sync.Once for lazy initialization ensures thread-safety without regressions for test suites. Co-authored-by: himattm <6266621+himattm@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. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements caching for the color map within the StatusLine struct to improve performance by reducing repeated allocations during statusline rendering. It introduces a getColorMap method that utilizes sync.Once for thread-safe lazy initialization. The review feedback suggests removing the redundant initialization of the color map in the New constructor to simplify the code and ensure consistent use of the lazy-loading getter.
| pluginManager: plugin.NewManager(), | ||
| nativePlugins: plugins.NewRegistry(), | ||
| isIdle: checkIsIdle(input.SessionID), | ||
| colorMap: colors.ColorMap(), |
There was a problem hiding this comment.
The initialization of colorMap in the New constructor is redundant because getColorMap handles lazy initialization via sync.Once. Removing this call simplifies the constructor and ensures consistent initialization through the getter.
| colorMap: colors.ColorMap(), | |
| colorMap: nil, |
|
Closing as superseded by direct commit 42f3149 on main, which consolidated the StatusLine color-map caching optimization without the generated .jules note. |
Understood. Acknowledging that this work is now superseded by the direct commit on main and stopping work on this task. |
1 participant
💡 What: Caches the return value of
colors.ColorMap()at theStatusLineinstance level rather than repeatedly invoking it on every plugin execution.🎯 Why:
runPlugin()is invoked in parallel goroutines. Repeatedly allocating a large 100+ item map causes unnecessary memory allocation and garbage collection pressure in a performance-critical path.📊 Impact: Reduces heap allocations during status line rendering by one map allocation per configured section, speeding up rendering by reducing GC pressure.
🔬 Measurement: Run benchmark rendering, observe reduced allocs/op using Go benchmarks. Tested explicitly with the race detector (
go test -race ./...) to ensure thread-safe caching.PR created automatically by Jules for task 15580878599962567874 started by @himattm