perf: single-pass chain traversal in LogValue/ToMap/formatVerbose - #124
Conversation
Add snapshot(OopsError) OopsError that walks the error chain once, collecting and merging all attributes (scalars, tags, maps). The three hot-path rendering methods — LogValue, ToMap, formatVerbose — call snapshot() at entry and read fields directly, reducing chain traversals from O(n×h) to O(h) per render call. Also extract mergeMaps() from mergeNestedErrorMap() so the merge logic is shared between the existing public accessors and snapshot(). Results on a 1-level chain: -46% bytes/op, -14 allocs/op on ToMap and LogValue. Gains scale with chain depth.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #124 +/- ##
==========================================
+ Coverage 89.30% 89.49% +0.19%
==========================================
Files 15 15
Lines 1159 1209 +50
==========================================
+ Hits 1035 1082 +47
- Misses 100 102 +2
- Partials 24 25 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Refactors LogValue, ToMap, and formatVerbose to traverse the OopsError chain a single time via a new package-private snapshot() helper, replacing ~13 independent O(h) accessor calls per render with one O(h) walk. Also extracts mergeMaps() from mergeNestedErrorMap() so the snapshot can reuse the merge logic without re-walking the chain.
Changes:
- Add
snapshot(OopsError) OopsErrorinerror.gothat resolves all chain-derived fields (deepest-non-zero scalars, merged/lazy/dereferenced maps, deduped tags, explicit-vs-auto trace) in a singlerecursivepass. - Rewrite
LogValue,ToMap, andformatVerboseto callsnapshotonce and read fields directly from the resolved struct. - Extract
mergeMaps([]map[string]any)frommergeNestedErrorMapinkv.goso it can be shared withsnapshot.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
error.go |
Adds snapshot(); rewrites LogValue, ToMap, formatVerbose to use the single-pass result. |
kv.go |
Extracts mergeMaps() from mergeNestedErrorMap() for reuse by snapshot(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
LogValue(),ToMap(), andformatVerbose()each called ~13 independent accessors (Code(),Time(),Domain(),Tags(),Trace(),Hint(),Public(),Owner(),Context(),User(),Tenant(),request(),response()), each traversing the full error chain — O(n×h) traversals per render callsnapshot(OopsError) OopsError(package-private function) that walks the chain once, collecting scalars with deepest-non-zero semantics, merging maps, deduplicating tags, and resolving explicit-vs-auto trace prioritysnapshot()at entry and read fields directly — O(h) instead of O(n×h)mergeMaps()frommergeNestedErrorMap()so the merge logic is sharedBenchmark results (1-level chain, Apple M3)
BenchmarkToMapallocs/opBenchmarkToMapB/opBenchmarkLogValueallocs/opBenchmarkLogValueB/opBenchmarkMarshalJSONallocs/opBenchmarkMarshalJSONB/opGains scale with chain depth.