Skip to content

perf(octicons): cache embedded data URIs on demand#2851

Open
dexhunter wants to merge 2 commits into
github:mainfrom
dexhunter:dex/cache-octicon-data-uris
Open

perf(octicons): cache embedded data URIs on demand#2851
dexhunter wants to merge 2 commits into
github:mainfrom
dexhunter:dex/cache-octicon-data-uris

Conversation

@dexhunter

@dexhunter dexhunter commented Jul 10, 2026

Copy link
Copy Markdown

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

  • Add a race-safe, on-demand cache for successful (name, theme) DataURI reads.
  • Preserve lookup for every embedded PNG, including files not listed in required_icons.txt.
  • Add missing-lookup, all-embedded-file, mutation-isolation, and concurrent-first-use tests.
  • Add reproducible cold and warm benchmarks for one-icon and default inventories.

Performance

Inventory State Main B/op This PR B/op Main allocs/op This PR allocs/op
31 icons first registration 240,464 247,080 534 538
31 icons warm registration 193,085 4,464 527 31
1 icon first registration 53,592 60,208 23 27
1 icon warm registration 4,160 144 17 1

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

  • No tool or API changes
  • Tool schema or behavior changed
  • New tool added

Prompts tested (tool changes only)

  • Not applicable; this does not change tool schemas or behavior.

Security / limits

  • No security or limits impact
  • Auth / permissions considered
  • Data exposure, filtering, or token/size limits considered

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

  • I am renaming tools as part of this PR
  • I am not renaming tools as part of this PR

Lint & tests

  • Linted locally with ./script/lint
  • Tested locally with ./script/test
  • go test -race ./...
  • Strict cold/warm evaluator with concurrent-first-use race coverage
  • git diff --check

Docs

  • Not needed
  • Updated (README / docs / examples)

@dexhunter dexhunter requested a review from a team as a code owner July 10, 2026 15:11
Copilot AI review requested due to automatic review settings July 10, 2026 15:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread pkg/octicons/octicons_benchmark_test.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cache embedded Octicon data URIs across repeated registrations

2 participants