Skip to content

Fix nocopy values decoded from streams#26

Merged
kelindar merged 2 commits into
masterfrom
codex/fix-nocopy-stream-alias
Jul 25, 2026
Merged

Fix nocopy values decoded from streams#26
kelindar merged 2 commits into
masterfrom
codex/fix-nocopy-stream-alias

Conversation

@kelindar

@kelindar kelindar commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • allocate stable slices when decoding from generic streams
  • add a regression test for adjacent short nocopy.String fields

Root cause

streamReader.Slice reused one 10-byte scratch buffer. nocopy.String and other retained nocopy codecs alias the returned slice, so decoding the next short value overwrote earlier values. Generic streams have no reusable backing buffer, so each retained slice must own its bytes. Slice-backed Unmarshal remains zero-copy.

Reproduction

Before the fix, decoding {First: "first", Second: "second"} through NewDecoder(bytes.NewReader(...)) produced {First: "secon", Second: "second"}.

Benchmarks

Apple M2, Go 1.25, 10 runs for the targeted benchmark. The pre-fix result is fast because it aliases mutable scratch memory and is incorrect.

benchmark master fix delta
two short nocopy.String values via stream 39.42 ns/op, 0 B, 0 allocs 53.32 ns/op, 16 B, 2 allocs +35.3%, +2 required allocations
repository binary/stream-dec 132.6 ns/op, 1 alloc 138.2 ns/op, 2 allocs +4.2%, +1 allocation
slice-backed nocopy/str-dec 32.0 ns/op, 0 allocs 31.9 ns/op, 0 allocs unchanged

Full repository benchmark suite: cd bench && go run . (100 samples, 10 ms/sample).

Checks

  • go test ./...
  • go vet ./...
  • git diff --check

@github-actions

Copy link
Copy Markdown
100.0%

passed

████████████████████100.0% on changed lines

1 lines changed   0 uncovered   1 file

 📂 1 file changed
File Coverage Uncovered Lines
🟢 reader.go 100.0%
 📋 Full diff-cover report

Diff Coverage

Diff: origin/master...HEAD, staged and unstaged changes

  • reader.go (100%)

Summary

  • Total: 1 line
  • Missing: 0 lines
  • Coverage: 100%

🛡️ diff-cover-action

@kelindar
kelindar marked this pull request as ready for review July 25, 2026 21:47
@kelindar
kelindar merged commit 5786e46 into master Jul 25, 2026
1 check passed
@kelindar
kelindar deleted the codex/fix-nocopy-stream-alias branch July 25, 2026 21:47
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.

1 participant