perf(octicons): cache embedded data URIs on demand - #2851
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
This PR optimizes the pkg/octicons package by adding an on-demand, race-safe cache for successfully read embedded Octicon PNG data URIs, reducing repeated base64 encoding work during repeated tool/server registrations while preserving existing behavior for misses and returning fresh icon slices.
Changes:
- Introduced an RWMutex-protected (name, theme) -> data URI cache for successful embedded icon reads, leaving missing lookups uncached.
- Updated DataURI to use the cache while keeping the underlying embedded read logic in a dedicated helper.
- Added tests for all embedded icons, cache semantics (misses not cached), mutation isolation, and concurrent first-use; added cold/warm benchmarks.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/octicons/octicons.go | Adds a race-safe on-demand cache and routes DataURI through it. |
| pkg/octicons/octicons_test.go | Adds test coverage for cache behavior, embedded inventory coverage, concurrency, and slice mutation isolation. |
| pkg/octicons/octicons_benchmark_test.go | Adds benchmarks for cold/warm cache behavior and icon registration allocation profiles. |
Sorry, something went wrong.
|
Superseded by #2895, which builds on this performance improvement by moving the base64 work to icon generation time. Thanks @dexhunter for identifying the repeated encoding overhead and proposing the caching improvement. |
Sorry, something went wrong.
Summary
Cache successful embedded Octicon data-URI reads on first use so repeated stateless server and tool registration no longer rereads and base64-encodes the same PNGs. Missing lookups remain uncached, and every Icons call still returns a fresh mutable slice.
Why
Fixes #2850
Repeated all-tool registration currently allocates a median 193,085 B/op and 527 allocs/op for the 31 required icons after warmup. The same process may rebuild this inventory for multiple stateless HTTP requests.
What changed
Performance
For the default inventory, warm allocation bytes fall by 97.69% and allocation count falls by 94.12%. The disclosed cold cost is +6,616 B and +4 allocations while the cache is populated. No icon is read or encoded in init().
Supplementary autoresearch: 20-step public dashboard.
MCP impact
Prompts tested (tool changes only)
Security / limits
Only embedded PNG data is cached. Missing lookups are not retained. With all current variants used, retained data-URI payload is 46,840 bytes plus bounded map overhead.
Tool renaming
Lint & tests
Docs