Skip to content

Only call reserve if the src isn't contiguous - #867

Merged
Darksonn merged 1 commit into
tokio-rs:masterfrom
conradludgate:bytesmut-put-contiguous-reserve
Nov 8, 2026
Merged

Darksonn merged 1 commit into
tokio-rs:masterfrom
conradludgate:bytesmut-put-contiguous-reserve

Conversation

@conradludgate

@conradludgate conradludgate commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Follow up to #865, #866 fixed the regression with the hot-path, but apparently I misread my benchmark results and it didn't fully solve the issue of my original regression.

#794 introduced a change to optimize for non-contiguous src buffers during put, but it calls it every time, even if it is not needed. I cannot fully explain why this improves my benchmark results, but it does.

LLM-generated justification (Codex)

The following explanation was generated by an LLM (Codex), based on its local benchmark and code-generation investigation.

There is a code-generation effect in addition to the redundant capacity check.

For a contiguous source, extend_from_slice already reserves for the entire copy. Reserving before it leaves two capacity checks in the generated code: LLVM does not infer reserve_inner's postcondition and remove the second one. The new condition folds away for &[u8], removing the up-front check.

This also makes the function cheaper to inline. In a controlled version of the reproducer, on rustc 1.99.1 / LLVM 23.1.1 with #866 applied, LLVM reports inline costs of 250–260 against a threshold of 250 before this change, versus 180–190 afterwards. The put calls become inlined, and the fixed 14/5-byte copies become direct stores instead of calls through the generic put/memcpy path.

Pinned to one Zen 5 core, median time per 100k labels across seven runs changed from 660 µs to 278 µs. As controls, forcing inlining without this change also gives 278 µs; forcing no inlining with the change gives 626 µs. These controls use #[inline(always)] and #[inline(never)] on BytesMut's implementation of BufMut::put; plain #[inline] was not tested. The put_slice control and fragmented-input case stay approximately unchanged.

The effect is context-dependent: #866 alone already fixes the exact minimized example on this compiler, where LLVM uses a higher threshold. The original Measured workload was also rerun; its timings are noisier, so this should not be claimed to account for the whole historical regression. The compiler remarks and controls do explain how removing this seemingly harmless reserve can have a much larger effect than the cost of one extra branch.

@Darksonn Darksonn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@Darksonn
Darksonn force-pushed the bytesmut-put-contiguous-reserve branch from 3925518 to 5e5bb2f Compare November 8, 2026 11:55
@Darksonn
Darksonn enabled auto-merge (squash) November 8, 2026 11:58
@Darksonn
Darksonn merged commit 78a3253 into tokio-rs:master Nov 8, 2026
18 checks passed
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.

2 participants