From 25cf8d9b45623e040a8b774a80a7304eca3e99b5 Mon Sep 17 00:00:00 2001 From: Lukas Lalinsky Date: Sat, 5 Sep 2026 06:16:13 +0000 Subject: [PATCH] perf: copy string and binary values out of the reader buffer 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 and that machinery is never needed. 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. Measured with callgrind, instructions per decode: case old new event 1746.74 1621.74 -7.2% struct with 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 saving is a fixed amount per string value rather than a proportion, so it shows up largest where strings are short and numerous, and washes out when a single large memcpy dominates -- which is why `long` moves so much between the two builds. The control stays flat in both, which is what makes the rest readable. --- CHANGELOG.md | 1 + src/binary.zig | 5 +++-- src/string.zig | 5 +++-- src/utils.zig | 38 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5703301..f60bb0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Decoding an optional is about 18% cheaper, by letting each type's own unpacker recognise a nil header instead of testing for one beforehand +- Decoding string and binary values copies straight out of the reader's buffer when the value is already buffered, which is 3-7% cheaper on string-heavy messages ### Removed - `omit_nulls` and `omit_defaults` on `UnionAsMapOptions`, which were never read; a union-as-map always writes exactly one entry, so there is nothing to omit diff --git a/src/binary.zig b/src/binary.zig index b1673f1..959dcf3 100644 --- a/src/binary.zig +++ b/src/binary.zig @@ -9,6 +9,7 @@ const maybeUnpackNull = @import("null.zig").maybeUnpackNull; const packHeaderAndInt = @import("utils.zig").packHeaderAndInt; const unpackIntValue = @import("int.zig").unpackIntValue; +const readSliceFast = @import("utils.zig").readSliceFast; const unpackShortIntValue = @import("int.zig").unpackShortIntValue; pub fn sizeOfPackedBinaryHeader(len: usize) !usize { @@ -62,7 +63,7 @@ pub fn unpackBinary(reader: *std.Io.Reader, allocator: std.mem.Allocator) ![]u8 const data = try allocator.alloc(u8, len); errdefer allocator.free(data); - try reader.readSliceAll(data); + try readSliceFast(reader, data); return data; } @@ -74,7 +75,7 @@ pub fn unpackBinaryInto(reader: *std.Io.Reader, buf: []u8) ![]u8 { } const data = buf[0..len]; - try reader.readSliceAll(data); + try readSliceFast(reader, data); return data; } diff --git a/src/string.zig b/src/string.zig index baa1202..5e9c470 100644 --- a/src/string.zig +++ b/src/string.zig @@ -9,6 +9,7 @@ const maybeUnpackNull = @import("null.zig").maybeUnpackNull; const packHeaderAndInt = @import("utils.zig").packHeaderAndInt; const reserveArray = @import("utils.zig").reserveArray; +const readSliceFast = @import("utils.zig").readSliceFast; const unpackIntValue = @import("int.zig").unpackIntValue; const unpackShortIntValue = @import("int.zig").unpackShortIntValue; @@ -103,7 +104,7 @@ pub fn unpackStringValue(reader: *std.Io.Reader, allocator: std.mem.Allocator, c const data = try allocator.alloc(u8, len); errdefer allocator.free(data); - try reader.readSliceAll(data); + try readSliceFast(reader, data); return data; } @@ -148,7 +149,7 @@ pub fn unpackStringInto(reader: *std.Io.Reader, buf: []u8) ![]u8 { } const data = buf[0..len]; - try reader.readSliceAll(data); + try readSliceFast(reader, data); return data; } diff --git a/src/utils.zig b/src/utils.zig index 552d78b..241c386 100644 --- a/src/utils.zig +++ b/src/utils.zig @@ -46,6 +46,21 @@ pub inline fn takeInt(reader: *std.Io.Reader, comptime T: type) !T { return reader.takeInt(T, .big); } +/// Copies `dest.len` bytes out of the reader. When they are already buffered, +/// which is always the case when decoding from a slice, this is a plain memcpy +/// and skips `readSliceAll`'s short-read loop. Falls back to it otherwise, so a +/// streaming reader whose buffer cannot hold the value still works. +pub inline fn readSliceFast(reader: *std.Io.Reader, dest: []u8) !void { + const buffered = reader.buffered(); + if (buffered.len >= dest.len) { + @branchHint(.likely); + @memcpy(dest, buffered[0..dest.len]); + reader.toss(dest.len); + return; + } + return reader.readSliceAll(dest); +} + /// Compares `value` against a comptime-known `name`. The length test is a /// compare against a constant, and the byte compare that follows has a /// comptime-known length, so it lowers to inline compares rather than a call. @@ -93,3 +108,26 @@ pub const NoAllocator = struct { }; } }; + +test "readSliceFast: copies straight out of the buffer when fully buffered" { + const data = [_]u8{ 1, 2, 3, 4, 5, 6, 7, 8 }; + var reader = std.Io.Reader.fixed(&data); + + var dest: [5]u8 = undefined; + try readSliceFast(&reader, &dest); + try std.testing.expectEqualSlices(u8, data[0..5], &dest); + // The bytes must be consumed, not just peeked. + try std.testing.expectEqualSlices(u8, data[5..], reader.buffered()); +} + +test "readSliceFast: falls back when the value is not fully buffered" { + const data = [_]u8{ 1, 2, 3, 4, 5, 6, 7, 8 }; + var reader = std.Io.Reader.fixed(&data); + // Pretend only half the value has arrived so far. + reader.end = 4; + + var dest: [8]u8 = undefined; + // A fixed reader cannot refill, so taking the fallback surfaces + // EndOfStream. Taking the fast path here would instead read past `end`. + try std.testing.expectError(error.EndOfStream, readSliceFast(&reader, &dest)); +}