Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2330792aa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return ( | ||
| self.prefill_context_parallel_size > 1 | ||
| and self.decode_context_parallel_size == 1 | ||
| ) |
There was a problem hiding this comment.
Gate decode sharding off for direct-write MLA models
When PCP > 1 and DCP == 1, this enables sharding for every MLA model, but the new cache replication only covers the generic MLAAttention/non-skipped indexer insertion paths. DeepSeek-V3.2 instead writes both MLA and indexer KV directly inside fused_norm_rope (vllm/models/deepseek_v32/attention.py:397-414) and then calls the indexer with skip_k_cache_insert=True; DeepSeek-V4 similarly inserts into its SWA and compressor caches directly. On non-owner ranks these paths therefore never receive the owner's decode KV, and because ownership can change after batch compaction, a later decode can read an incomplete cache and produce incorrect output. Either replicate writes in these direct-insertion paths or disable request sharding for those model implementations.
Useful? React with 👍 / 👎.
|
✅ @pisceskkk, CI is now available for this PR.
|
|
This pull request has merge conflicts that must be resolved before it can be |
2330792 to
d75dbb2
Compare
|
This pull request has merge conflicts that must be resolved before it can be |
GirasoleY
left a comment
There was a problem hiding this comment.
Thanks for upstream this. The change itself lgtm.
Just to understand the data. It looks like in your experiment this gives better TPOT than TP in a aggregated setup? I wonder if you could provide more details? Are these done in a larger batch size setup, and is it because the decode forward is actually faster, or is it because the mixed batch now runs faster?
I had a similar GLM5.2 experiment previously and the data shows in a pure decode setup, and it only provide minor throughput gain at high batch size (upto 6%), so just want to check if this aligns with what you see as well.
Yes, these results were measured with large batches and a continuous stream of incoming requests. The TPOT improvement mainly comes from faster mixed-batch computation rather than pure decode. This is also the main reason why PCP can outperform TP in mixed deployment. Even sharding decode tokens across PCP ranks may not necessarily outperform replicated-decode PCP in a pure-decode setup, since it introduces an additional all-gather and therefore comes with its own trade-off. :( |
d75dbb2 to
8d1484e
Compare
8d1484e to
f9376a7
Compare
Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: QiuChunshuo <qiuchunshuo@huawei.com>
Signed-off-by: QiuChunshuo <qiuchunshuo@huawei.com>
Signed-off-by: QiuChunshuo <qiuchunshuo@huawei.com>
Signed-off-by: QiuChunshuo <qiuchunshuo@huawei.com>
f9376a7 to
42fc76a
Compare
Purpose
PCP currently replicates every decode request on every PCP rank. For PCP-only deployments (
DCP == 1), every rank already has a complete KV-cache replica, so this repeats the same decode computationpcp_sizetimes.This PR:
DCP > 1, because each DCP rank must consume its local KV shard.Design trade-off. This approach distributes decode requests across the PCP group in a data-parallel fashion and removes redundant decode computation. However, keeping the replicated KV cache consistent across PCP ranks requires additional communication to propagate the owner-produced KV updates. This compute-communication trade-off makes the realized gains smaller and more workload-dependent than the ideal reduction in redundant computation would suggest.
Duplicate-work check
I searched open PCP/decode issues and PRs before submission. This does not duplicate:
Related performance tracking: #31473.
AI assistance
OpenAI Codex was used to analyze, implement, test, benchmark, and prepare this draft. The commit includes an AI co-author trailer.
Test Result
DeepSeek-V2-Lite-Chat correctness and functionality
ignore_eos=false.Long-context PCP decode performance
Performance was measured on NVIDIA H20. Every measured case used 128k input tokens, 1k output tokens, concurrency 32, 128 requests, temperature 0, seed 0, and
max_num_batched_tokens=32768, with identical prompts and request order for the compared states. Every case completed 128/128 requests with 0 failures.PCP sharding headline
The direct comparison is PCP before sharding versus PCP after sharding.
4x H20 — random two-layer DeepSeek-V3.2 FP8 checkpoint (80/90/95/97/99% hit rates)
This random two-layer, 64-expert checkpoint is a controlled topology/performance probe, not a production-model estimate.
Throughput — PCP sharding before → after
Latency — PCP sharding before → after
The sharded PCP4 80% row is the retest on physical GPUs 4-7: 128/128 completed, 0 failed, actual hit rate 79.9805%, 1.927 req/s, 1,973.216 output tokens/s, TTFT mean/p99 3,347.783/10,309.820 ms, TPOT 12.875 ms/token, ITL 13.082 ms/token, and E2E mean/p99 16,518.981/22,152.288 ms.
8x H20 — 1/4-depth DeepSeek-V3.2 FP8 checkpoint (95/97/99% hit rates)
Throughput — PCP sharding before → after
Latency — PCP sharding before → after
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.