From cfded4d559f8039866f101d1ddb086088c41b3ee Mon Sep 17 00:00:00 2001 From: Samuel Berthe Date: Sun, 5 Jul 2026 17:12:56 +0200 Subject: [PATCH] perf(kv): skip merge of absent context/user/tenant maps in snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mergeMaps allocates a fresh map even for empty input, so every snapshot() paid up to three map allocations for fields the error chain never set. Consumers only ever check len() on these fields, so leaving them nil is equivalent; ToMap's in-place "id" insertion now guards against the nil case explicitly. benchstat (darwin/arm64, Apple M3, -count=6 -cpu=1): before after ToMap 5.120Ki ± 0% 5.073Ki ± 0% -0.92% (p=0.002 n=6) LogValue 5.264Ki ± 0% 5.217Ki ± 0% -0.89% (p=0.002 n=6) ToMap 62.00 allocs 61.00 allocs -1.61% (p=0.002 n=6) LogValue 60.00 allocs 59.00 allocs -1.67% (p=0.002 n=6) Wall-clock deltas were not statistically significant on this run (background noise on the benchmark machine), but the B/op and allocs/op reductions are fully reproducible (0% variance, p=0.002). --- error.go | 12 ++++++++++-- kv.go | 14 +++++++++++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/error.go b/error.go index 9037b88..f5f602d 100644 --- a/error.go +++ b/error.go @@ -744,8 +744,12 @@ func (o OopsError) ToMap() map[string]any { //nolint:gocyclo if s.userID != "" || len(s.userData) > 0 { // snapshot() owns s.userData (freshly merged in mergeMaps), so it can - // be used directly instead of copying. + // be used directly instead of copying. It is nil when only the user + // ID was set. user := s.userData + if user == nil { + user = map[string]any{} + } if s.userID != "" { user["id"] = s.userID } @@ -754,8 +758,12 @@ func (o OopsError) ToMap() map[string]any { //nolint:gocyclo if s.tenantID != "" || len(s.tenantData) > 0 { // snapshot() owns s.tenantData (freshly merged in mergeMaps), so it can - // be used directly instead of copying. + // be used directly instead of copying. It is nil when only the tenant + // ID was set. tenant := s.tenantData + if tenant == nil { + tenant = map[string]any{} + } if s.tenantID != "" { tenant["id"] = s.tenantID } diff --git a/kv.go b/kv.go index a72810e..bd840a5 100644 --- a/kv.go +++ b/kv.go @@ -303,9 +303,17 @@ func snapshot(o OopsError) OopsError { //nolint:gocyclo } s.tags = lo.Uniq(s.tags) - s.context = dereferencePointers(lazyMapEvaluation(mergeMaps(contextMaps))) - s.userData = dereferencePointers(lazyMapEvaluation(mergeMaps(userDataMaps))) - s.tenantData = dereferencePointers(lazyMapEvaluation(mergeMaps(tenantDataMaps))) + // Only merge when the chain actually carries maps: mergeMaps allocates + // even for empty input, and consumers only ever check len() on these. + if len(contextMaps) > 0 { + s.context = dereferencePointers(lazyMapEvaluation(mergeMaps(contextMaps))) + } + if len(userDataMaps) > 0 { + s.userData = dereferencePointers(lazyMapEvaluation(mergeMaps(userDataMaps))) + } + if len(tenantDataMaps) > 0 { + s.tenantData = dereferencePointers(lazyMapEvaluation(mergeMaps(tenantDataMaps))) + } return s }