Add render cache for SVG icons

[silverwind]

Cache the final rendered template.HTML output for SVG icons that use non-default size or class parameters using sync.Map. Icons rendered with default parameters bypass the cache and use a direct read-only map lookup.

Benchmark results for rendering 1000 varied SVG icons under high concurrency (16 goroutines):

	Per page (1000 SVGs)	Allocs	Memory
Uncached	0.356ms	8,014	1.17MB
Cached	0.015ms	1,000	16KB

[Copilot]

Pull request overview

Adds an in-memory render cache for SVG icon HTML in modules/svg, targeting reduced allocations/latency when the same icon is rendered repeatedly with non-default size/class under concurrency.

Changes:

• Introduces a global sync.Map cache for rendered template.HTML for (icon,size,class) combinations.
• Bypasses the cache for default parameters to keep the fast path as a direct map lookup.
• Clears the render cache during svg.Init() to avoid stale entries across re-initialization.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[silverwind]

Removed all cache clears. The code paths install/non-install page never happen in the same run, so we never need to clear this cache.

[wxiaoguang]

Mutex will make it slower, right? Is it really justified?

Have you benchmarked?

Yes, IIRC, with mutex it was 10 times slower using concurrency of 16. But that was on a page with 1000 varied SVGs, so not really representative.

What kind of Mutex did you use? I don't think RWMutex can be the problem.

[wxiaoguang]

Actually I guess no one would feel the "cache".

I think big diff pages ought to have that many SVGs. It's at minimum 5 SVGs per file, so a 200 file diff would have 1000+ SVGs. 300ms is probably still miniscule compared to full render time but not completely zero.

Why 300ms? If I understand correctly, your tests mean that "1000 SVGs take 0.356ms", right?

[silverwind]

Right, microseconds, not milliseconds :)

[wxiaoguang]

I beg you to understand the problem and use facts to discuss.

[silverwind]

I will benchmark mutex vs. no-mutex in a bit. Scenario will be concurrency 16, 200 SVGs with 50 variations. That should give representative results.

[silverwind]

Written by Claude.

Benchmark results (Apple M3 Max, go test -bench=. -benchmem -count=5, 200 icons, 50 variations, GOMAXPROCS=16, b.SetParallelism(16)):

BenchmarkRenderHTML_CachedNoMutex-16       435403636           2.912 ns/op          0 B/op          0 allocs/op
BenchmarkRenderHTML_Cached-16                8632720         130.5   ns/op          0 B/op          0 allocs/op
BenchmarkRenderHTML_Uncached-16              5451798         226.2   ns/op          5 B/op          0 allocs/op
BenchmarkRenderHTML_DefaultSize-16        1000000000           0.831 ns/op          0 B/op          0 allocs/op

• CachedNoMutex: same code path as Cached but without RWMutex (pre-warmed read-only cache)
• Cached: full renderHTML path with RWMutex cache hit
• Uncached: cache miss every time (unique keys), string manipulation + mutex write
• DefaultSize: fast path (default size, no extra class, no cache/mutex involved)

RWMutex + function call overhead adds ~128ns at 16 goroutines. The cache is still a net win (~131 vs ~225 ns), but the mutex is clearly the bottleneck in the cached path.

Benchmark code (svg_test.go)

package svg

import (
	"fmt"
	"html/template"
	"testing"

	gitea_html "code.gitea.io/gitea/modules/htmlutil"
)

type benchmarkCall struct {
	icon  string
	size  int
	class string
}

func setupBenchmark(numIcons, numVariations int) []benchmarkCall {
	svgIcons = make(map[string]svgIconItem, numIcons)
	for i := range numIcons {
		name := fmt.Sprintf("icon-%d", i)
		svgIcons[name] = svgIconItem{
			html: fmt.Sprintf(`<svg class="svg %s" width="16" height="16"><path d="M0 0h16v16H0z"/></svg>`, name),
		}
	}
	svgRenderedCache = make(map[svgCacheKey]template.HTML)
	calls := make([]benchmarkCall, numIcons*numVariations)
	for i := range numIcons {
		for v := range numVariations {
			calls[i*numVariations+v] = benchmarkCall{
				icon:  fmt.Sprintf("icon-%d", i),
				size:  16 + v,
				class: "extra-class",
			}
		}
	}
	return calls
}

// BenchmarkRenderHTML_CachedNoMutex benchmarks the same path as Cached but without RWMutex.
func BenchmarkRenderHTML_CachedNoMutex(b *testing.B) {
	calls := setupBenchmark(200, 50)
	for _, c := range calls {
		RenderHTML(c.icon, c.size, c.class)
	}
	numCalls := len(calls)
	b.SetParallelism(16)
	b.ResetTimer()
	b.RunParallel(func(pb *testing.PB) {
		i := 0
		for pb.Next() {
			c := &calls[i%numCalls]
			size, class := gitea_html.ParseSizeAndClass(defaultSize, "", c.size, c.class)
			if svgItem, ok := svgIcons[c.icon]; ok {
				if size != defaultSize || class != "" {
					cacheKey := svgCacheKey{c.icon, size, class}
					if cachedHTML, cached := svgRenderedCache[cacheKey]; cached && !svgItem.mocking {
						_ = cachedHTML
					}
				}
			}
			i++
		}
	})
}

// BenchmarkRenderHTML_Cached benchmarks the full renderHTML with cache hits.
func BenchmarkRenderHTML_Cached(b *testing.B) {
	calls := setupBenchmark(200, 50)
	for _, c := range calls {
		RenderHTML(c.icon, c.size, c.class)
	}
	numCalls := len(calls)
	b.SetParallelism(16)
	b.ResetTimer()
	b.RunParallel(func(pb *testing.PB) {
		i := 0
		for pb.Next() {
			c := &calls[i%numCalls]
			RenderHTML(c.icon, c.size, c.class)
			i++
		}
	})
}

// BenchmarkRenderHTML_Uncached benchmarks renderHTML without cache hits (unique keys every time).
func BenchmarkRenderHTML_Uncached(b *testing.B) {
	calls := setupBenchmark(200, 50)
	numCalls := len(calls)
	b.SetParallelism(16)
	b.ResetTimer()
	b.RunParallel(func(pb *testing.PB) {
		i := 0
		for pb.Next() {
			c := &calls[i%numCalls]
			RenderHTML(c.icon, 1000+i, c.class)
			i++
		}
	})
}

// BenchmarkRenderHTML_DefaultSize benchmarks the fast path (default size, no extra class, no cache).
func BenchmarkRenderHTML_DefaultSize(b *testing.B) {
	calls := setupBenchmark(200, 50)
	numCalls := len(calls)
	b.SetParallelism(16)
	b.ResetTimer()
	b.RunParallel(func(pb *testing.PB) {
		i := 0
		for pb.Next() {
			RenderHTML(calls[i%numCalls].icon)
			i++
		}
	})
}

[silverwind]

Mutex overhead seems a bit much, not sure though.

[wxiaoguang]

RWMutex.RLock atomic.Add invalidates CPU cache for all cores.

If you need the "limit", the RWMutex is still needed. Otherwise, sync.Map can't count the items.

If you need to make sync.Map have item count, then it needs another "atomic", then the performance degrades to the same level as RWMutex.RLock

---

OK, maybe can use count with a write-only mutex.

[wxiaoguang]

c6884dd

BenchmarkRenderHTML_CachedNoMutex
BenchmarkRenderHTML_CachedNoMutex-12    	166848481	         6.809 ns/op
BenchmarkRenderHTML_Cached
BenchmarkRenderHTML_Cached-12           	168768458	         6.907 ns/op
BenchmarkRenderHTML_Uncached
BenchmarkRenderHTML_Uncached-12         	109087372	        15.36 ns/op
BenchmarkRenderHTML_DefaultSize
BenchmarkRenderHTML_DefaultSize-12      	996117556	         1.219 ns/op

[silverwind]

lgtm

[silverwind]

Written by Claude.

The ~20x speedup (130 ns → 6.9 ns) comes from eliminating CPU cache line invalidation on the read path:

• RWMutex.RLock() does an atomic.Add (a write) to increment the reader count. This invalidates the cache line across all cores, causing contention when many goroutines read concurrently.
• sync.Map.Load() uses only atomic loads (reads) on its internal read-only map — no writes to shared memory, so no cross-core cache line bouncing.

sync.Map is designed exactly for this pattern: frequent reads, rare writes. Its read map is effectively lock-free for lookups, making cached performance nearly identical to the no-mutex baseline.

[wxiaoguang]

OK, finally improved the render time of a page with 1000 SVGs about 300us (0.3ms), for most normal pages, less than 0.1ms