Repository navigation
Inline BytesMut's specialized BufMut methods and add put_u8 fast paths - #850
Conversation
|
I was a bit surprised, personally, about the I'm admittedly a bit out of my depth here, but the assembly looks like this at Graviton4 (c8g, 253 → 92 ns) pays the forwarding round-trip every byte. Granite Rapids (c8i, 66 → 66 ns) apparently predicts and forwards the same-address store+load at near-zero cost. The new assembly (with this change) is streamlined and performs well on both architectures. |
…nthropics#437) **`encode_to_bytes` was 3–4× slower than `encode_to_vec` on every benchmark shape, with or without LTO; it is now within noise of it.** Pure performance change; no behavioural or wire impact. ## Why `encode_to_bytes` built a `BytesMut::with_capacity(size)`, wrote the message through it, and froze it. `bytes` does not mark `<BytesMut as BufMut>::put_slice` `#[inline]` (it does for `Vec<u8>`), `BytesMut` has no `put_u8` override, and LLVM folds `reserve_inner` into `put_slice`, so it stays out of line even under fat LTO. Every tag and varint byte our encoders write through `put_u8` therefore became a call, a reserve check, a one-byte libc `memcpy`, and an `advance_mut` re-check; through `Vec<u8>` the same write is a compare and a store. The doc comment already said `encode_to_bytes` is "equivalent to `Bytes::from(self.encode_to_vec())`" — this makes the implementation literally that. The upstream fix is proposed separately in tokio-rs/bytes#850; this change is worthwhile regardless, since buffa cannot make its users take a newer `bytes`, and the `Vec` path is never slower. ## Change - `Message::{encode_to_bytes, try_encode_to_bytes}`, `ViewEncode::{encode_to_bytes, try_encode_to_bytes}` and the generated lazy-view inherent methods delegate to their `Vec` twins and convert with `Bytes::from`. Output bytes and the returned `Bytes` representation are unchanged: `BytesMut::freeze` on a vec-backed buffer already went through `From<Vec<u8>> for Bytes`, and with `len == capacity` (guaranteed by the two-pass size) that conversion allocates nothing. - `Rope`'s tail writes go through the inherent, inlined `BytesMut::extend_from_slice` instead of its `BufMut` impl; `to_contiguous_bytes` builds a `Vec`. - New `benchmarks/buffa/benches/encode_sink.rs` compares the sinks — `encode_to_vec` vs `encode_to_bytes`, encode into a pre-sized `Vec` vs `BytesMut`, and "frame a header then encode into your own `BytesMut`" vs "frame then `put_slice(encode_to_vec())`" — on `log_record`, `api_response`, `google_message1` and the `log_record` view. A `bench-nolto` profile and `task bench-encode-sink` run it at both the LTO bench profile and the profile a downstream `cargo build --release` gets. ## Results `encode_to_bytes`, `main` → this change, criterion median per dataset batch, one pinned core (no-LTO / fat-LTO). Development machine (Xeon 8488C): | shape | main | this change | `encode_to_vec` (reference) | |---|---|---|---| | log_record | 21.4 / 23.3 µs | 6.46 / 6.04 µs | 6.1 / 5.9 µs | | api_response | 8.31 / 8.15 µs | 2.17 / 1.97 µs | 2.0 / 1.9 µs | | google_message1 | 257 / 264 ns | 64 / 64 ns | 61 / 60 ns | | log_record (view) | 20.9 / 20.4 µs | 5.62 / 5.51 µs | 5.5 / 5.4 µs | Clean EC2 instances (Amazon Linux 2023, rustc 1.95.0): | shape | c8i.xlarge (Xeon 6975P-C) | c8g.xlarge (Graviton4) | |---|---|---| | log_record | 16.8 / 18.4 µs → 5.83 / 5.62 µs | 24.7 / 24.4 µs → 8.84 / 8.16 µs | | api_response | 6.21 / 7.01 µs → 1.98 / 1.87 µs | 8.01 / 8.44 µs → 2.94 / 2.66 µs | | google_message1 | 199 / 216 ns → 60 / 64 ns | 305 / 309 ns → 101 / 95 ns | | log_record (view) | 16.0 / 17.5 µs → 5.28 / 4.83 µs | 23.3 / 23.0 µs → 7.74 / 7.12 µs | `encode_to_vec` on the same EC2 runs: 5.4–5.6 / 8.0–8.6 µs, 1.8–2.0 / 2.6–2.8 µs, 58–63 / 89–98 ns — so `encode_to_bytes` is now within ~5 % of it on both architectures, and LTO did not rescue the old path on either. Callers that `encode` straight into their own `BytesMut` still pay the slow sink until `bytes` changes; the bench's last two rows show that encoding to a `Vec` and appending it with one `put_slice` is ~2.7× faster even counting the copy (log_record: 21.9 µs vs 8.0 µs). ## Compatibility Patch-level. No API or wire change; regenerated lazy-view code is source-compatible (method signatures unchanged) and previously generated code keeps working against the new runtime. ## Testing `cargo test -p buffa -p buffa-codegen -p buffa-test`, `cargo clippy -p buffa -p buffa-codegen --all-targets -- -D warnings`, `cargo fmt --check`. The existing `encode_to_bytes_over_limit_panics` and two-pass ledger tests cover the delegated paths; benchmarks as above. ## Follow-ups (not in this PR) - `encode_to_vec` can write through `Vec::spare_capacity_mut()` (`&mut [MaybeUninit<u8>]` implements `BufMut`) and `set_len` once, since the size is exact: measured 1.25× (varint-heavy) to 3.2× (string-heavy) over the current `Vec` path with generated code unchanged, for one line of `unsafe`. Separate PR with its own safety argument. - `Rope`'s tail as a `Vec<u8>`: a further ~1.3–1.5× on byte-heavy tails, at ≤2 small allocations per flushed segment. --------- Co-authored-by: Iain McGinniss <[email protected]>
|
@seanmonstar any advice on whether this type of optimization PR is worth pursuing, and if so how to find a good reviewer for it? |
… sizing (connectrpc#300) Three small buffer-management fixes on the request/response path, plus a bench that isolates them from HTTP. Additive API (one new `Envelope` fn); no wire change. Reviewed with the repo's `rust-code-reviewer` and `rust-api-ergonomics-reviewer` agents; their Medium findings (keep `decode*` signatures concrete, changelog accuracy) are applied. ## What and why - **`encode_view_body` / `encode_view_body_with_min_segment` write through a `Vec<u8>`, not a `BytesMut`.** These size their own buffer and encode the view into it (the path every generated `impl Encodable<M> for MView` takes). `bytes` does not inline `<BytesMut as BufMut>::put_slice`, so every tag and varint byte written through a `BytesMut` is an out-of-line call; a pre-sized `Vec<u8>` + `Bytes::from` is zero-copy, allocation-free at `len == capacity`, and 3–4× faster. Owned-message paths (`Encodable for M`, `PreEncoded`, client request encoding) call buffa's `encode_to_bytes`, which buffa ≥ 0.9.3 already routes through a `Vec` (anthropics/buffa#437), so they're left alone — verified below. Upstream `bytes` fix: tokio-rs/bytes#850. - **New `Envelope::decode_bytes_with_limit(&mut Bytes, max)`** — `decode_with_limit` for a body already held as `Bytes`; the payload is split off by reference count (`Buf::copy_to_bytes` is zero-copy on `Bytes`) rather than the body first being copied into a `BytesMut`. The three places that did that copy just to de-frame — the gRPC unary and server-streaming handlers on the server, and the Connect client-streaming response scan on the client — use it: one allocation and one body-sized memcpy less per message. `decode` / `decode_with_limit` keep their `&mut BytesMut` signatures and now share a private body with the new function. - **gzip `compress_inner` no longer doubles its pre-sized output buffer** before the first write; it grows only when spare capacity is exhausted (3 allocations / ~3× input bytes → 2 / ~1× per compressed response). - **`benches/rpc/benches/encode_frame.rs`**: `view_encode/*` (view → `Bytes` via buffa's two entry points and `encode_view_body`) and `deframe/*` (`decode_with_limit` on a `BytesMut` copy vs `decode_bytes_with_limit` on the `Bytes`), over a 346 B, 4.4 KB and 271 KB log-record message and a 256 KB four-string blob. ## Numbers criterion medians, one pinned core (Xeon 8488C), `main` → this branch: | bench | main | this branch | |---|---|---| | `view_encode/small_300B/connectrpc_encode_view_body` | 628 ns | 166 ns | | `view_encode/logs_4KB/connectrpc_encode_view_body` | 4.56 µs | 1.21 µs | | `view_encode/logs_256KB/connectrpc_encode_view_body` | 264 µs | 75.4 µs | | `view_encode/blob_256KB/connectrpc_encode_view_body` (memcpy-bound) | 5.33 µs | 5.59 µs | | `deframe/small_300B` copy → in place | 50 ns | 25 ns | | `deframe/logs_4KB` copy → in place | 103 ns | 25 ns | | `deframe/logs_256KB` copy → in place | 5.42 µs | 25 ns | | gzip, 256 KB text (allocs / bytes allocated, one-off count) | 3 / 792 KB | 2 / 265 KB | End-to-end (`rpc_bench`, loopback, ~70 µs floor): unary −1…−8 %, gRPC server-streaming −6 %, other arms within noise — these are per-message constant costs, visible as CPU more than latency. Why the owned paths are not touched: on `main` with only the buffa dependency swapped (0.9.2 → 0.9.2 + anthropics/buffa#437), `Encodable::encode` goes 647 → 184 ns, 4.87 → 1.38 µs, 282 → 86 µs (= the `Vec` reference), while `encode_view_body` stays at 563 ns / 4.16 µs / 237 µs — the buffa bump fixes everything except the sites changed here. ## Testing `cargo test --workspace` (937 passed), `cargo test -p connectrpc --no-default-features` (514), `cargo clippy --workspace --all-features --all-targets -- -D warnings`, `cargo fmt --check`; conformance server and client suites pass. New unit test that `decode_bytes_with_limit` aliases its input, leaves incomplete input untouched, and rejects over-limit frames. ## Follow-ups (not here) - Client request framing without the copy (fused `Vec` when uncompressed, or header + payload as two body frames ≥ 16 KiB): saves one alloc + memcpy per request but makes HTTP/1.1 request bodies chunked; wants its own discussion. - zstd output overhang: `compress` returns a `Vec` with `cap = compress_bound(N)`; a tiny result pins an N-sized buffer until sent. --------- Signed-off-by: Ryan Brewster <[email protected]> Co-authored-by: Claude <[email protected]>
GlenDC
left a comment
There was a problem hiding this comment.
nice, ~4x on put_u8 here, on par with vec push now
|
Also @rpb-ant please rebase this branch on top of master, while you are on it. |
|
Happy to push & tweak :) |
<BytesMut as BufMut>::{put, put_slice, put_bytes} were the only BufMut
methods on BytesMut without #[inline] (tokio-rs#595 inlined extend_from_slice and
noted these as the follow-up; tokio-rs#459 did the same for Vec<u8>). Without it,
and often even with LTO once reserve_inner has been folded in, every
put_slice into a BytesMut is an out-of-line call, and put_u8 (trait
default: put_slice(&[n])) additionally pays a one-byte memcpy and a
second capacity check in advance_mut. Byte-at-a-time writers such as
varint encoders are dominated by this.
Adds #[inline] to the three methods and a put_u8/put_i8 override on
BytesMut and Vec<u8> shaped like Vec::push.
benches/bytes_mut.rs (x86_64, pinned cores):
put_slice_bytes_mut 16.97 ns -> 3.18 ns (put_slice_vec: 2.8)
put_u8_bytes_mut 492 ns -> 69 ns (put_u8_vec_push: 70)
put_u8_vec 205 ns -> 70 ns
Cover growing from empty, writing after a split, and negative put_i8.
eb0bd26 to
63744ad
Compare
|
lgtm now, thanks! |
Motivation
put,put_slice, andput_bytesare the onlyBufMutmethods onBytesMutwithout#[inline]Without it every
put_sliceinto a BytesMut is an out-of-line call (even with LTO, since folding inreserve_innerbumps it past the normal instruction limit).put_u8(which defaults toput_slice(&[n])) additionally pays a one-byte memcpy and a second capacity check inadvance_mut, which hurts byte-at-a-time writers such as varint encoders.Summary
Adds
#[inline]to the three methods, and aput_u8/put_i8override onBytesMutClean-instance numbers (Amazon Linux 2023, rustc 1.95.0, one pinned core, median of 3 passes of
cargo bench --bench bytes_mut, ns/iter):put_u8_bytes_mutput_slice_bytes_mutbytes_mut_extendput_u8_vecput_u8_vec_push(baseline)put_slice_vec