-
Notifications
You must be signed in to change notification settings - Fork 4
β‘ Bolt: Cache colors.ColorMap() in StatusLine to avoid repeated allocations
#74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ## 2024-04-29 - Cache Large Map Allocations at Instance Level | ||
| **Learning:** Returning a large `map[string]string` from a function like `colors.ColorMap()` on every plugin invocation creates significant repeated memory allocation overhead, particularly in parallel rendering operations like `StatusLine.Render()` which executes 5-10 times. | ||
| **Action:** Always cache large reference types (like maps) at the struct instance level (e.g. within `StatusLine`) during initialization to avoid redundant allocations in hot loops. Add fallback initialization `if sl.colorMap == nil` for backward compatibility with manual struct initialization in tests. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -35,6 +35,7 @@ type StatusLine struct { | |||||||||||||||||
| isIdle bool | ||||||||||||||||||
| bashPlugins []plugin.Plugin // Cached discovered bash plugins | ||||||||||||||||||
| bashPluginsOnce sync.Once | ||||||||||||||||||
| colorMap map[string]string // Cached color map to avoid allocations | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // New creates a new StatusLine renderer | ||||||||||||||||||
|
|
@@ -45,6 +46,7 @@ func New(input Input, cfg config.Config) *StatusLine { | |||||||||||||||||
| pluginManager: plugin.NewManager(), | ||||||||||||||||||
| nativePlugins: plugins.NewRegistry(), | ||||||||||||||||||
| isIdle: checkIsIdle(input.SessionID), | ||||||||||||||||||
| colorMap: colors.ColorMap(), | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -612,6 +614,11 @@ func (sl *StatusLine) getConfigBool(key string, defVal bool) bool { | |||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| func (sl *StatusLine) runPlugin(name string) string { | ||||||||||||||||||
| // Initialize colorMap if nil (for tests that construct StatusLine manually) | ||||||||||||||||||
| if sl.colorMap == nil { | ||||||||||||||||||
| sl.colorMap = colors.ColorMap() | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+618
to
+620
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This lazy initialization is subject to a data race. Since
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| // Build plugin input | ||||||||||||||||||
| input := plugin.Input{ | ||||||||||||||||||
| Prism: plugin.PrismContext{ | ||||||||||||||||||
|
|
@@ -634,7 +641,7 @@ func (sl *StatusLine) runPlugin(name string) string { | |||||||||||||||||
| ContextWindowSize: sl.input.Context.ContextWindow, | ||||||||||||||||||
| }, | ||||||||||||||||||
| Config: sl.getPluginConfig(name), | ||||||||||||||||||
| Colors: colors.ColorMap(), | ||||||||||||||||||
| Colors: sl.colorMap, | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By caching and reusing |
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Try native plugin first (much faster - no subprocess) | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To safely handle lazy initialization of
colorMapin a concurrent environment, add async.Oncefield to theStatusLinestruct. This is necessary becauserunPluginis called from multiple goroutines inrenderLine, and the current lazy initialization logic is not thread-safe.