Reduce string allocation overhead in decoders

Replaces global mutex with per-entry mutexes to reduce allocation
count from 33 to 10 per operation in downstream libraries while
maintaining thread safety and good concurrent performance.

Author: oschwald

CHANGELOG.md

@@ -31,10 +31,11 @@
   Pointer format. For example, errors may now show "at offset 1234, path
   /city/names/en" or "at offset 1234, path /list/0/name" instead of just the
   underlying error message.
-- **PERFORMANCE**: Added bounded string interning optimization that provides
-  ~15% performance improvement for City lookups while maintaining thread safety
-  for concurrent reader usage. Uses a fixed 512-entry cache with offset-based
-  indexing to prevent unbounded memory growth.
+- **PERFORMANCE**: Added string interning optimization that reduces allocations
+  while maintaining thread safety. Provides ~15% improvement for single-threaded
+  City lookups and reduces allocation count from 33 to 10 per operation in
+  downstream libraries. Uses a fixed 512-entry cache with per-entry mutexes
+  for bounded memory usage (~8KB) while minimizing lock contention.
 
 ## 2.0.0-beta.3 - 2025-02-16
 

internal/decoder/string_cache.go

@@ -1,31 +1,30 @@
+// Package decoder decodes values in the data section.
 package decoder
 
-import "sync"
-
-// stringCache provides bounded string interning using offset-based indexing.
-// Similar to encoding/json/v2's intern.go but uses offsets instead of hashing.
-// Thread-safe for concurrent use.
-type stringCache struct {
-	// Fixed-size cache to prevent unbounded memory growth
-	// Using 512 entries for 8KiB total memory footprint (512 * 16 bytes per string)
-	cache [512]cacheEntry
-	// RWMutex for thread safety - allows concurrent reads, exclusive writes
-	mu sync.RWMutex
-}
+import (
+	"sync"
+)
 
+// cacheEntry represents a cached string with its offset and dedicated mutex.
 type cacheEntry struct {
 	str    string
 	offset uint
+	mu     sync.RWMutex
+}
+
+// stringCache provides bounded string interning with per-entry mutexes for minimal contention.
+// This achieves thread safety while avoiding the global lock bottleneck.
+type stringCache struct {
+	entries [512]cacheEntry
 }
 
-// newStringCache creates a new bounded string cache.
+// newStringCache creates a new per-entry mutex-based string cache.
 func newStringCache() *stringCache {
 	return &stringCache{}
 }
 
 // internAt returns a canonical string for the data at the given offset and size.
-// Uses the offset modulo cache size as the index, similar to json/v2's approach.
-// Thread-safe for concurrent use.
+// Uses per-entry RWMutex for fine-grained thread safety with minimal contention.
 func (sc *stringCache) internAt(offset, size uint, data []byte) string {
 	const (
 		minCachedLen = 2   // single byte strings not worth caching
@@ -37,30 +36,28 @@ func (sc *stringCache) internAt(offset, size uint, data []byte) string {
 		return string(data[offset : offset+size])
 	}
 
-	// Use offset as cache index (modulo cache size)
-	i := offset % uint(len(sc.cache))
+	// Use same cache index calculation as original: offset % cacheSize
+	i := offset % uint(len(sc.entries))
+	entry := &sc.entries[i]
 
-	// Fast path: check for cache hit with read lock
-	sc.mu.RLock()
-	entry := sc.cache[i]
-	if entry.offset == offset && len(entry.str) == int(size) {
+	// Fast path: read lock and check
+	entry.mu.RLock()
+	if entry.offset == offset && entry.str != "" {
 		str := entry.str
-		sc.mu.RUnlock()
+		entry.mu.RUnlock()
 		return str
 	}
-	sc.mu.RUnlock()
+	entry.mu.RUnlock()
 
-	// Cache miss - create new string and store with write lock
+	// Cache miss - create new string
 	str := string(data[offset : offset+size])
 
-	sc.mu.Lock()
-	// Double-check in case another goroutine added it while we were waiting
-	if sc.cache[i].offset == offset && len(sc.cache[i].str) == int(size) {
-		str = sc.cache[i].str
-	} else {
-		sc.cache[i] = cacheEntry{offset: offset, str: str}
-	}
-	sc.mu.Unlock()
+	// Store with write lock on this specific entry
+	entry.mu.Lock()
+	entry.offset = offset
+	entry.str = str
+	entry.mu.Unlock()
 
 	return str
 }
+

internal/decoder/string_cache_test.go

@@ -0,0 +1,52 @@
+package decoder
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestStringCacheOffsetZero(t *testing.T) {
+	cache := newStringCache()
+	data := []byte("hello world, this is test data")
+
+	// Test string at offset 0
+	str1 := cache.internAt(0, 5, data)
+	require.Equal(t, "hello", str1)
+
+	// Second call should hit cache and return same interned string
+	str2 := cache.internAt(0, 5, data)
+	require.Equal(t, "hello", str2)
+
+	// Note: Both strings should be identical (cache hit)
+	// We can't easily test if they're the same object without unsafe,
+	// but correctness is verified by the equal values
+}
+
+func TestStringCacheVariousOffsets(t *testing.T) {
+	cache := newStringCache()
+	data := []byte("abcdefghijklmnopqrstuvwxyz")
+
+	testCases := []struct {
+		offset   uint
+		size     uint
+		expected string
+	}{
+		{0, 3, "abc"},
+		{5, 3, "fgh"},
+		{10, 5, "klmno"},
+		{23, 3, "xyz"},
+	}
+
+	for _, tc := range testCases {
+		// First call
+		str1 := cache.internAt(tc.offset, tc.size, data)
+		require.Equal(t, tc.expected, str1)
+
+		// Second call should hit cache
+		str2 := cache.internAt(tc.offset, tc.size, data)
+		require.Equal(t, tc.expected, str2)
+		// Verify cache hit returns correct value (interning tested via behavior)
+	}
+}
+