ryankert01 opened a new pull request, #3708:
URL: https://github.com/apache/iggy/pull/3708
Closes #3706.
## Problem
`IggyMessageView::user_headers_map` copied the user-headers bytes twice per
message:
```rust
let wire = WireUserHeaders::from_slice(headers)?; // validate +
copy_from_slice(WHOLE blob)
let map = user_headers_from_wire(&wire)?; // then copy every key
and value again
```
`WireUserHeaders::from_slice` is `validate_user_headers(buf)?` followed by
`Bytes::copy_from_slice(buf)`. The view already holds the bytes as a borrowed
`&[u8]`, and the decoder only ever iterates them, so that whole-blob copy was
pure overhead, up to `MAX_USER_HEADERS_SIZE` (100 KB) per message.
## Change
The view now validates in place and decodes straight from the borrowed slice:
```rust
if let Err(e) = validate_user_headers(headers) { warn!(...); return
Ok(None); }
let map = user_headers_from_validated_slice(headers)?;
```
`user_headers_from_validated_slice` is new and holds the decode loop;
`user_headers_from_wire` delegates to it via `wire.as_bytes()`, so there is a
single implementation. Both `validate_user_headers` and
`WireUserHeaderIterator` were already public in `iggy_binary_protocol`, so this
adds no new surface to the wire crate.
## Correctness
**Error partitioning is preserved exactly.** Structural failures still
return `Ok(None)` with the same warning; unknown kind codes still return `Err`.
This holds because `from_slice` *is* `validate_user_headers` plus a copy, so no
input can pass one check and fail the other. The only thing dropped is the copy.
**No retention or lifetime change.** Keys and values still go through
`new_unchecked`, which uses `Bytes::copy_from_slice`, not `slice_ref`. The
returned map does not keep the parent message buffer alive. A `slice_ref`
variant would have been faster still, but it would pin up to 100 KB behind any
single retained header, so it was deliberately not taken.
`user_headers_from_validated_slice` is `pub(crate)` on purpose. It relies on
prior validation, and the underlying iterator slices TLV fields without bounds
checks, so misuse panics rather than truncating. Keeping it crate-private means
that footgun never reaches the public API; the precondition is documented under
`# Panics`.
Five tests added, covering round-trip through a serialized message and view,
a message with no headers, a structurally malformed blob, an unknown kind code,
and equivalence between decoding a borrowed slice and an independently copied
buffer.
## Measurement
Criterion microbenchmark over `user_headers_map`, two runs (p < 0.05 on
every case):
| case | run 1 | run 2 |
| --- | --- | --- |
| 1 hdr x 16 B | -22.6% | -21.7% |
| 4 hdr x 200 B | -17.6% | -16.0% |
| 4 hdr x 16 B | -6.4% | -5.1% |
| 16 hdr x 64 B | -4.1% | -2.9% |
| 32 hdr x 200 B | -5.1% | -3.0% |
The full range is shown deliberately. The win tracks total header **bytes**
(the memcpy removed) and is diluted by header **count**, since the per-field
allocations are untouched. It is strong in the common few-headers case and
modest at high header counts.
These are microbenchmarks; in production this sits behind network and disk
I/O. The harness is not included here (it would introduce criterion, the first
microbenchmark framework in the repo) and can be a separate discussion.
## 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]
