Skip to content

perf: encode RESP replies straight into the output buffer (drop per-reply BytesMut)#505

Merged
ELares merged 1 commit into
mainfrom
perf/encode-into-output-buffer
Jul 4, 2026
Merged

perf: encode RESP replies straight into the output buffer (drop per-reply BytesMut)#505
ELares merged 1 commit into
mainfrom
perf/encode-into-output-buffer

Conversation

@ELares

@ELares ELares commented Jul 4, 2026

Copy link
Copy Markdown
Owner

What

Every RESP reply was encoded into a fresh BytesMut::with_capacity(64) and then copied into the connection's Vec<u8> output buffer -- one heap allocation + one buffer-to-buffer copy per reply, on every command.

ironcache_protocol::encode (and its 7 internal helpers) are now generic over &mut impl BufMut instead of &mut BytesMut. Since BytesMut: BufMut the change is backward-compatible for any external caller, and Vec<u8> is also a BufMut sink -- so the reply encoder writes directly into the serve loop's output buffer.

Applied at all six reply-encode wrappers:

  • serve.rs (the hot single-key reply path)
  • multikey.rs, spanning_move.rs, spanning_combine.rs, whole_keyspace.rs, coordinator.rs (the cross-shard fan-out reply encoders -- same wasteful pattern)
  • encode_to_vec now builds a Vec directly (no BytesMut round-trip)

Why

Optimization #3 of the datapath hot-path analysis (per-op allocation removal), on the reply side -- complements #504's O(P^2)->O(P) read-buffer fix on the request side. Both target the per-op allocation/copy asymmetry vs Redis identified while benchmarking against Redis 8 / Dragonfly.

Correctness

The encoded bytes are byte-identical -- this only changes where the encoder writes, not what. Verified:

  • 79 protocol encode/decode tests
  • 571 ironcache-server tests (incl. the RESP2/RESP3 reply-shaping suites)
  • 272 ironcache lib tests (the cross-shard reply paths)
  • fmt --all --check + clippy -D warnings --features io_uring + invariant lints clean; 0 dashes

The differential-vs-real-redis CI check is the byte-for-byte gate.

Note on measured impact

Like #504, isolating a throughput delta needs a CPU-bound regime (the prior AWS runs were latency/loadgen-bound). This is a strict reduction in per-reply allocation + copy work; a consolidated CPU-bound re-bench of the accumulated datapath optimizations is a follow-up.

🤖 Generated with Claude Code

…eply BytesMut)

Every reply was encoded into a fresh `BytesMut::with_capacity(64)` and then copied
into the connection's `Vec<u8>` output buffer -- one heap allocation + one
buffer-to-buffer copy per reply, on every command.

`ironcache_protocol::encode` (and its seven internal helpers) are now generic over
`&mut impl BufMut` instead of `&mut BytesMut`. `BytesMut` still satisfies `BufMut`, so
the signature change is backward-compatible for any external caller; and `Vec<u8>` is
also a `BufMut` sink, so the reply encoder can write DIRECTLY into the serve loop's
output buffer. All six reply-encode wrappers (the serve hot path + the multikey /
spanning-move / spanning-combine / whole-keyspace / coordinator cross-shard reply
encoders) now call `encode(out, ..)` with no temp buffer; `encode_to_vec` likewise
builds a `Vec` directly.

Optimization #3 of the datapath hot-path analysis (per-op allocation removal), on the
reply side; complements the #504 read-buffer O(P^2)->O(P) fix on the request side.

Byte-identical output (verified: 79 protocol encode/decode tests, 571 ironcache-server
tests incl. the RESP2/RESP3 reply-shaping suites, 272 ironcache lib tests all pass);
fmt + clippy -D warnings (with --features io_uring) + invariant lints clean; 0 dashes.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Zeke <ezequiel.lares@outlook.com>
@ELares

ELares commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

Deep self-review: verdict byte-identical + correct

The adversarial review read the full diff, the complete encoder, all 6 wrapper sites, AND the bytes 1.11.1 BufMut source, and grepped every encode call site. No issues found. Key confirmations:

  • Byte-identical: the encoder + its 7 helpers use ONLY put_u8/put_slice, which for Vec<u8> specialize to extend_from_slice (pure append) -- no BytesMut-specific method, no capacity-branching (remaining_mut is never consulted), so the emitted byte sequence is provably identical for both sinks.
  • Generic recursion compiles: bytes provides impl<T: BufMut + ?Sized> BufMut for &mut T, so &mut B is itself BufMut; the recursive encode(out, ..) calls implicit-reborrow and reuse out across array/map iterations.
  • Backward-compatible: the only encode callers are the 6 updated wrappers; the bench + tests use encode_to_vec. Nothing passes a BytesMut anymore (and BytesMut: BufMut would still compile if it did).
  • Append preserved: every wrapper's out: &mut Vec<u8> is appended to, never cleared -- load-bearing for the serve loop that accumulates pipelined replies. encode_to_vec builds a Vec directly (byte-identical to the old BytesMut + .to_vec()).
  • No realloc regression: extend_from_slice grows via amortized doubling; the change strictly removes the per-reply 64-byte alloc + temp->out copy.

The differential-vs-real-redis CI check is the byte-for-byte gate. Merging on green.

@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown

perf-gate (A5)

Same-runner ratchet of HEAD against the merge-base (both rebuilt and measured in this job).
PASS = within the noise band, WARN = a real move inside budget (does not fail), FAIL = past budget in the bad direction.

metric base head delta% band budget verdict
qps_median (peak) 90053.41 97026.90 7.74% +/-9.59% drop <= 15% PASS
bytes_per_key int 45.02 45.02 0.00% det rise <= 5% PASS
bytes_per_key embstr 61.07 61.07 0.00% det rise <= 5% PASS
bytes_per_key raw 333.15 333.15 0.00% det rise <= 5% PASS

Overall: PASS

  • qps: noisy on shared CI, so the band comes from the base reps spread (floored at 5%); a drop is only a regression past the 15% budget.
  • bytes_per_key: deterministic (allocator-true memmodel), so a tight 5% rise budget; any rise beyond it FAILs.
  • Open-loop tails / criterion micro-benches are reported-not-failed (tail noise is high) and are not part of this ratchet.
  • An intentional perf trade is landed by raising the relevant budget in this PR with a documented reason (CI never auto-commits a baseline).

@ELares ELares merged commit 7d66408 into main Jul 4, 2026
16 checks passed
@ELares ELares deleted the perf/encode-into-output-buffer branch July 4, 2026 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant