perf: read fixed-size values and map keys without copying

[lalinsky]

Follow-up to #18, which did the same for the encode path.

Problem

Decoding read every fixed-size value into a stack array via readSliceAll, and every map key into a [256]u8 stack buffer, before looking at either. Both route through compiler_rt's generic memcpy. Decoding a 4-field struct made 7 memcpy calls to move chunks of 2 to 8 bytes, plus 4 calls to unpackStringHeader to parse what is almost always a single fixstr byte.

Changes

• Fixed-size values read via Reader.takeInt, which is pub inline and loads straight out of the reader's buffer. This replaces readSliceAll into a stack array in unpackIntValue and readFloatValue.
• unpackStringBorrowed returns a map key as a slice of the reader's buffer instead of a copy. A key is compared and discarded, so it never outlives the next read. Its fast path resolves a buffered fixstr without parsing a header at all, which covers every field name of 31 characters or fewer — i.e. essentially all of them, since fixstr encodes lengths 0–31.
• eqlLiteral compares a key against a comptime-known field name. The length test is against a constant and the byte compare that follows has a comptime length, so it lowers to inline compares rather than a call to mem.eql.
• unpackUnionAsMap and unpackUnionAsTagged now take comptime opts, matching what packUnionAsMap/packUnionAsTagged already needed in perf: write fixed-size values directly into the writer buffer #18, so their key names reach eqlLiteral.
• The [256]u8 key buffers in packStructAsMap and both union decoders are gone. Nothing copies a key anymore.

A panic this introduced, and the fix

Reader.takeInt and Reader.take both go fill → fillUnbuffered → rebase, and defaultRebase ends with:

assert(r.buffer.len - r.seek >= capacity);

So a reader whose buffer is smaller than the value being decoded panics (Debug/ReleaseSafe) or hits UB (ReleaseFast). A network stream can easily be configured that way. The old readSliceAll never had this problem, because it drains the buffered bytes and then reads directly into the destination slice, needing no reader capacity at all — so the hazard arrived with this change.

Rather than copy as a fallback, both sites check capacity and return error.ReaderBufferTooSmall, marked @branchHint(.unlikely). error.ReaderBufferTooSmall is a new error in the decode error set, which is user-visible.

The guards cost ~6 instructions per struct decode. Note that utils.takeInt must be inline: std.Io.Reader.takeInt is itself pub inline, and wrapping it in an ordinary function reintroduced the very call frame std was avoiding — ReleaseSafe decode went from 574 to 669 Ir/op until I marked it.

Tests

Three new tests in src/msgpack.zig, built on a TrickleReader that serves one byte per fill from a caller-sized buffer. This matters: every existing test uses Reader.fixed, whose buffer is the whole payload, so it is always large enough and no value ever straddles a fill. That is exactly why the panic was invisible to the suite.

• decodes correctly at buffer sizes 8, 9, 16, 64 (8 is the largest fixed-size payload, and forces every key and value to straddle a fill, so the fast paths miss and the slow paths run)
• buffer sizes 1, 2, 4, 7 return error.ReaderBufferTooSmall rather than panicking
• a 33-character field name encodes as str8 rather than fixstr; a 16-byte reader buffer errors, a 64-byte one decodes. This also covers the str8 key path in packStringLiteral, which nothing else exercised.

I verified the tests fail without the fix: with the guards removed, the too-small-buffer test aborts with SIGABRT inside defaultRebase.

Results

Instructions per operation, callgrind (deterministic). Encode is untouched at 184/249.

	before	after	msgpuck (-O3)
decode numeric, ReleaseFast	708	362	168
decode numeric, ReleaseSafe	1009	576	190
decode message, ReleaseFast	815	451	258
decode message, ReleaseSafe	1098	823	291

The decode path for the numeric struct now issues no memcpy calls. The gap to msgpuck narrows from 4.2× to 2.2× on the numeric struct, and to 1.7× on the message struct, where msgpuck is doing the same two string copies.

Verified against a harness that decodes the identical payload with msgpack.zig and msgpuck in one process, asserting the encoders agree byte-for-byte before measuring. Wall-clock numbers are omitted: the machine drifted ~20% between runs on unmodified code. Instruction counts reproduce exactly.

Things considered and rejected

• std.StaticStringMap for key dispatch. It stores keys as runtime []const u8 in a table and compares them byte-at-a-time with a runtime length. The unrolled eqlLiteral compare is strictly less work for a comptime-known field list. Its length-bucketing idea would matter for very wide structs, where a comptime switch on the fixstr header byte (which is 0xa0 + len) would give the same bucketing with no runtime table.
• Hand-rolled takeArray/takeSlice helpers. Measured: takeArray was 84 Ir/op worse than std.Io.Reader.takeInt on the message decode, and takeSlice made no difference against plain reader.take. Both deleted.

Summary by CodeRabbit

• New Features

  • Added borrowed string decoding to reduce copying and temporary buffer usage.
  • Exported the borrowed string decoder for direct use.
  • Improved struct and union decoding for string-based keys and tags.

• Bug Fixes

  • Streaming decoding now handles values split across reader fills.
  • Returns ReaderBufferTooSmall safely when buffers cannot hold decoded values, without panicking.
  • Improved integer and floating-point decoding across reader boundaries.

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes add buffer-safe integer decoding, borrowed string unpacking, and shared literal comparison helpers. Struct and union key decoding adopt borrowed strings, union options become comptime parameters, and streaming tests cover trickle reads and undersized buffers.

Changes

Reader decoding and key matching

Layer / File(s)	Summary
Shared reader helpers src/utils.zig, src/int.zig, src/float.zig	Adds safe big-endian integer extraction and literal comparison helpers, then uses takeInt for integer and float payload decoding.
Borrowed string decoding src/string.zig, src/msgpack.zig	Adds unpackStringBorrowed, validates reader capacity, supports a fixstr fast path, and re-exports the function.
Struct and union key matching src/struct.zig, src/union.zig	Uses borrowed field names for exact and prefix matching, removes fixed key buffers, and makes union options and format selection compile-time decisions.
Streaming decode validation src/msgpack.zig	Adds a one-byte-fill reader and tests successful straddling reads plus ReaderBufferTooSmall errors for undersized buffers and map keys.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Reader
  participant unpackStringBorrowed
  participant StructOrUnionDecoder
  Reader->>unpackStringBorrowed: provide buffered string payload
  unpackStringBorrowed->>StructOrUnionDecoder: return borrowed field-name slice
  StructOrUnionDecoder->>StructOrUnionDecoder: perform literal or prefix match

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main performance change: avoiding copies when reading fixed-size values and map keys.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf/decode-borrow-keys

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

src/union.zig (1)

239-246: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider combining identical switch arms.

The .field_name and .field_name_prefix arms in the opts.tag_value switch are identical — both call unpackStringBorrowed and store to union_field_name. The prefix payload is not needed here (it's used later in the is_match switch), so the arms can be merged.

♻️ Proposed refactor

         .field_name => {
             const field_name = try unpackStringBorrowed(reader);
             union_field_name = field_name;
         },
-        .field_name_prefix => {
-            const field_name = try unpackStringBorrowed(reader);
-            union_field_name = field_name;
-        },
+        .field_name_prefix => {
+            const field_name = try unpackStringBorrowed(reader);
+            union_field_name = field_name;
+        },

Or combined:

-        .field_name => {
-            const field_name = try unpackStringBorrowed(reader);
-            union_field_name = field_name;
-        },
-        .field_name_prefix => {
-            const field_name = try unpackStringBorrowed(reader);
-            union_field_name = field_name;
-        },
+        .field_name, .field_name_prefix => {
+            const field_name = try unpackStringBorrowed(reader);
+            union_field_name = field_name;
+        },

🤖 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 `@src/union.zig` around lines 239 - 246, In the opts.tag_value switch, merge
the identical .field_name and .field_name_prefix arms into a single switch arm
that calls unpackStringBorrowed(reader) and assigns the result to
union_field_name; retain the field_name_prefix value handling in the later
is_match switch.

🤖 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.

Nitpick comments:
In `@src/union.zig`:
- Around line 239-246: In the opts.tag_value switch, merge the identical
.field_name and .field_name_prefix arms into a single switch arm that calls
unpackStringBorrowed(reader) and assigns the result to union_field_name; retain
the field_name_prefix value handling in the later is_match switch.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b86ca445-da92-4b7a-a93b-c3010a392a7a

📥 Commits

Reviewing files that changed from the base of the PR and between c7b0dc8 and 032a380.

📒 Files selected for processing (7)

• src/float.zig
• src/int.zig
• src/msgpack.zig
• src/string.zig
• src/struct.zig
• src/union.zig
• src/utils.zig