perf: copy string and binary values out of the reader buffer

[lalinsky]

Fixes #32.

unpackString and unpackBinary filled their allocation with readSliceAll, which goes through readSliceShort and loops to handle partial reads. On the decodeFromSlice path the reader is a Reader.fixed whose buffer holds the whole message, so the bytes are always contiguous and present — that machinery never does anything.

readSliceFast memcpys straight out of the buffer when the value is fully buffered, and falls back to readSliceAll otherwise so a streaming reader whose buffer cannot hold the value still works. Four call sites: the allocating and into-buffer forms of both string and binary.

Measurement

Callgrind, instructions per decode, one library version per binary:

case	old	new
event	1746.74	1621.74	−7.2%	struct, 7 short strings
strings	5138.68	4914.68	−4.4%	array of 32 strings
long	461.29	424.29	−8.0%	one 2000-byte string
plain	519.01	517.01	−0.4%	no strings, control

Repeated at ReleaseSafe: −6.7%, −2.7%, −0.2%, +0.4%. The control stays flat in both, which is what makes the rest readable.

The saving is a fixed amount per string value, not a proportion — roughly 8–20 instructions of loop setup. So it shows up largest where strings are short and numerous, and washes out when one large memcpy dominates the cost. That's why long moves so much between the two builds: the same absolute saving against a 461-instruction baseline in ReleaseFast and a 3345-instruction one in ReleaseSafe.

Smaller than #38's 18%, but consistent in direction across builds with flat controls.

Tests

Two unit tests on readSliceFast — one for the fast path (checks the bytes are consumed, not just peeked) and one that forces the fallback by shortening the reader's end, since a fixed reader cannot refill and therefore surfaces EndOfStream if and only if the fallback was taken.

One thing I could not test

I wanted an end-to-end test decoding a string larger than a streaming reader's buffer. TrickleReader, the existing helper for that, hangs forever on it: its readVec ignores the data vectors it is handed and returns 0 once its own buffer fills, so readSliceAll into a larger destination spins. Its doc comment claims to model "the reader's buffer can be smaller than the value being decoded", and the existing tests only ever use it below that threshold or to assert an error.

That is pre-existing and unrelated to this change, but it is a live trap for the next person who writes that test. Filing separately.

zig build test: 185/185 pass, up from 183.

[coderabbitai]

Warning

Review limit reached

Next included review available in 5 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: c58fe149-4f8c-4656-8728-16116a9ba69f

📥 Commits

Reviewing files that changed from the base of the PR and between 52b624d and 25cf8d9.

📒 Files selected for processing (4)

• CHANGELOG.md
• src/binary.zig
• src/string.zig
• src/utils.zig

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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