⚡ Bolt: Cache color map to reduce allocations during plugin execution#80
⚡ Bolt: Cache color map to reduce allocations during plugin execution#80
Conversation
In performance-sensitive areas with parallel execution like `statusline.go`'s `runPlugin`, dynamically calling `colors.ColorMap()` allocates a new, relatively large map for every plugin invocation. Repeatedly allocating maps inside hot loops or goroutines creates unnecessary memory churn and GC pressure. This PR caches the static read-only map at the instance level (using `sync.Once` inside the struct) so it is only allocated once per rendering cycle, rather than re-created inside functions that execute concurrently across multiple routines. 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 introduces instance-level caching for the color map in the StatusLine struct to reduce memory allocations during parallel plugin execution. Feedback suggests adding a warning about the thread-safety of the shared map and recommends using package-level caching for static data to further optimize resource usage.
| } | ||
|
|
||
| // getColorMap returns the color map, creating it once if needed | ||
| func (sl *StatusLine) getColorMap() map[string]string { |
There was a problem hiding this comment.
Sharing a single map instance across multiple plugins executed in parallel introduces a concurrency risk. In Go, maps are not thread-safe for concurrent operations if at least one of them is a write. While plugins are generally expected to treat the Colors map as read-only, a native plugin that accidentally modifies the map (e.g., to add a temporary color) will cause a fatal panic due to concurrent map access. Adding a warning comment helps clarify the expected usage for future native plugin developers.
| func (sl *StatusLine) getColorMap() map[string]string { | |
| // getColorMap returns the color map, creating it once if needed. | |
| // WARNING: The returned map is shared across all plugins and must be treated as read-only | |
| // to avoid concurrent map access panics during parallel execution. | |
| func (sl *StatusLine) getColorMap() map[string]string { |
| colorMap map[string]string // Cached color map | ||
| colorMapOnce sync.Once |
There was a problem hiding this comment.
Since the color map is derived from static constants and does not depend on any instance-specific state, it is more efficient to cache it at the package level rather than the instance level. This ensures the map is allocated exactly once for the lifetime of the process, regardless of how many StatusLine instances are created. This is particularly relevant if StatusLine is instantiated frequently (e.g., once per render cycle).
|
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 PR has been superseded and stopping work on this task. |
1 participant
💡 What
Cached the return value of
colors.ColorMap()on theStatusLinestruct usingsync.Onceinstead of calling it dynamically for every plugin executed.🎯 Why
In performance-sensitive areas with parallel execution like
statusline.go'srunPlugin, dynamically callingcolors.ColorMap()allocates a new, relatively large map for every plugin invocation. Because plugins are run in parallel across goroutines, repeatedly allocating these maps creates unnecessary memory churn and garbage collection (GC) pressure.📊 Impact
Eliminates N memory allocations per statusline render (where N is the number of active plugins). Reduces GC pressure and improves overall rendering latency by caching the reference type once per instance.
🔬 Measurement
Run a benchmark or profile
go test -bench . -benchmemto observe the reduced number of allocations (allocs/op) and memory (B/op) during the statusline rendering phase when multiple plugins are enabled.PR created automatically by Jules for task 12924269374588308592 started by @himattm