⚡ perf: rewrite decode/encode hot paths - zero-alloc decoding, SWAR utils, cached metadata, panic-safety by gaby · Pull Request #76 · gofiber/schema

gaby · 2026-07-17T23:58:15Z

Summary

Profile-guided performance rewrite of the schema package built on the new SWAR utilities in gofiber/utils/v2 v2.2.0, hardened by iterative adversarial review (multi-agent finder/verifier rounds, CodeRabbit, Copilot, maintainer review) and an empirical differential harness that A/Bs every behavior against main. Along the way the reviews surfaced — and this PR fixes — a series of long-standing correctness bugs and caller-crashing panics.

Decoding a flat struct is now allocation-free, large-struct decode dropped from 62 to 4 allocs/op, the benchmark geomean improved ~−76%, and statement coverage stands at 99.4%. Behavior is byte-identical to main across a ~1,500-case differential battery, except where a main bug was deliberately fixed (each listed below).

Benchmark improvements

benchstat, count=5, linux/amd64, origin/main vs this branch:

Benchmark	Time (ns/op)	Δ time	Memory (B/op)	Allocs/op
SimpleStructDecode	3474 → 430	−87.6%	479 → 0	30 → 0
LargeStructDecode	7908 → 1225	−84.5%	1200 → 144	62 → 4
CheckRequiredFields	1481 → 251	−83.0%	1048 → 0	15 → 0
TimeDurationDecoding	906 → 296	−67.3%	184 → 16	8 → 2
Decode error path (`BenchmarkAll`)	35.2 → 9.8	−71.9%	16 → 0	1 → 0
SimpleStructEncode	1848 → 1100	−40.5%	552 → 464	5 → 3
LargeStructEncode	3028 → 1498	−50.5%	1062 → 872	12 → 6
ParseTag	49.9 → 6.2	−87.5%	32 → 0	1 → 0
geomean		−76.0%

Parallel decode (LargeStructDecodeParallel) improved from ~780 → ~460 ns/op with the lock-free caches. Decoding 200 indexed slice elements (items.0…items.199) dropped from 33.5 KB / 18 allocs to 12.8 KB / 7 allocs. Micro-benchmarks flagged by CI were re-verified with same-machine A/Bs: ConvertInt is at parity with main (7.79 vs 7.77 ns after inlining the native-fit guard), and the IsMultipartFile/IsZero deltas are binary-layout variance — those functions are byte-identical to main (and isMultipartField is now precomputed per field, off the decode hot path entirely).

What changed

Decoder

Cached encoding.TextUnmarshaler type facts per field (derefUnmarshaler / elemUnmarshaler) instead of re-deriving them with reflect.New + TypeAssert on every decode() call — this was 61% of all decode allocations. Instances are bound to live values only in the branches that need them.
Precomputed field index chains: structInfo records each field's index chain (through embedded structs, with per-promotion copies) while it walks the type, so decode() and setDefaults() walk v.Field(i) directly instead of repeating reflect FieldByName lookups (25% of decode CPU). Terminal slice-element parts are marked explicitly (pathPart.elem).
Direct builtin setters (setBuiltinKind): primitives decode via v.SetInt/SetString/… instead of boxing through reflect.ValueOf; a table-parity test keeps it in lockstep with builtinConverters. Native int/uint parsing uses utils.ParseInt/ParseUint (SWAR fast path) with an inline native-fit guard that compiles away on 64-bit and rejects overflow on 32-bit.
Zero-boxing slice decode (decodeBuiltinSlice): slices of builtin kinds parse directly into slice slots sized by a single comma-counting pass (one MakeSlice total), built detached to preserve all-or-nothing error behavior. A test pins the "no builtin kind parses commas" invariant the sizing relies on.
setDefaults early-out: skipped entirely when the struct tree has neither default: tags nor exported anonymous embedded pointers to allocate (23% of decode CPU on the common path).
checkRequired consumes the required-field map precomputed in structInfo (previously rebuilt recursively on every Decode), with search paths precomputed so per-request checks allocate nothing.
SWAR path parsing: nextPathSegment scans for . a word (8 bytes) at a time using swar.Load8/ZeroLanes/Broadcast/FirstLane from gofiber/utils/v2/swar.
strings.SplitSeq replaces allocating strings.Split for comma/| value splitting; sentinel errors replace per-call errors.New; lazy error-map allocation.

Encoder

Per-type encoding plans: alias, omitempty flag, and encoder funcs are resolved once per struct type into a cached []encField (previously re-parsed tags and rebuilt pointer-wrapper closures per field on every Encode). Slice/pointer dispatch (elemEnc/nilAsNull/elemPtrNil/recurseStructPtr) is precomputed and faithful to historical output.

Caches & concurrency

structInfo and parsed-path caches moved to sync.Map (no per-key mutex atomics, no cache-line bouncing in parallel decodes), with path caches living on each structInfo keyed by the plain path string — composite-key typehash was 35% of remaining simple-decode CPU.
Generation-tagged cache entries (decoder *cacheEntry, encoder *encPlan): every cache hit validates its generation, so any call starting after SetAliasTag/RegisterConverter/RegisterEncoder returns observes the new configuration, even if a build racing the reconfiguration stored a stale entry after the clear. Registered converters live in an immutable copy-on-write map behind an atomic.Pointer (lock-free reads, race-free registration); tag reads go through the config lock. Deterministic stale-injection tests plus -race hammer tests pin all of this.

Bug fixes (behavior improvements over `main`)

Unexported embedded pointers: on main, any Decode into a struct containing an unexported anonymous pointer field failed entirely with a recovered reflect.Value.Set using value obtained using unexported field — even with an empty src, and sibling fields did not decode. Such structs now decode normally; keys targeting fields behind the unsettable pointer are skipped cleanly (TestUnexportedEmbeddedPointerSkipped).
Double-embedded pointer panic: fields promoted through two levels of embedded pointers failed with a recovered reflect: indirection through nil pointer to embedded struct; the index-chain walk allocates intermediate nil pointers, so they now decode correctly.
Shadowed promoted fields: when an outer field shadows a promoted field's Go name under a different alias, the key now decodes into the field its alias identifies.
Same-named promoted fields with distinct aliases (two embeds each declaring X with aliases xa/xb) both decode now; main silently dropped them.
Slices of pointers to named types ([]*MyInt where type MyInt int): main panicked (int is not assignable to MyInt); values now convert before the pointer wrap.
default: on named types: type Role string, *Level, []MyInt, and even named pointer types (type MyIntPtr *MyInt) all panicked into a recovered error and the default was silently never applied; defaults are now converted/built against the actual field type.
ZeroEmpty + pointer-element slices: decoding an empty value into a []*T field with ZeroEmpty enabled used to panic; empty items now produce nil pointer elements.
Required-field validation gaps: isEmpty omitted int16 and native uint, so empty submissions to such required fields were silently accepted.
Caller's src map no longer mutated: passing multipart files used to inject file keys into — and overwrite caller-provided entries of — the caller's src map; file keys are now merged into an internal copy (taken only on the multipart path, so the common no-files decode is unchanged).
32-bit integer overflow: int/uint parsing rejects out-of-range values instead of silently truncating.

Panic-safety (previously crashed the calling process)

Encode into a nil dst map → now returns schema: dst map must not be nil (was an uncaught panic).
Encode of a non-nil pointer to an unsupported type (*map, *[]int) → clean "encoder not found" error (was a nil-encoder call crash); nil values keep the historical "null" output, empty/omitempty slices of such elements keep their historical skip/emit behavior, and []*Struct{nil,nil} still emits ["null","null"].
Encode now recovers panics (from reflection or user-registered encoders) into errors, mirroring Decode.
Decode with nil src + multipart files → previously panicked before the recovery handler was installed; the recover now guards the entire call and a nil src is handled by the internal merge.

Review & verification

Iterative adversarial review until clean: multi-angle finder fan-outs with 3-skeptic verification per candidate plus sweep rounds (multi-agent workflow), external CodeRabbit, Copilot, and maintainer review threads (all addressed on the PR), and a standing differential harness running ~1,500 labeled Decode/Encode cases against an origin/main build — slices/named/pointer element matrices, embedding/promotion/shadowing to depth 3, defaults, required-field error keys, TextUnmarshaler shapes, multipart, malformed/fuzzed path keys, encoder shapes, reconfiguration. Byte-identical output except the intended fixes above.
Statement coverage 99.4%; the only uncovered statements are race-window branches in the generation guards (exercised by hammer tests) and deliberately kept defensive guards. The coverage sweep itself surfaced two of the bug fixes listed above.
Full suite passes with -race -count=1 -shuffle=on; gofumpt clean; golangci-lint 0 issues. Dead code removed along the way (isValidStructPointer, MultiError.merge, convertPointer, duplicate type asserts).
New tests pin every fix and hot path: SWAR segment scanning (word boundaries, UTF-8), embedded/shadowed/ambiguous promotion, slice growth detachment and stale-data zeroing, named-type conversions (decode, defaults, pointers, slices), encoder pointer/slice classification incl. non-nil empty slices, caller-src immutability with multipart files, reconfiguration hammers and stale-entry injection for both caches, setBuiltinKind/builtinConverters parity, and the no-comma parse invariant.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

Speed up the schema package using the new SWAR primitives from gofiber/utils/v2 and by eliminating per-call allocations: - nextPathSegment: scan for '.' a word (8 bytes) at a time using the new swar package (Load8/ZeroLanes/Broadcast/FirstLane) instead of byte-by-byte, speeding up path parsing on cache misses and unknown keys. - checkRequired: consume the required-field map already precomputed in structInfo.requiredFields (previously computed but unused) instead of rebuilding it with findRequiredFields on every Decode call. - tagOptions: keep the raw comma-separated tag string and scan it lazily instead of allocating a []string via strings.Split; parseTag is now zero-alloc, which also removes an allocation per tagged field on every Encode call. - setDefaults: allocate the MultiError lazily and split '|'-separated slice defaults with strings.SplitSeq instead of strings.Split. - decode: split comma-separated slice values with strings.IndexByte + strings.SplitSeq, dropping the intermediate []string. benchstat (count=5, linux/amd64): LargeStructDecode 7.908µ → 6.895µ -12.81% 1200B → 748B 62 → 49 allocs SimpleStructDecode 3.474µ → 2.890µ -16.81% 479B → 190B 30 → 22 allocs CheckRequiredFields 1481n → 337.2n -77.23% 1048B → 0B 15 → 0 allocs TimeDurationDecoding 905.6n → 705.7n -22.07% 184B → 40B 8 → 5 allocs ParseTag 49.87n → 5.319n -89.33% 32B → 0B 1 → 0 allocs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

Profile-guided round two. Decoding a simple struct is now allocation-free (was 22 allocs/op) and large-struct decode drops from 49 to 6 allocs/op: - Cache encoding.TextUnmarshaler type facts per field (derefUnmarshaler / elemUnmarshaler) instead of re-deriving them with reflect.New + TypeAssert on every decode call; instances are bound to live values only in the branches that need them. This was 61% of all decode allocations. - Precompute field index chains (pathHop) in parsePath so decode walks v.Field(i) directly instead of repeating reflect FieldByName lookups (25% of decode CPU). The chains also allocate intermediate embedded nil pointers, fixing a panic ("indirection through nil pointer to embedded struct") when a promoted field sat behind two levels of embedded pointers; ensureAnonymousPtrs is folded into the hop metadata so the per-hop cache lookups and lock acquisitions disappear too. - Add setBuiltinKind, decoding primitives straight into the destination via v.SetInt/SetString/... instead of boxing through reflect.ValueOf. - Add decodeBuiltinSlice: slices of builtin kinds parse directly into slice slots (2 fixed allocs) instead of one reflect.Value per element, still built detached to preserve all-or-nothing error behavior. - Precompute structInfo.hasDefaults so setDefaults skips its whole reflection walk (23% of decode CPU) when no default tags exist. - Pre-size the remaining pooled slice materialization instead of growing from zero capacity. benchstat vs previous commit (count=5, linux/amd64): SimpleStructDecode 2890n → 618n -79% 190B → 0B 22 → 0 allocs LargeStructDecode 6.90µ → 1.70µ -75% 748B → 192B 49 → 6 allocs TimeDurationDecoding 706n → 356n -50% 40B → 16B 5 → 2 allocs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

The encoder re-parsed struct tags (reflect.StructTag.Lookup + parseTag), re-resolved encoder funcs, and rebuilt pointer-wrapper closures for every field on every Encode call. Build that plan once per struct type ([]encField with resolved alias, omitempty flag, and encoder funcs), memoized in a sync.Map invalidated by RegisterEncoder/SetAliasTag. Also pre-size slice output instead of appending from an empty slice, and allocate the error map lazily (encoding errors are the exception). benchstat (count=5, linux/amd64): LargeStructEncode ~3.1µs → ~1.3µs -58% ~1070B → ~880B 12 → 6 allocs SimpleStructEncode ~1.8µs → ~1.0µs -45% ~550B → ~460B 5 → 3 allocs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

…ecode - Replace the RWMutex-guarded structInfo and path caches with sync.Map: the per-key lock/unlock atomics disappear from the decode hot path and parallel decodes no longer bounce the mutex cache line. The mutex now only serializes configuration writes (tag, converter registration). - Hoist the "interface must be a (pointer to) struct" errors into sentinels; Decode's invalid-destination path is now allocation-free. - decodeBuiltinSlice: count elements upfront (comma splitting is deterministic per kind since no builtin syntax admits commas) and fill an exactly-sized slice, dropping the detached-header allocation and the append closure: one MakeSlice per slice field. - cache.converter: skip the map lookup when no custom converters are registered. benchstat (count=3, linux/amd64): All (error path) 35ns → 10ns 16B → 0B 1 → 0 allocs LargeStructDecode 1.70µ → 1.51µ 192B → 144B 6 → 4 allocs LargeStructDecodeParallel ~530-780ns/op, allocs 6 → 4 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

Growing a slice-of-structs one index at a time reallocated and copied the whole slice for every new index, making "items.0"..."items.N" decodes O(N^2). Grow with doubling headroom instead, and extend in place when the existing capacity suffices — zeroing the newly exposed slots so reused destination slices cannot leak stale element data into the result. Decoding 200 indexed struct elements: 33.5KB/18 allocs -> 12.8KB/7 allocs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

…art flag Profiling showed 35% of the remaining simple-decode CPU inside sync.Map lookups, almost all of it typehash/nilinterhash over the composite {path, type} cache key. Move the parsed-path cache onto structInfo keyed by the plain path string (cheap AES string hash), and resolve the root structInfo once per Decode call instead of once per key. Cache reset stays a single m.Clear() since the path caches now live on the infos. Also precompute fieldInfo.isMultipart so the decoder skips the multipart type comparisons for every non-multipart field. benchstat (count=3, linux/amd64): SimpleStructDecode 647ns → 470ns -27% LargeStructDecode 1.50µ → 1.23µ -18% LargeStructDecodeParallel ~530ns → ~450ns Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

Adversarial review of the optimization branch surfaced three behavioral regressions (all reproduced against main) plus cleanup debt. Fixes: - setDefaults gate: skipping the walk also skipped its historical side effect of allocating nil anonymous embedded pointers. The gate is now needsDefaultsWalk (defaults OR embedded pointers reachable), restoring the old contract that promoted fields are reachable after Decode. - Promoted-field resolution: hops were re-resolved with t.FieldByName, whose name-based promotion rules disagree with the alias table when an outer field shadows a promoted field's Go name (nil-interface panic via mismatched cached unmarshaler facts) or when two embeds share a name (silently skipped keys turned into hard errors). structInfo now records each field's index chain while it walks the embedded structs, so paths resolve by identity; shadowed and same-named promoted fields decode correctly by alias. - Slice growth: extending within existing capacity wrote into memory the caller may share through other slices aliasing the original array; growth always detaches into a fresh backing array again. - Cache invalidation races: a structInfo/encoder-plan build racing SetAliasTag/RegisterConverter/RegisterEncoder could re-insert stale metadata after the clear, permanently latching the old configuration. Generation counters now guard both caches (check before store, re-check and delete after), and the encoder reads tag/regenc under the config lock, with a hammer test pinning the reconfiguration contract. Cleanup and efficiency from the same review: replace the parallel int parsing with utils.ParseNativeInt/ParseNativeUint (proper range errors on 32-bit instead of silent truncation); precompute required-field search paths so checkRequired allocates nothing; single-pass slice sizing via comma counting; drop dead MultiError.merge, isValidStructPointer and the write-only unmarshaler instance field; reuse identical unmarshaler facts in createField; thread rootInfo through Decode; make the terminal slice-element part explicit (pathPart.elem) instead of inferring it from empty hops; simplify tagOptions with strings.SplitSeq. New tests pin every fixed behavior: embedded-pointer allocation without defaults, shadowed/ambiguous promoted fields, slice growth detachment, encoder reconfiguration under concurrency, setBuiltinKind/builtinConverters table parity, and the no-commas parse invariant the slice fast path relies on. benchstat vs main after fixes: geomean -76% time; decode paths unchanged or faster than before the fixes (required-fields check -19%). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

codecov · 2026-07-17T23:59:33Z

Codecov Report

❌ Patch coverage is 98.85584% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.23%. Comparing base (70537ba) to head (657f120).

Files with missing lines	Patch %	Lines
encoder.go	97.08%	1 Missing and 2 partials ⚠️
decoder.go	98.70%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #76      +/-   ##
==========================================
+ Coverage   96.65%   99.23%   +2.57%     
==========================================
  Files           4        4              
  Lines         866     1039     +173     
==========================================
+ Hits          837     1031     +194     
+ Misses         16        4      -12     
+ Partials       13        4       -9

Flag	Coverage Δ
unittests	`99.23% <98.85%> (+2.57%)`	⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

coderabbitai · 2026-07-18T00:08:08Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR refactors schema metadata and path caching, updates decoder traversal and builtin conversion, adds embedded-pointer and slice-growth coverage, and introduces generation-safe cached encoding plans.

Changes

Schema caching and decoding

Layer / File(s)	Summary
Cached path and field metadata `cache.go`, `cache_test.go`	Per-struct path caches now store index-hop navigation, embedded-pointer allocation data, unmarshaler metadata, required-field mappings, and raw tag options.
Decoder traversal and validation `decoder.go`, `decoder_test.go`, `multierror_test.go`	Decoding uses cached metadata for defaults, required fields, promoted fields, embedded-pointer allocation, multipart checks, detached slice growth, and merged errors.
Builtin conversion and slice decoding `converter.go`, `converter_test.go`, `decoder.go`, `decoder_test.go`	Builtin values and slices use direct assignment, streaming comma parsing, cached unmarshaler facts, and consistent conversion errors.

Encoder plan caching

Layer / File(s)	Summary
Cached encoding plans `encoder.go`, `encoder_test.go`	Encoder field plans are cached by struct type and invalidated when aliases or custom encoders are reconfigured, with concurrency and panic-recovery coverage.

Repository support changes

Layer / File(s)	Summary
Ignore rules and supporting validation `.gitignore`, `util_test.go`, `multierror_test.go`	Ignore rules cover test binaries, tag-option tests use the raw option representation, and error merging tests cover nil inputs.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Decoder
  participant Cache
  participant Destination
  Client->>Decoder: Decode source into destination
  Decoder->>Cache: Load root struct metadata
  Cache-->>Decoder: Return path and field metadata
  Decoder->>Destination: Walk index chains and allocate embedded pointers
  Decoder->>Destination: Decode scalar or slice values
  Decoder-->>Client: Return decoded result or errors

Possibly related PRs

gofiber/schema#29: Overlaps with tag-option and utility-test changes.
gofiber/schema#31: Covers related multipart and comma-separated slice decoding.
gofiber/schema#45: Refactors related path caching and embedded-pointer decoding.

Suggested reviewers: sixcolors, renewerner87, efectn

Poem

I hopped through paths where pointers grow,
And cached each field in rows below.
Slices split clean, encoders sing,
Fresh plans bloom when tags take wing.
A bunny’s nod: “Ship the spring!”

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The .gitignore update is unrelated to the schema optimization work and appears to be an extra change outside the issue scope.	Remove the .gitignore-only housekeeping change or explain why it is needed for the schema optimization PR.
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address `#2` by optimizing parsePath, required-field discovery, defaults, reflection access, splitting, and conversions.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: performance-focused decode/encode hot-path rewrites with cached metadata, SWAR parsing, and safety fixes.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch claude/schema-performance-optimization-kw0q51

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

util_test.go (1)
47-48: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Restore isValidStructPointer coverage.

Deleting TestIsValidStructPointer removes regression coverage for valid pointers, nil pointers, non-struct values, and non-struct pointers. Restore the test unless equivalent coverage now exists elsewhere.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@util_test.go` around lines 47 - 48, Restore TestIsValidStructPointer coverage
for valid struct pointers, nil pointers, non-struct values, and non-struct
pointers; if equivalent tests already exist, ensure those cases remain covered
instead of duplicating them.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.gitignore:
- Around line 1-2: Restore the .gitignore rules for coverage.coverprofile,
vendor, and .idea alongside the existing *.test binary rule, ensuring these
generated or environment-specific files and directories remain excluded from
commits.

In `@cache.go`:
- Around line 35-45: The cache configuration read path is unsynchronized:
registerConverter updates regconv under l while converter reads it directly.
Update converter and the related configuration access around registerConverter
and the referenced ranges to read an immutable configuration snapshot atomically
or copy the configuration under l before use, ensuring Decode cannot
concurrently access regconv during reconfiguration.

In `@decoder.go`:
- Around line 176-185: Prevent the anonymous embedded-pointer allocation in the
decoder from calling Set on unsettable fields: update the metadata construction
for anonymousPtrFields or guard the allocation loop before field.Set. Ensure
unexported or otherwise non-settable embedding chains are skipped safely while
preserving allocation for settable nil pointers, including the related walker
logic around the other reported location.

In `@encoder.go`:
- Around line 86-87: Make cached encoder plans generation-tagged and have
Encode’s cache-hit path validate the entry against the current encGen before
using it; reject mismatches and rebuild. Update the plan publication flow to
atomically publish the tagged plan rather than relying on Store followed by a
generation check. In encoder.go lines 86-87 and 126-130, apply these
implementation changes; in encoder_test.go lines 803-819, coordinate a delayed
old-generation store after reconfiguration and verify Encode ignores it.

---

Outside diff comments:
In `@util_test.go`:
- Around line 47-48: Restore TestIsValidStructPointer coverage for valid struct
pointers, nil pointers, non-struct values, and non-struct pointers; if
equivalent tests already exist, ensure those cases remain covered instead of
duplicating them.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: afea30d0-f1c1-4c3c-9a19-335eae55229d

📥 Commits

Reviewing files that changed from the base of the PR and between 70537ba and c91c3ba.

📒 Files selected for processing (11)

.gitignore
cache.go
cache_test.go
converter.go
converter_test.go
decoder.go
decoder_test.go
encoder.go
encoder_test.go
multierror_test.go
util_test.go

📜 Review details

🔇 Additional comments (9)

util_test.go (1)

39-39: LGTM!

encoder.go (1)

8-79: LGTM!

Also applies to: 164-235

cache.go (1)

12-32: LGTM!

Also applies to: 63-198, 390-575

cache_test.go (1)

4-83: LGTM!

Also applies to: 98-107

decoder.go (1)

21-24: LGTM!

Also applies to: 112-151, 186-307, 451-779, 882-895

decoder_test.go (1)

13-13: LGTM!

Also applies to: 2292-2524, 3730-3733

multierror_test.go (1)

39-59: LGTM!

converter.go (1)

93-94: LGTM!

Also applies to: 132-133, 166-260

converter_test.go (1)

4-4: LGTM!

Also applies to: 69-169

…panic Coverage-driven test sweep over every reachable branch. Writing the tests surfaced a real long-standing bug: decoding an empty value into a []*T field with ZeroEmpty enabled appended reflect.Zero of the dereferenced element type, panicking with "int is not assignable to *int" (also broken on main via reflect.Append). Empty items now produce nil pointer elements. New tests: malformed path keys (empty, leading/trailing/double separators, bad indices, paths past scalar fields) with and without IgnoreUnknownKeys; pointer-element and custom-converter slices through the pooled path (comma fallback, Convert-to-element, error indices, ZeroEmpty); multipart field shape helpers; array-of-pointer field walks; nil-error appendError; out-of-range converter kinds; nested encoder error collection; decoder reconfiguration hammer (mirroring the encoder one); and concurrent same-path decoding. Also removed code the coverage work proved dead: the duplicate slice-level TypeAssert in isTextUnmarshaler, an unreachable else branch in the slice unmarshaler loop (reflect.New always yields a pointer), and never-nil cache.get guards. Metadata builds now read the alias tag under the configuration lock (cache.aliasTag) so the new concurrency tests are race-clean. Remaining uncovered statements are three nanosecond race-window branches in the generation guards (exercised by the hammer tests, not deterministically hittable) and two deliberately kept defensive guards against future shape drift. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

- Restore the .gitignore rules (coverage.coverprofile, vendor, .idea) that the earlier *.test addition accidentally clobbered. - Make converter registration race-free: regconv is now an immutable map published through an atomic pointer (copy-on-write under the config lock), so Decode's lock-free converter lookups can never race a concurrent RegisterConverter map write. - Close the residual stale-consume window in both metadata caches: structInfo and encoder-plan entries are tagged with the configuration generation they were built under and validated on every cache hit, so an entry stored by a build that raced a reconfiguration is never served once the reconfiguration returned. This replaces the store-then-delete dance with a simpler invariant, with deterministic stale-injection tests for both decoder and encoder. - Never Set unsettable embedded pointers: anonymousPtrFields only lists exported anonymous pointers, and walkIndexChain reports chains blocked by an unsettable nil pointer as unreachable instead of panicking (the old code panicked into a decode error for structs with unexported embedded pointers; siblings and defaults now decode normally). Adds a RegisterConverter-during-Decode hammer test alongside the existing reconfiguration hammers. Hot-path benchmarks unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

Final review sweep found scanDefaultsTree still counting unexported anonymous pointer fields, which the CodeRabbit fix excluded from allocation — so a struct tree whose only embedded pointers are unexported ran the full setDefaults walk on every Decode with zero effect. Mirror the IsExported filter so needsDefaultsWalk only reflects pointers the walk can actually allocate, with a test pinning all three gate cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

The removed isValidStructPointer helper's four cases (valid struct pointer, nil struct pointer, struct value, pointer to non-struct) are decided by the cached plan's recurseStructPtr flag plus a runtime nil check since the encoder plan refactor; cover them behaviorally at the Encode level instead of unit-testing a deleted helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (1)

decoder_test.go (1)
4240-4250: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Enforce the all-or-nothing slice-conversion contract in failure tests. Both tests only check the returned error, allowing partial destination mutation to regress unnoticed.

decoder_test.go#L4240-L4250: seed s.P and assert it remains unchanged after "1,x" fails.

decoder_test.go#L4285-L4289: seed s.C and assert it remains unchanged after "a,bad,c" fails.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@decoder_test.go` around lines 4240 - 4250, The slice-conversion failure tests
must verify that destination fields remain unchanged, not just that conversion
errors are returned. In decoder_test.go:4240-4250, seed s.P before decoding and
assert it is unchanged for {"1,x"}; in decoder_test.go:4285-4289, seed s.C
before decoding and assert it is unchanged for {"a,bad,c"}. Preserve the
existing MultiError and ConversionError assertions at both sites.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@decoder_test.go`:
- Around line 4427-4438: Update the decoder setup in the test around NewDecoder
and IgnoreUnknownKeys so it uses the default unknown-key behavior instead of
suppressing unknown fields. Keep the input containing x and a, and retain the
assertion that the sibling field decodes, so the test distinguishes a
parsed-but-unreachable promoted field from x being absent from metadata.
- Around line 4497-4503: The concurrent decode in the goroutine currently
discards its result, allowing panic or race-related errors to pass unnoticed.
Send the error returned by d.Decode through the existing done channel, then
after RegisterConverter returns receive and assert that error is nil while
preserving the deferred channel close.

---

Nitpick comments:
In `@decoder_test.go`:
- Around line 4240-4250: The slice-conversion failure tests must verify that
destination fields remain unchanged, not just that conversion errors are
returned. In decoder_test.go:4240-4250, seed s.P before decoding and assert it
is unchanged for {"1,x"}; in decoder_test.go:4285-4289, seed s.C before decoding
and assert it is unchanged for {"a,bad,c"}. Preserve the existing MultiError and
ConversionError assertions at both sites.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 51423904-2c9c-4523-9dc0-8568d9cb32f2

📥 Commits

Reviewing files that changed from the base of the PR and between c91c3ba and 64ada3d.

📒 Files selected for processing (8)

.gitignore
cache.go
cache_test.go
converter_test.go
decoder.go
decoder_test.go
encoder.go
encoder_test.go

🚧 Files skipped from review as they are similar to previous changes (5)

converter_test.go
cache_test.go
encoder.go
cache.go
decoder.go

📜 Review details

🔇 Additional comments (3)

.gitignore (1)

1-7: LGTM!

encoder_test.go (1)

6-6: LGTM!

Also applies to: 824-923

decoder_test.go (1)

15-15: LGTM!

Also applies to: 4143-4239, 4251-4284, 4290-4426, 4439-4496, 4504-4513

Panic audit of both entry points. Three crashes found (all reproduce on main) and one hardening gap, now fixed: - Decode with a nil src map plus multipart files panicked on the nil-map write before the recovery handler was installed. The recover is now installed first thing after dst validation, and a nil src is allocated when files need to be merged in. - Encode into a nil dst map panicked on the first assignment; it now returns a "dst map must not be nil" error. - Encoding a non-nil pointer to an unsupported element type (*map, *[]T) called a nil inner encoder closure and crashed. typeEncoder no longer builds closures over nil element encoders; such fields report "encoder not found" while nil values keep the historical "null" output (and omitempty still suppresses it) via the plan's nilAsNull flag. - Encode had no panic recovery at all, so any reflection edge case or a panicking user-registered encoder crashed the process. It now recovers into an error, mirroring Decode. Decoder-side reflection panics (negative indices, unexported fields, unmarshaler edge cases) were audited and remain converted to errors by the existing recover, which now covers the entire call. Regression tests cover all four fixes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

Two CodeRabbit suggestions on the new tests: - TestUnexportedEmbeddedPointerSkipped now uses the default unknown-key behavior so it distinguishes a parsed-but-unreachable promoted field (no error) from the field missing from metadata entirely (which would surface as an UnknownKeyError instead of silently passing). - TestDecoderRegisterConverterDuringDecode now propagates the concurrent decode's error through the channel and fails on it instead of discarding a recovered panic or race-related failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

Code review (xhigh) found a confirmed decode panic and two lesser issues. - []*NamedType (e.g. []*MyInt where `type MyInt int`) panicked: the builtin converter yields the underlying kind (int), which was Set into the *MyInt pointer without converting to the named type first — the Convert was gated behind !isPtrElem and skipped. The panic unwound the whole decode loop (recovered as "panic while decoding"), silently dropping other fields. Extracted appendConvertedItem, which converts before wrapping, fixing both the main and comma-split sites. - isEmpty omitted int16 and native uint, so a required int16/uint field submitted empty was never reported empty, bypassing `required`. - needsDefaultsWalk (was scanDefaultsTree) no longer re-parses tags under a per-field config lock: create() snapshots the alias tag once and passes it to createField and the walk, cutting O(fields x depth) RLock churn from each structInfo build (ParsePathCacheMiss ~830ns -> ~510ns). Regression tests cover []*named-int/[]*named-string decoding and required int16/uint empty detection. Hot-path decode benchmarks unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

A multi-agent review found the same converter-result/named-type mismatch that appendConvertedItem fixed for slice decoding, still present in all three setDefaults `default:` paths. Builtin converters (and convertPointer) return the underlying Go kind, which was Set/Append-ed into the named target without conversion, panicking (recovered into a MultiError) so the default was silently never applied: - named scalar: `type Role string; R Role schema:"r,default:admin"` - pointer-to-named: `type Level int; P *Level schema:"p,default:7"` - named-element slice: `type MyInt int; A []MyInt schema:"a,default:1|2|3"` Each now converts the value to the field's (possibly named) element/target type before reflect.Append / Set, mirroring the decode path's value.Convert(t). Unnamed-type defaults are unaffected (Convert to the same type is identity). Regression test covers named string/int/bool/float, pointer-to-named, and named-element slices. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

A multi-agent review caught an encoder regression: this branch made typeEncoder return nil for pointers to unencodable types (a panic fix), so []*Struct (and other slices whose element has no encoder) got elemEnc==nil. The encode loop checked elemEnc==nil and errored BEFORE the empty/omitempty guard, so an empty or omitempty []*Struct now returned "schema: encoder not found" — on main such empty fields encoded cleanly (skipped under omitempty, or emitted as an empty slice), breaking the omitempty skip contract. Reorder so slice emptiness/omitempty is handled first: an empty slice has nothing to encode and is skipped (omitempty) or emitted empty even when its element type has no encoder; only a non-empty such slice errors. Non-slice unencodable fields (map, chan, array, non-nil pointer to unencodable) still error unconditionally, matching main. Non-empty []*Struct now errors cleanly where main panicked. Regression test covers omitempty-skip, empty-emit, non-empty-error, and map-field-still-errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@encoder_test.go`:
- Around line 1010-1032: Add allocated empty-slice cases to the test structs
used in the omitempty and plain encoding scenarios by setting Children to
make([]*Inner, 0) instead of leaving it nil. Preserve the existing assertions
verifying omitempty skips the field and plain encoding emits an empty value,
ensuring both behaviors are covered for non-nil empty slices.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d1c32193-3f66-4248-b7fb-e6eeb3e7f06b

📥 Commits

Reviewing files that changed from the base of the PR and between 9571910 and ec80ef9.

📒 Files selected for processing (2)

encoder.go
encoder_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

encoder.go

Differential testing against main showed the previous empty-slice fix over-corrected: main encodes a []*Struct with nil elements as ["null",...] (matching a scalar nil *Struct), and errors unconditionally on a value []Struct. The prior fix errored on the former and emitted an empty slice for empty of the latter. Restore faithful behavior via encField.elemPtrNil (a slice whose element is a pointer with no encoder): nil elements encode as "null", a non-nil such element errors (main panicked). Slices whose element is unencodable and not a pointer (e.g. []Struct) error unconditionally as before. Empty/omitempty pointer-element slices are still skipped/emitted-empty (the original regression fix). Confirmed byte-identical to main across the full encoder differential except the intended fixes (main panics -> clean errors). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

The encoder's slice empty/omitempty handling is driven by Len()==0, not a nil check, so a non-nil empty slice (make([]*Inner, 0)) must behave identically to a nil one. Add explicit make([]*Inner, 0) cases alongside the existing nil cases for both omitempty (skipped) and plain (emitted empty) scenarios. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

CI flagged a BenchmarkConvertPointer regression. A same-machine A/B showed convertPointer is byte-identical to main and statistically the same speed (~34ns both), so the flag was CI run-to-run variance, not a real regression. But the earlier ParseNativeInt switch did give up utils.ParseInt's SWAR fast path (ParseNativeInt always takes the generic parseSigned path), which is slower for longer numeric inputs. Route native int/uint conversion back through utils.ParseInt/ParseUint (fast path) with an explicit native-fit range check (parseNativeInt/parseNativeUint): a no-op on 64-bit, and on 32-bit it still rejects out-of-range values that a plain int() conversion would truncate. This keeps both the performance and the 32-bit correctness, and is shared by convertInt/convertUint and setBuiltinKind. setBuiltinKind/builtinConverters parity test still passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

ReneWerner87

LGTM

Copilot

Pull request overview

Performance-focused rewrite of the schema package’s encode/decode hot paths, adding SWAR-assisted path scanning, more aggressive caching, and allocation reductions while preserving (and expanding) behavioral coverage with new regression and concurrency tests.

Changes:

Decoder: precomputed field index chains + per-field TextUnmarshaler facts, SWAR path parsing, faster builtin setters and slice decoding, and reduced/guarded defaults + required-field checks.
Encoder: per-struct-type encoding plans cached with generation-based invalidation for safe concurrent reconfiguration.
Tests/benchmarks: substantial new regression and concurrency coverage for the new caching and parsing behaviors.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
util_test.go	Updates tagOptions test to match new tagOptions representation; removes obsolete helper test.
multierror_test.go	Renames and adjusts merge test to new `mergeErrors` API.
encoder.go	Adds cached per-type encoding plan, generation invalidation, nil-dst handling, and panic recovery.
encoder_test.go	Adds extensive tests for encoder plan invalidation, nil-dst behavior, panic recovery, and edge cases.
decoder.go	Adds faster decode paths (builtin setters/slices), SWAR-aware parsing integration, and default/required optimizations.
decoder_test.go	Adds regression and concurrency tests for new decode semantics and caching behavior.
converter.go	Adds native int/uint range checks and introduces `setBuiltinKind` fast setter.
converter_test.go	Adds parity tests to keep `setBuiltinKind` aligned with `builtinConverters`, plus invariants for slice sizing.
cache.go	Reworks metadata/path caches (sync.Map + generation), adds SWAR segment scan, and optimizes tag option parsing.
cache_test.go	Adds SWAR segment tests, defaults-walk gating tests, and updated path-cache detachment assertions.
.gitignore	Ignores `*.test` binaries.

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

Review follow-up on the pointer-default path. The flagged case (*MyInt) was already correct — *int is convertible to *MyInt (both unnamed pointer types with identical underlying base types), covered by TestDefaultsOnNamedTypes — but the adjacent case of a NAMED pointer type (type MyIntPtr *MyInt) was not: *int is not convertible to MyIntPtr, so its default panicked into a recovered error (identically before and after the earlier Convert change) and was never applied. Build the default as reflect.New(elem) and convert the parsed value to the element type instead: *elem is assignable to the field even when the field type is itself a named pointer type. This replaces convertPointer's base-kind pointer construction; convertPointer is now production-dead and removed along with its tests/benchmark (the benchmark was also the noisy CI row that false-flagged a variance-only regression). TestDefaultsOnNamedTypes now covers the named-pointer-type default too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

gaby · 2026-07-19T13:14:06Z

Benchmark

Current: This Pull Request
Previous: main branch

Benchmark suite	Current: `11e3f89`	Previous: `70537ba`	Ratio
`BenchmarkNextPathSegment`	`15.68` ns/op 0 B/op 0 allocs/op
`BenchmarkNextPathSegment - ns/op`	`15.68` ns/op
`BenchmarkNextPathSegment - B/op`	`0` B/op
`BenchmarkNextPathSegment - allocs/op`	`0` allocs/op
`BenchmarkParsePathCacheMiss`	`496.9` ns/op 504 B/op 9 allocs/op
`BenchmarkParsePathCacheMiss - ns/op`	`496.9` ns/op
`BenchmarkParsePathCacheMiss - B/op`	`504` B/op
`BenchmarkParsePathCacheMiss - allocs/op`	`9` allocs/op
`BenchmarkConvertBool`	`3.432` ns/op 0 B/op 0 allocs/op	`3.471` ns/op 0 B/op 0 allocs/op	`0.99`
`BenchmarkConvertBool - ns/op`	`3.432` ns/op	`3.471` ns/op	`0.99`
`BenchmarkConvertBool - B/op`	`0` B/op	`0` B/op	`1`
`BenchmarkConvertBool - allocs/op`	`0` allocs/op	`0` allocs/op	`1`
`BenchmarkConvertInt`	`10.1` ns/op 0 B/op 0 allocs/op	`8.486` ns/op 0 B/op 0 allocs/op	`1.19`
`BenchmarkConvertInt - ns/op`	`10.1` ns/op	`8.486` ns/op	`1.19`
`BenchmarkConvertInt - B/op`	`0` B/op	`0` B/op	`1`
`BenchmarkConvertInt - allocs/op`	`0` allocs/op	`0` allocs/op	`1`
`BenchmarkAll`	`3.458` ns/op 0 B/op 0 allocs/op	`33.2` ns/op 16 B/op 1 allocs/op	`0.10`
`BenchmarkAll - ns/op`	`3.458` ns/op	`33.2` ns/op	`0.10`
`BenchmarkAll - B/op`	`0` B/op	`16` B/op	`0`
`BenchmarkAll - allocs/op`	`0` allocs/op	`1` allocs/op	`0`
`BenchmarkSliceManyIndicesDecode`	`41242` ns/op 19601 B/op 20 allocs/op
`BenchmarkSliceManyIndicesDecode - ns/op`	`41242` ns/op
`BenchmarkSliceManyIndicesDecode - B/op`	`19601` B/op
`BenchmarkSliceManyIndicesDecode - allocs/op`	`20` allocs/op
`BenchmarkDecoderMultipartFiles`	`1612` ns/op 96 B/op 5 allocs/op	`6217` ns/op 1567 B/op 48 allocs/op	`0.26`
`BenchmarkDecoderMultipartFiles - ns/op`	`1612` ns/op	`6217` ns/op	`0.26`
`BenchmarkDecoderMultipartFiles - B/op`	`96` B/op	`1567` B/op	`0.0612635609444799`
`BenchmarkDecoderMultipartFiles - allocs/op`	`5` allocs/op	`48` allocs/op	`0.10`
`BenchmarkIsMultipartFile/IsMultipartFile-0`	`7.482` ns/op 0 B/op 0 allocs/op	`6.869` ns/op 0 B/op 0 allocs/op	`1.09`
`BenchmarkIsMultipartFile/IsMultipartFile-0 - ns/op`	`7.482` ns/op	`6.869` ns/op	`1.09`
`BenchmarkIsMultipartFile/IsMultipartFile-0 - B/op`	`0` B/op	`0` B/op	`1`
`BenchmarkIsMultipartFile/IsMultipartFile-0 - allocs/op`	`0` allocs/op	`0` allocs/op	`1`
`BenchmarkIsMultipartFile/IsMultipartFile-1`	`7.695` ns/op 0 B/op 0 allocs/op	`6.89` ns/op 0 B/op 0 allocs/op	`1.12`
`BenchmarkIsMultipartFile/IsMultipartFile-1 - ns/op`	`7.695` ns/op	`6.89` ns/op	`1.12`
`BenchmarkIsMultipartFile/IsMultipartFile-1 - B/op`	`0` B/op	`0` B/op	`1`
`BenchmarkIsMultipartFile/IsMultipartFile-1 - allocs/op`	`0` allocs/op	`0` allocs/op	`1`
`BenchmarkIsMultipartFile/IsMultipartFile-2`	`5.944` ns/op 0 B/op 0 allocs/op	`5.616` ns/op 0 B/op 0 allocs/op	`1.06`
`BenchmarkIsMultipartFile/IsMultipartFile-2 - ns/op`	`5.944` ns/op	`5.616` ns/op	`1.06`
`BenchmarkIsMultipartFile/IsMultipartFile-2 - B/op`	`0` B/op	`0` B/op	`1`
`BenchmarkIsMultipartFile/IsMultipartFile-2 - allocs/op`	`0` allocs/op	`0` allocs/op	`1`
`BenchmarkIsMultipartFile/IsMultipartFile-3`	`7.514` ns/op 0 B/op 0 allocs/op	`6.856` ns/op 0 B/op 0 allocs/op	`1.10`
`BenchmarkIsMultipartFile/IsMultipartFile-3 - ns/op`	`7.514` ns/op	`6.856` ns/op	`1.10`
`BenchmarkIsMultipartFile/IsMultipartFile-3 - B/op`	`0` B/op	`0` B/op	`1`
`BenchmarkIsMultipartFile/IsMultipartFile-3 - allocs/op`	`0` allocs/op	`0` allocs/op	`1`
`BenchmarkIsMultipartFile/IsMultipartFile-4`	`4.06` ns/op 0 B/op 0 allocs/op	`3.738` ns/op 0 B/op 0 allocs/op	`1.09`
`BenchmarkIsMultipartFile/IsMultipartFile-4 - ns/op`	`4.06` ns/op	`3.738` ns/op	`1.09`
`BenchmarkIsMultipartFile/IsMultipartFile-4 - B/op`	`0` B/op	`0` B/op	`1`
`BenchmarkIsMultipartFile/IsMultipartFile-4 - allocs/op`	`0` allocs/op	`0` allocs/op	`1`
`BenchmarkIsMultipartFile/IsMultipartFile-5`	`7.509` ns/op 0 B/op 0 allocs/op	`6.964` ns/op 0 B/op 0 allocs/op	`1.08`
`BenchmarkIsMultipartFile/IsMultipartFile-5 - ns/op`	`7.509` ns/op	`6.964` ns/op	`1.08`
`BenchmarkIsMultipartFile/IsMultipartFile-5 - B/op`	`0` B/op	`0` B/op	`1`
`BenchmarkIsMultipartFile/IsMultipartFile-5 - allocs/op`	`0` allocs/op	`0` allocs/op	`1`
`BenchmarkIsMultipartFile/IsMultipartFile-6`	`13.08` ns/op 0 B/op 0 allocs/op	`13.18` ns/op 0 B/op 0 allocs/op	`0.99`
`BenchmarkIsMultipartFile/IsMultipartFile-6 - ns/op`	`13.08` ns/op	`13.18` ns/op	`0.99`
`BenchmarkIsMultipartFile/IsMultipartFile-6 - B/op`	`0` B/op	`0` B/op	`1`
`BenchmarkIsMultipartFile/IsMultipartFile-6 - allocs/op`	`0` allocs/op	`0` allocs/op	`1`
`BenchmarkHandleMultipartField`	`184.7` ns/op 48 B/op 2 allocs/op	`182.9` ns/op 48 B/op 2 allocs/op	`1.01`
`BenchmarkHandleMultipartField - ns/op`	`184.7` ns/op	`182.9` ns/op	`1.01`
`BenchmarkHandleMultipartField - B/op`	`48` B/op	`48` B/op	`1`
`BenchmarkHandleMultipartField - allocs/op`	`2` allocs/op	`2` allocs/op	`1`
`BenchmarkLargeStructDecode`	`1440` ns/op 144 B/op 4 allocs/op	`7014` ns/op 1200 B/op 62 allocs/op	`0.21`
`BenchmarkLargeStructDecode - ns/op`	`1440` ns/op	`7014` ns/op	`0.21`
`BenchmarkLargeStructDecode - B/op`	`144` B/op	`1200` B/op	`0.12`
`BenchmarkLargeStructDecode - allocs/op`	`4` allocs/op	`62` allocs/op	`0.06451612903225806`
`BenchmarkLargeStructDecodeParallel`	`565` ns/op 144 B/op 4 allocs/op	`3495` ns/op 1200 B/op 62 allocs/op	`0.16`
`BenchmarkLargeStructDecodeParallel - ns/op`	`565` ns/op	`3495` ns/op	`0.16`
`BenchmarkLargeStructDecodeParallel - B/op`	`144` B/op	`1200` B/op	`0.12`
`BenchmarkLargeStructDecodeParallel - allocs/op`	`4` allocs/op	`62` allocs/op	`0.06451612903225806`
`BenchmarkSimpleStructDecode`	`600.8` ns/op 0 B/op 0 allocs/op	`3181` ns/op 479 B/op 30 allocs/op	`0.19`
`BenchmarkSimpleStructDecode - ns/op`	`600.8` ns/op	`3181` ns/op	`0.19`
`BenchmarkSimpleStructDecode - B/op`	`0` B/op	`479` B/op	`0`
`BenchmarkSimpleStructDecode - allocs/op`	`0` allocs/op	`30` allocs/op	`0`
`BenchmarkCheckRequiredFields`	`229.1` ns/op 0 B/op 0 allocs/op	`1200` ns/op 1048 B/op 15 allocs/op	`0.19`
`BenchmarkCheckRequiredFields - ns/op`	`229.1` ns/op	`1200` ns/op	`0.19`
`BenchmarkCheckRequiredFields - B/op`	`0` B/op	`1048` B/op	`0`
`BenchmarkCheckRequiredFields - allocs/op`	`0` allocs/op	`15` allocs/op	`0`
`BenchmarkTimeDurationDecoding`	`307.2` ns/op 16 B/op 2 allocs/op	`783.6` ns/op 184 B/op 8 allocs/op	`0.39`
`BenchmarkTimeDurationDecoding - ns/op`	`307.2` ns/op	`783.6` ns/op	`0.39`
`BenchmarkTimeDurationDecoding - B/op`	`16` B/op	`184` B/op	`0.08695652173913043`
`BenchmarkTimeDurationDecoding - allocs/op`	`2` allocs/op	`8` allocs/op	`0.25`
`BenchmarkSimpleStructEncode`	`898.5` ns/op 463 B/op 3 allocs/op	`1602` ns/op 549 B/op 5 allocs/op	`0.56`
`BenchmarkSimpleStructEncode - ns/op`	`898.5` ns/op	`1602` ns/op	`0.56`
`BenchmarkSimpleStructEncode - B/op`	`463` B/op	`549` B/op	`0.84`
`BenchmarkSimpleStructEncode - allocs/op`	`3` allocs/op	`5` allocs/op	`0.60`
`BenchmarkSimpleStructEncodeParallel`	`483.5` ns/op 501 B/op 3 allocs/op	`778.9` ns/op 587 B/op 5 allocs/op	`0.62`
`BenchmarkSimpleStructEncodeParallel - ns/op`	`483.5` ns/op	`778.9` ns/op	`0.62`
`BenchmarkSimpleStructEncodeParallel - B/op`	`501` B/op	`587` B/op	`0.85`
`BenchmarkSimpleStructEncodeParallel - allocs/op`	`3` allocs/op	`5` allocs/op	`0.60`
`BenchmarkLargeStructEncode`	`1199` ns/op 872 B/op 6 allocs/op	`2743` ns/op 1060 B/op 12 allocs/op	`0.44`
`BenchmarkLargeStructEncode - ns/op`	`1199` ns/op	`2743` ns/op	`0.44`
`BenchmarkLargeStructEncode - B/op`	`872` B/op	`1060` B/op	`0.82`
`BenchmarkLargeStructEncode - allocs/op`	`6` allocs/op	`12` allocs/op	`0.50`
`BenchmarkLargeStructEncodeParallel`	`708.9` ns/op 913 B/op 6 allocs/op	`1407` ns/op 1037 B/op 12 allocs/op	`0.50`
`BenchmarkLargeStructEncodeParallel - ns/op`	`708.9` ns/op	`1407` ns/op	`0.50`
`BenchmarkLargeStructEncodeParallel - B/op`	`913` B/op	`1037` B/op	`0.88`
`BenchmarkLargeStructEncodeParallel - allocs/op`	`6` allocs/op	`12` allocs/op	`0.50`
`BenchmarkTimeDurationEncoding`	`150.9` ns/op 89 B/op 1 allocs/op	`324.1` ns/op 142 B/op 2 allocs/op	`0.47`
`BenchmarkTimeDurationEncoding - ns/op`	`150.9` ns/op	`324.1` ns/op	`0.47`
`BenchmarkTimeDurationEncoding - B/op`	`89` B/op	`142` B/op	`0.63`
`BenchmarkTimeDurationEncoding - allocs/op`	`1` allocs/op	`2` allocs/op	`0.50`
`BenchmarkMultiErrorError`	`189.3` ns/op 40 B/op 2 allocs/op	`184.9` ns/op 40 B/op 2 allocs/op	`1.02`
`BenchmarkMultiErrorError - ns/op`	`189.3` ns/op	`184.9` ns/op	`1.02`
`BenchmarkMultiErrorError - B/op`	`40` B/op	`40` B/op	`1`
`BenchmarkMultiErrorError - allocs/op`	`2` allocs/op	`2` allocs/op	`1`
`BenchmarkParseTag`	`5.641` ns/op 0 B/op 0 allocs/op	`49.4` ns/op 32 B/op 1 allocs/op	`0.11`
`BenchmarkParseTag - ns/op`	`5.641` ns/op	`49.4` ns/op	`0.11`
`BenchmarkParseTag - B/op`	`0` B/op	`32` B/op	`0`
`BenchmarkParseTag - allocs/op`	`0` allocs/op	`1` allocs/op	`0`
`BenchmarkIsZero`	`20.39` ns/op 0 B/op 0 allocs/op	`19.12` ns/op 0 B/op 0 allocs/op	`1.07`
`BenchmarkIsZero - ns/op`	`20.39` ns/op	`19.12` ns/op	`1.07`
`BenchmarkIsZero - B/op`	`0` B/op	`0` B/op	`1`
`BenchmarkIsZero - allocs/op`	`0` allocs/op	`0` allocs/op	`1`

This comment was automatically generated by workflow using github-action-benchmark.

Benchmarks vs main flagged ConvertInt +19% (8.5 -> 10.1 ns): the parseNativeInt/parseNativeUint wrappers added a call frame and tuple return around an ~8ns parse. Write the native-fit guard inline at each call site instead — `int64(int(v)) == v` folds away entirely on 64-bit and still rejects out-of-range values on 32-bit. Same-machine A/B (count=8): main 7.79 ns vs branch 7.77 ns — parity restored, and the 32-bit range check plus setBuiltinKind/builtinConverters parity tests still pass. The other two flagged deltas are not code regressions: isMultipartField and isZero are byte-identical to main (verified by diff), and the same-machine A/B shows parity (50.17 vs 49.88 ns; 16.10 vs 16.22 ns) — the CI deltas there are binary-layout/runner variance. Both are also off the decode hot path now (isMultipartField is precomputed per field into fieldInfo.isMultipart at cache build). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

gaby · 2026-07-19T13:52:56Z

Summary of improvements:

Decoding (biggest wins)

SimpleStructDecode: 5.3× faster (3181 → 601 ns), and 30 allocs → 0
LargeStructDecodeParallel: 6.2× faster (3495 → 565 ns), 62 → 4 allocs
LargeStructDecode: 4.9× faster (7014 → 1440 ns), 1200 → 144 B
CheckRequiredFields: 5.2× faster, 15 allocs → 0 (1048 B → 0)
DecoderMultipartFiles: 3.9× faster, 48 → 5 allocs, 94% less memory
TimeDurationDecoding: 2.6× faster, 8 → 2 allocs

Encoding

SimpleStructEncode: 1.8× faster, 5 → 3 allocs
LargeStructEncode: 2.3× faster, 12 → 6 allocs
TimeDurationEncoding: 2.1× faster, 2 → 1 alloc
Parallel variants track the same ~2× gains

Internals

ParseTag: 8.8× faster (49.4 → 5.6 ns), now zero-alloc
BenchmarkAll: 9.6× faster (33.2 → 3.5 ns), 1 alloc → 0

ReneWerner87

Impressive work — I went through the full diff and tried to break the trickier parts (the generation-counter cache design, the decodeBuiltinSlice sizing invariant, the encoder plan mapping) by tracing the race interleavings and running a few differential probes against main. I couldn't find a correctness issue, and the tests pin exactly the invariants the optimizations rely on. A few small observations, mostly about the PR description rather than the code:

Undocumented bugfix that's worth advertising: unexported embedded pointers. On main, any Decode into a struct that merely contains an unexported anonymous pointer field fails entirely with a recovered reflect: reflect.Value.Set using value obtained using unexported field — even with an empty src, and sibling fields don't decode either. With this branch such structs decode normally, and keys targeting fields behind the unsettable pointer are skipped (pinned by TestUnexportedEmbeddedPointerSkipped). This seems like one of the more impactful behavior improvements in the PR, so it might deserve a spot in the bug-fix list of the description.
Small description drift around ParseNativeInt/ParseNativeUint. The description says int/uint parsing now uses utils.ParseNativeInt/ParseNativeUint, but after 59b82ee the code uses utils.ParseInt/ParseUint with the inline native-fit guard (int64(int(v)) == v). Functionally equivalent — just flagging it so the description matches the final code.
Pre-existing, optional follow-up: Decode mutates the caller's src map when multipart files are passed. src[path] = []string{""} injects file keys into the caller's map and silently overwrites an existing caller-provided entry under the same key. Not introduced by this PR (verified identical on main), but since this code was already reshuffled for the nil-src fix, it could be a cheap opportunity to stop mutating caller state — happy to leave it for a separate PR though.
Tiny consistency nit: the decoder cache stores cacheEntry by value in its sync.Map while the encoder cache stores *encPlan by pointer. Both are correct (the value store only boxes on a cache miss); aligning the two would just make the pattern easier to follow.
Note on the generation guards (no action needed): I traced the interleavings, including a delayed stale store landing after the clear, and the user-facing guarantee holds in every case — any call that starts after SetAliasTag/RegisterEncoder returns observes the new configuration. A call already in flight during the reconfiguration can still briefly observe old metadata, which is inherent and fine; the comment wording ("must never be served once the reconfiguration returned") is marginally stronger than that, in case you want to tighten the phrasing while you're in there.

Also verified on my end: go.mod already pins gofiber/utils/v2 v2.2.0 on main, so the absence of module changes is correct, and CI is fully green. Nice numbers on the benchmarks — the allocation-free flat-struct decode in particular.

gaby · 2026-07-19T15:55:19Z

The PR description is out of date. Let me fix that

…lish Address maintainer review follow-ups: - Decode no longer mutates the caller's src map when multipart files are passed: file keys are merged into an internal copy, so the caller's map gains no keys and a caller-provided value under a file's key is not overwritten (pre-existing behavior on main; test added). The copy is taken only on the multipart path, so the common no-files decode is unchanged (still 0 allocs for flat structs). - Store decoder cache entries by pointer (*cacheEntry), matching the encoder's *encPlan pattern. - Tighten the generation-guard comments to state the actual guarantee: any call starting after a reconfiguration returns observes the new configuration; a call already in flight may briefly use old metadata, which is inherent to concurrent reconfiguration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

gaby · 2026-07-19T16:21:42Z

Thanks for the thorough review @ReneWerner87 — all five points are addressed (657f120 + description update):

Unexported embedded pointers — agreed it's one of the more impactful fixes; the description's bug-fix list now leads with it, using your framing (on main the entire Decode failed even with an empty src, siblings included).
ParseNativeInt drift — the description was updated alongside 59b82ee; it now describes the actual final code (utils.ParseInt/ParseUint + inline native-fit guard that compiles away on 64-bit).
src mutation with multipart files — took the cheap opportunity in this PR: file keys are now merged into an internal copy, so the caller's map gains no keys and caller-provided entries under a file's key stay intact. The copy is only taken on the multipart path, so the common no-files decode is unchanged (flat-struct decode still 0 allocs). Pinned by TestDecodeDoesNotMutateCallerSrc.
cacheEntry vs *encPlan — aligned; the decoder cache now stores *cacheEntry by pointer, matching the encoder pattern.
Generation-guard phrasing — comments tightened to state exactly the guarantee you traced: any call starting after the reconfiguration returns observes the new configuration; a call already in flight may briefly use old metadata, which is inherent.

Full suite still green under -race -shuffle=on, lint clean, coverage 99.4%.

Generated by Claude Code

claude added 8 commits July 17, 2026 14:31

chore: remove accidentally committed test binary and ignore *.test

12f1a4a

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TogBKWQrUdFqaPyboqUwSx

gaby requested a review from a team as a code owner July 17, 2026 23:58

gaby requested review from ReneWerner87, efectn and sixcolors and removed request for a team July 17, 2026 23:58

ReneWerner87 added the 🧹 Updates label Jul 17, 2026

gaby changed the title ~~perf: optimize decode/encode hot paths with SWAR utils and alloc removal~~ perf: rewrite decode/encode hot paths — zero-alloc decoding, SWAR utils, cached metadata Jul 17, 2026

gaby changed the title ~~perf: rewrite decode/encode hot paths — zero-alloc decoding, SWAR utils, cached metadata~~ perf: rewrite decode/encode hot paths with SWAR support Jul 18, 2026

gaby changed the title ~~perf: rewrite decode/encode hot paths with SWAR support~~ ⚡ perf: rewrite decode/encode hot paths with SWAR support Jul 18, 2026

coderabbitai Bot reviewed Jul 18, 2026

View reviewed changes

Comment thread .gitignore

Comment thread cache.go

Comment thread decoder.go

Comment thread encoder.go Outdated

claude added 4 commits July 18, 2026 00:10

coderabbitai Bot reviewed Jul 18, 2026

View reviewed changes

Comment thread decoder_test.go

Comment thread decoder_test.go Outdated

claude added 5 commits July 18, 2026 06:25

coderabbitai Bot reviewed Jul 18, 2026

View reviewed changes

Comment thread encoder_test.go

claude added 3 commits July 19, 2026 05:57

ReneWerner87 requested a review from Copilot July 19, 2026 12:38

ReneWerner87 approved these changes Jul 19, 2026

View reviewed changes

Copilot started reviewing on behalf of ReneWerner87 July 19, 2026 12:38 View session

Copilot AI reviewed Jul 19, 2026

View reviewed changes

Comment thread decoder.go Outdated

ReneWerner87 reviewed Jul 19, 2026

View reviewed changes

gaby changed the title ~~⚡ perf: rewrite decode/encode hot paths with SWAR support~~ perf: rewrite decode/encode hot paths — zero-alloc decoding, SWAR utils, cached metadata, panic-safety Jul 19, 2026

gaby changed the title ~~perf: rewrite decode/encode hot paths — zero-alloc decoding, SWAR utils, cached metadata, panic-safety~~ ⚡ perf: rewrite decode/encode hot paths — zero-alloc decoding, SWAR utils, cached metadata, panic-safety Jul 19, 2026

ReneWerner87 merged commit 9cc2149 into main Jul 19, 2026
19 checks passed

ReneWerner87 deleted the claude/schema-performance-optimization-kw0q51 branch July 19, 2026 16:40

ReneWerner87 changed the title ~~⚡ perf: rewrite decode/encode hot paths — zero-alloc decoding, SWAR utils, cached metadata, panic-safety~~ ⚡ perf: rewrite decode/encode hot paths - zero-alloc decoding, SWAR utils, cached metadata, panic-safety Jul 19, 2026

Uh oh!

Conversation

gaby commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Benchmark improvements

What changed

Decoder

Encoder

Caches & concurrency

Bug fixes (behavior improvements over main)

Panic-safety (previously crashed the calling process)

Review & verification

Uh oh!

codecov Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ReneWerner87 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

gaby commented Jul 19, 2026

Benchmark

Uh oh!

gaby commented Jul 19, 2026

Uh oh!

ReneWerner87 left a comment

Choose a reason for hiding this comment

Uh oh!

gaby commented Jul 19, 2026

Uh oh!

gaby commented Jul 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gaby commented Jul 17, 2026 •

edited

Loading

Bug fixes (behavior improvements over `main`)

codecov Bot commented Jul 17, 2026 •

edited

Loading

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading