feat(io/rdma): reuse thread-local scratch buffers on batch hot path#389
Merged
Merged
Conversation
Avoid per-batch heap allocations in RdmaBatchReadWrite (indices, merged WRs + per-WR sges, chunk-expansion vector, per-EP signal counters) by reusing thread_local pools with slot reuse, and expand chunked WRs in place. The pools belong to the outermost call on a thread; same-thread re-entry transparently falls back to local buffers. Small/medium-message, large-batch RDMA transfers see ~3-7% lower latency / higher BW (scales with batch); large messages unchanged. Internal-only; no ABI/wire change.
There was a problem hiding this comment.
Pull request overview
This PR optimizes the RDMA batch transfer hot path by reusing per-thread scratch containers inside RdmaBatchReadWrite() to avoid repeated heap allocations, and by performing chunk-expansion in-place only when needed. The changes are internal to src/io/rdma/common.cpp and preserve the existing PlanChunks() API while introducing a new in-place planning helper.
Changes:
- Add
thread_localscratch pools (with a same-thread re-entrancy fallback) to reuse vectors acrossRdmaBatchReadWrite()calls. - Replace
ExpandChunkedWorkRequests()with an in-place chunk expansion into a pooled buffer, and skip the expansion pass entirely when no WR exceedschunkBytes. - Introduce
PlanChunksInto()to fill a caller-owned chunk plan buffer while keeping the existingPlanChunks()API intact.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Hi @maning00, CI is green and this is ready for review whenever you have time. Thanks! |
maning00
approved these changes
Jun 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RdmaBatchReadWrite()(src/io/rdma/common.cpp) allocates severalstd::vectorson every batch transfer:
indices(size = batchSize)mergedWrsplus a per-WRsges.reserve(maxSge)(≈ one heap allocation per WR)ExpandChunkedWorkRequests(std::move(...))epWrsSinceSignal,epMergedSinceSignalFor small/medium messages these allocations sit on the critical path before the
WRs are posted, so they directly add to per-batch latency and scale with batch size.
This PR converts those scratch containers to
thread_localpools reused acrosscalls (slot reuse preserves each WR's inner
sgescapacity), expands chunked WRsin place into a second pooled buffer (the chunk plan is reused via a new
PlanChunksInto()that fills a caller-owned buffer), and skips the chunk-expansionpass entirely when no WR exceeds
chunkBytes. Single-SGE WR initialization is sharedvia one
initSingleSgeWrhelper.Safety / hygiene addressed in this PR:
cheap RAII depth sentinel makes any same-thread re-entry (e.g. from a completion
callback) transparently fall back to local buffers, so the outer call's pools are
never clobbered. (No such re-entry path exists today; this is defensive.)
earlier very large batch grew it far beyond the current need.
ExpandChunkedWorkRequests()is removed.No public header / ABI / wire-format change — internal to
src/io/rdma/common.cpp.Motivation / Measurements
On 2× MI308X (gfx942, ROCm 7.2.0), 2×200G bnxt RoCE, cross-node, GPU memory, 4 QP,
session + batch transfer,
op-type write. A/B = same binary with vs. without thischange (initiator side), identical network config.
A microbench of just the per-batch allocation cost (batchSize=128) shows
~7.5 µs → ~0.9 µs (88 % of the allocation cost removed), scaling ~linearly with batch.
End-to-end benchmark (
tests/python/io/benchmark.py):The absolute saving grows with batch size (≈2 µs @128 → ≈7 µs @512), matching the
microbench's linear prediction. Large messages are bandwidth-bound and unaffected,
as expected.
Risk / Correctness
data validation (
torch.equal) passes across the full size sweep for bothwriteand the chunked path.benchmark's
torch.equalvalidation over the full size sweep after this revision.Test plan