ryankert01 opened a new pull request, #3707:
URL: https://github.com/apache/iggy/pull/3707

   Closes #3705.
   
   ## Problem
   
   `IggyMessage::write_to_buffer` serialized the fixed 64-byte message header 
by allocating a throwaway `Bytes` and immediately copying it away:
   
   ```rust
   buf.put_slice(&self.header.to_bytes());   // malloc 64B -> memcpy -> free
   ```
   
   `IggyMessageHeader::to_bytes` builds a `BytesMut`, writes 8 fields and 
freezes it. So every message paid a malloc, a refcount-block allocation, a 
64-byte memcpy and a free, purely to move bytes into a buffer that was already 
sized to receive them.
   
   The caller is a per-message loop: `IggyMessagesBatch::from(&[IggyMessage])` 
(`messages_batch.rs:309`) iterates every message in the batch. 
`IggyMessage::to_bytes` had the same pattern.
   
   ## Change
   
   Added `IggyMessageHeader::write_to(&self, buf: &mut BytesMut)`, which 
appends the 8 fields directly into a caller-supplied buffer. `to_bytes` now 
delegates to it, so encoder and decoder cannot drift apart. Both call sites in 
`iggy_message.rs` use it.
   
   Both `write_to_buffer` and `write_to` reserve their full output up front. 
This matters twice over: it keeps an unsized destination buffer to a single 
growth (appending eight fields individually would otherwise realloc 
repeatedly), and giving the writes a proven capacity removes the per-field 
capacity checks in the preallocated batch path. The second effect turned out to 
dominate.
   
   `write_to` follows existing precedent in the tree: 
`core/partitions/src/iggy_index.rs:37` has `fn write_to(&self, buffer: &mut 
Vec<u8>)`.
   
   ## Correctness
   
   Byte-identical output: same field order, same little-endian encoding, 
cross-checked against the `IGGY_MESSAGE_*_OFFSET_RANGE` constants and the 
decode order in both `from_bytes` and `from_raw_bytes`. No wire-format change, 
no `#[repr(C)]` type touched, `IGGY_MESSAGE_HEADER_SIZE` unchanged at 64. 
`to_bytes` is retained and still used by `binary_protocol` and `server`.
   
   Four tests added. The equivalence test deliberately builds its expected 
bytes with `to_le_bytes` rather than comparing against `to_bytes`, since 
`to_bytes` now shares `write_to`'s implementation and so cannot serve as an 
oracle for it. Two further tests pin the buffer-growth behaviour for an empty 
destination and for a buffer with existing content.
   
   ## Allocation behaviour
   
   Measured with a counting global allocator, as (allocations, reallocations) 
per `write_to_buffer` call:
   
   | payload | master | this PR |
   | --- | --- | --- |
   | 1 B, empty destination | (2,1) | **(1,0)** |
   | 64 B, empty destination | (2,1) | **(1,0)** |
   | 1024 B, empty destination | (2,1) | **(1,0)** |
   | any, preallocated destination | (1,0) | **(0,0)** |
   
   Better than master on both paths: the public unsized-buffer case drops from 
two allocations plus a realloc to a single allocation, and the in-tree batch 
path becomes allocation-free.
   
   ## Measurement
   
   Criterion microbenchmark over batch assembly, versus master, on an otherwise 
idle machine (p < 0.05 on every case, reproducible across runs to within 0.02%):
   
   | case | change |
   | --- | --- |
   | 100 msg x 64 B | -82.7% |
   | 100 msg x 1024 B | -57.8% |
   | 1000 msg x 256 B | -78.5% |
   
   Most of this comes from the elided per-field capacity checks rather than the 
removed allocation; an earlier revision of this PR without the up-front reserve 
measured only -17% to -22%.
   
   These are microbenchmarks. In production this path sits behind network and 
disk I/O, so the end-to-end effect will be smaller. The benchmark harness is 
not included in this PR; it introduces criterion, which would be the first 
microbenchmark framework in the repo and is a separate discussion. Happy to 
share it or open it as its own PR.
   
   ## Verification
   
   `cargo test -p iggy_common --all-features`, `cargo clippy -p iggy_common 
--all-features --all-targets -- -D warnings`, and `cargo check --workspace 
--all-features --all-targets` all pass. `cargo fmt --all` applied.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to