[Store] auto-enable MC_STORE_MEMCPY in TCP-only environments - #1936
Merged
Conversation
When MC_STORE_MEMCPY is not explicitly set, auto-detect based on installed transports: enable memcpy only when TCP is the sole transport (no RDMA, NVLink, etc.), since TCP loopback is less efficient than direct memcpy for same-host transfers. In RDMA environments the default remains disabled, as RDMA is more resource-efficient. Add TransferEngine::isTcpOnly() API that checks whether TCP is the only installed transport via MultiTransport::transport_map_. This is future-proof: any new transport registered via installTransport() is automatically accounted for without maintaining a protocol list. TENT path returns false unconditionally since TENT already rejects TCP loopback without MC_STORE_MEMCPY. Signed-off-by: Tianchen Ding <dtcccc@linux.alibaba.com>
dtcccc
requested review from
XucSh,
YiXR,
alogfans,
chestnut-Q,
doujiang24,
stmatengss and
ykwd
as code owners
April 21, 2026 02:49
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces an auto-detection mechanism for the MC_STORE_MEMCPY setting. When the environment variable is not explicitly set, the system now checks if the environment is TCP-only. If so, memcpy is enabled to optimize performance by avoiding TCP loopback overhead; otherwise, it remains disabled for more efficient transports like RDMA. This logic is implemented across TransferSubmitter, TransferEngine, and MultiTransport classes. I have no feedback to provide.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
alogfans
approved these changes
Apr 21, 2026
Closed
1 task
3 tasks
stmatengss
added a commit
that referenced
this pull request
May 16, 2026
…PY auto-enables (#2001) * [Store] fix: require same-process endpoint for LOCAL_MEMCPY strategy isLocalTransfer compared only the IP of handle.transport_endpoint_ to the local endpoint, so two processes on the same host (same IP, different ports) were treated as LOCAL_MEMCPY-eligible. The memcpy worker then dereferenced handle.buffer_address_, which is a virtual address only valid in the owning process, and segfaulted inside __memcpy_avx512_unaligned_erms. This was latent before #1936 (MC_STORE_MEMCPY defaulted to off). The TCP-only auto-enable exposed it on multi-process workloads such as the TorchSpec inference/trainer pipeline. Compare the full transport endpoint instead, matching the check already used by Client::IsReplicaOnLocalMemory. Cross-process same-host transfers now correctly fall through to TRANSFER_ENGINE; same-process transfers still take the memcpy fast path. Fixes the crash reported with MC_STORE_MEMCPY auto-enabled on TCP-only hosts. --------- Co-authored-by: Teng Ma <teng-ma@linux.alibaba.com>
A-Liuhao
pushed a commit
to A-Liuhao/Mooncake
that referenced
this pull request
Jun 25, 2026
…-ai#1936) When MC_STORE_MEMCPY is not explicitly set, auto-detect based on installed transports: enable memcpy only when TCP is the sole transport (no RDMA, NVLink, etc.), since TCP loopback is less efficient than direct memcpy for same-host transfers. In RDMA environments the default remains disabled, as RDMA is more resource-efficient. Add TransferEngine::isTcpOnly() API that checks whether TCP is the only installed transport via MultiTransport::transport_map_. This is future-proof: any new transport registered via installTransport() is automatically accounted for without maintaining a protocol list. TENT path returns false unconditionally since TENT already rejects TCP loopback without MC_STORE_MEMCPY. Signed-off-by: Tianchen Ding <dtcccc@linux.alibaba.com>
A-Liuhao
pushed a commit
to A-Liuhao/Mooncake
that referenced
this pull request
Jun 25, 2026
…PY auto-enables (kvcache-ai#2001) * [Store] fix: require same-process endpoint for LOCAL_MEMCPY strategy isLocalTransfer compared only the IP of handle.transport_endpoint_ to the local endpoint, so two processes on the same host (same IP, different ports) were treated as LOCAL_MEMCPY-eligible. The memcpy worker then dereferenced handle.buffer_address_, which is a virtual address only valid in the owning process, and segfaulted inside __memcpy_avx512_unaligned_erms. This was latent before kvcache-ai#1936 (MC_STORE_MEMCPY defaulted to off). The TCP-only auto-enable exposed it on multi-process workloads such as the TorchSpec inference/trainer pipeline. Compare the full transport endpoint instead, matching the check already used by Client::IsReplicaOnLocalMemory. Cross-process same-host transfers now correctly fall through to TRANSFER_ENGINE; same-process transfers still take the memcpy fast path. Fixes the crash reported with MC_STORE_MEMCPY auto-enabled on TCP-only hosts. --------- Co-authored-by: Teng Ma <teng-ma@linux.alibaba.com>
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
3 participants
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.
Description
Background
#577 disabled
MC_STORE_MEMCPYby default (false) for better RDMA performance stability — RDMA transfers are more stable and resource-efficient than memcpy operations. This was the correct decision for RDMA deployments.However, in TCP-only environments (where no RDMA hardware is available), disabling memcpy forces same-host transfers to go through TCP loopback, which is significantly less efficient than a direct
memcpy.What this PR does
This PR makes the
MC_STORE_MEMCPYdefault transport-aware without conflicting with #577's intent:MC_STORE_MEMCPYis not explicitly set, the system auto-detects installed transports via the newTransferEngine::isTcpOnly()API and enables memcpy only when TCP is the sole transport.MC_STORE_MEMCPYis explicitly set by the user (to0or1), the user's choice is always respected.false, exactly as Disable memcpy by default and improve stress workload test #577 intended.falsefalse(unchanged, per #577)falsetrue(auto-enabled)MC_STORE_MEMCPY=0(explicit)falsefalse(user respected)MC_STORE_MEMCPY=1(explicit)truetrue(user respected)Implementation
Added
TransferEngine::isTcpOnly()that queriesMultiTransport::transport_map_to check whether TCP is the only installed transport (size() == 1 && count("tcp") == 1). This is future-proof: any new transport protocol registered viainstallTransport()is automatically accounted for — no protocol whitelist/blacklist to maintain.The TENT path returns
falseunconditionally since TENT already rejects TCP loopback transfers whenMC_STORE_MEMCPYis disabled.Module
mooncake-transfer-engine)mooncake-store)mooncake-ep)mooncake-integration)mooncake-p2p-store)mooncake-wheel)mooncake-pg)mooncake-rl)Type of Change
How Has This Been Tested?
Checklist
./scripts/code_format.shbefore submitting.