Skip to content

perf: fast-path common pointer types in dereferencePointers + inline tag dedup#125

Merged
samber merged 4 commits into
mainfrom
perf/deref-pointer-fast-path
May 28, 2026
Merged

perf: fast-path common pointer types in dereferencePointers + inline tag dedup#125
samber merged 4 commits into
mainfrom
perf/deref-pointer-fast-path

Conversation

@samber
Copy link
Copy Markdown
Owner

@samber samber commented May 28, 2026

Changes

  • kv.go: add type-switch cases for *string, *int, *int64, *uint, *uint64, *float64, *bool in dereferencePointers — avoids reflect.ValueOf allocation for the most common pointer types; everything else still falls through to the reflect path
  • error.go: inline tag deduplication in Tags() using a seen map during accumulation, removing the lo.Uniq post-pass
  • kv_test.go: add TestSnapshot with 13 sub-tests covering all merge semantics (scalar field precedence, context/userData/tenantData map merging, tag dedup, explicit vs auto trace, lazy evaluation, pointer dereferencing, zero snapshot)

…tag dedup

- Add type-switch cases for *string, *int, *int64, *uint, *uint64,
  *float64, *bool in dereferencePointers to avoid reflect.ValueOf
  for the most common pointer types
- Inline tag deduplication in Tags() using a seen map, removing the
  lo.Uniq pass after accumulation
- Add TestSnapshot covering all snapshot merge semantics (13 cases)
Copilot AI review requested due to automatic review settings May 28, 2026 14:44
@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.17%. Comparing base (b7740ed) to head (524315a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #125      +/-   ##
==========================================
+ Coverage   89.49%   90.17%   +0.67%     
==========================================
  Files          15       15              
  Lines        1209     1231      +22     
==========================================
+ Hits         1082     1110      +28     
+ Misses        102       96       -6     
  Partials       25       25              
Flag Coverage Δ
unittests 90.17% <100.00%> (+0.67%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 targets lower allocations and faster hot paths in the error metadata pipeline by optimizing pointer dereferencing in KV contexts and tag deduplication, and it adds snapshot-focused tests to validate chain merge semantics.

Changes:

  • Add fast-path type-switch cases in dereferencePointers for common pointer types to avoid the reflect-based path.
  • Inline tag deduplication in OopsError.Tags() during traversal (removing the lo.Uniq post-pass).
  • Add TestSnapshot with subtests covering scalar precedence, map merges, tag dedup, trace precedence, lazy evaluation, pointer deref, and zero snapshots.

Reviewed changes

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

File Description
kv.go Adds fast-path pointer deref cases before falling back to reflect-based recursion.
error.go Reworks Tags() to deduplicate during accumulation with a seen map.
kv_test.go Adds TestSnapshot to exercise snapshot merge semantics and related behaviors.

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

Comment thread error.go
Comment thread kv.go Outdated
@samber samber merged commit 37b62d0 into main May 28, 2026
10 checks passed
@samber samber deleted the perf/deref-pointer-fast-path branch May 28, 2026 14:59
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.

2 participants