fix: read the binary encoding in one call - #20
Open
sveitser wants to merge 2 commits into
Open
Conversation
0.1.1 fixed the write side to emit the blob via `serialize_bytes`, but the read side still went through serde's per-element `Vec<u8>` path, collecting a sequence one byte at a time. Decode was the more expensive direction: 4.63 ms vs 3.95 ms encode for a 4 MiB payload under bincode. Switch the binary branch to `serde_bytes::deserialize`, which requests the whole blob via `deserialize_byte_buf`: one allocation, one copy. At 4 MiB this is 73 us, a 63x improvement, and decode now matches encode. Wire format is unchanged for length-prefixed formats (bincode, postcard). Self-describing formats that distinguish byte strings from arrays (CBOR, MessagePack) now emit a byte string; reads still accept either. The docs claimed the binary branch used "the default Vec<u8> serialization", which stopped being true on the write side in 0.1.1 and is now false on both; update lib.rs and README.md accordingly. Note the allocation profile change on the `deserialize` rustdoc: the buffer is now sized from the format's length prefix before any bytes are read, so readers of untrusted streams must bound it themselves. Add a regression test using a deserializer that serves only `deserialize_bytes`/`deserialize_byte_buf` and errors otherwise, and a criterion benchmark comparing this crate against a plain `Vec<u8>` field across bincode and JSON at 1 KiB to 4 MiB.
Binary deserialization now sizes its buffer from the format's length prefix before reading, so an unbounded `bincode::deserialize_from` over a socket allocates whatever the peer claims: 12 bytes of input naming 4 GiB costs 4 GiB of RSS and 2 s, against 1.3 MB and 14 us on the old per-byte path, which serde capped at 1 MiB of preallocation via `size_hint::cautious`. There is no fix above the format layer. Both `deserialize_bytes` and `deserialize_byte_buf` route to bincode's `fill_buffer`, which resizes before `read_exact`; only the per-byte sequence path avoids it, and that is the 63x slower read this change exists to remove. bincode charges the length against its size limit before allocating (`de/mod.rs:93-97`), so a reader with a limit rejects the claim in ~1 us without allocating. Add a test pinning that, so the mitigation the docs point callers at cannot silently rot, and document the exposure in the README rather than only in the rustdoc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
0.1.1 fixed the write side to emit the blob via
serialize_bytes, but the read side still went through serde's per-elementVec<u8>path, collecting a sequence one byte at a time. Decode was the more expensive direction: 4.63 ms vs 3.95 ms encode for a 4 MiB payload under bincode.Switch the binary branch to
serde_bytes::deserialize, which requests the whole blob viadeserialize_byte_buf: one allocation, one copy. At 4 MiB this is 73 us, a 63x improvement, and decode now matches encode.Wire format is unchanged for length-prefixed formats (bincode, postcard). Self-describing formats that distinguish byte strings from arrays (CBOR, MessagePack) now emit a byte string; reads still accept either. The docs claimed the binary branch used "the default Vec serialization", which stopped being true on the write side in 0.1.1 and is now false on both; update lib.rs and README.md accordingly.
Note the allocation profile change on the
deserializerustdoc: the buffer is now sized from the format's length prefix before any bytes are read, so readers of untrusted streams must bound it themselves.Add a regression test using a deserializer that serves only
deserialize_bytes/deserialize_byte_bufand errors otherwise, and a criterion benchmark comparing this crate against a plainVec<u8>field across bincode and JSON at 1 KiB to 4 MiB.