Uh oh!
There was an error while loading. Please reload this page.
perf(octicons): cache embedded data URIs on demand - #2851
Conversation
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 URIcache for successful embedded icon reads, leaving missing lookups uncached. - Updated
DataURIto 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. |
Uh oh!
There was an error while loading. Please reload this page.
36e7619 to
402e9b3CompareSamMorrowDrums
commented
Jul 16, 2026
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. |
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
Iconscall still returns a fresh mutable slice.Why
Fixes#2850
Repeated all-tool registration currently allocates a median
193,085 B/opand527 allocs/opfor the 31 required icons after warmup. The same process may rebuild this inventory for multiple stateless HTTP requests.What changed
(name, theme)DataURIreads.required_icons.txt.Performance
For the default inventory, warm allocation bytes fall by
97.69%and allocation count falls by94.12%. The disclosed cold cost is+6,616 Band+4 allocationswhile the cache is populated. No icon is read or encoded ininit().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,840bytes plus bounded map overhead.Tool renaming
Lint & tests
./script/lint./script/testgo test -race ./...git diff --checkDocs