perf: raise the GPU-to-NIC submission rate with channels, QPs, forward pairs - #9
Conversation
…stagger The combine forward warps replayed tokens in the dispatch arrival order frozen into the handle, which interleaves local-bypass tokens (no RDMA) with remote tokens in a network-timing-dependent way. A node whose replay front-loads local tokens back-loads all of its scale-out puts so with the NIC near saturation the shift never drains and surfaces as the peer node's exit-wait tail, so one node runs ~62 GB/s SO while the other runs ~76. Here we make the schedule deterministic: the scale-up warps sweep the linked list twice (remote-destined tokens first, local-bypass second) and the forward warps replay in the same two-pass order, so count-based tail gating is unchanged and every rank issues its puts on the same schedule. Pass 0 flushes its TMA record and remainder batches before pass 1 so the wire keeps draining while locals are reduced.
…s, forward pairs Raise the unordered combine's submission rate three ways: drop the 4-channel/SM cap and let shared memory and the GIN signal budget decide (8/SM at 12 SM) for more parallel submitters, raise the default GIN contexts 11 -> 13 so every SM of the recommended 12-SM launch owns its QP instead of sharing WQE rings, and add cooperative forward warp pairs that split each token's hidden dim across two warps so tokens are reduced faster and therefore submitted faster.
| #pragma unroll | ||
| for (int i = 0; i < kNumScaleupRanksPerLane; ++ i) | ||
| stored_token_idx[i] = -1; | ||
| #pragma unroll 1 |
There was a problem hiding this comment.
General note about this commit:
perf(combine): schedule scale-out puts remote-first to mitigate node stagger
We already know that there is an issue with the EFA latency, and that the odd nodes are slower then the even, and most likely we spend the time in the end of combine which is the wait.
So instead of root cause it and understand why we wait so long, you changed the ordering of the traffic which is to first handle the RDMA traffic and then the NVLINK traffic.
And this masked the issue, as the RDMA now have more time to arrive.
however, you added more compute operation and if the EFA latency was solved then this code impact would be much lower and potentially can even slow down the perf as you added more compute operation (the double sweep).
Having sayed that, i think this approach is intresting, but it should be addressed the following manner:
- first solve the actual EFA issue.
- apply this patch and see how it impact the result.
There was a problem hiding this comment.
I think these are separate issues, no matter how EFA behave, we should always deal with RDMA tokens then local tokens. We still see some variance which may or may not come from EFA. But worth noting that on p6 the max-min is only 1-5GB/s now, I shared the raw data.
| static constexpr int kMaxGinContextCnt = kMaxGinContextBudget; | ||
|
|
||
| // Default context count (== default QP count). 11 contexts -> 21 signals/context. | ||
| // Default context count (== default QP count). 13 contexts -> 17 signals/context. |
There was a problem hiding this comment.
This is the biggest change, it change the sharing mode from _GPU to _CTA on 12SM configuration.
I remember it had some impact on the dispatch kernel and that 11 gin context are the max that does not impact it. was it "solved" because you also forced the "prefer_overlap_with_compute" to be true?
this need to be better highlight in the commit message.
| num_smem_bytes, | ||
| num_qps, num_gpu_timeout_cycles, | ||
| cached_mode, do_cpu_sync, | ||
| not prefer_overlap_with_compute, |
There was a problem hiding this comment.
This control the double buffer you used. If now you are forcing the the overlap with compute then you dont need the double buffer as you have double warp count. This will actually reduce the number of warp due to SMEM limitation.
| num_smem_bytes and | ||
| "dispatch TMA pool exceeds the shared-memory budget"); | ||
| if (not prefer_overlap_with_compute) | ||
| num_channels_per_sm = std::min<int>(num_channels_per_sm, 4); |
There was a problem hiding this comment.
You basically deprecated this flag and changed the customer facing API. I am fine with it, but now the perf base line that we compare against is not the same, saying that we reach the same performance as official DeepEP on IB is not correct as they set this flag to false.
We need to make it clear that this flag is no longer behave the same when we publish those results both internally and externally.
| constexpr int kVecPerRole = kHiddenVec / kNumFwWarpsPerChannel; | ||
| constexpr int kUnrollFactor = get_max_unroll_factor<kVecPerRole, kAdjustRegisters ? 8 : 4>(); | ||
| const int vec_off = fw_role * kVecPerRole; | ||
| combine_reduce<kVecPerRole, kUnrollFactor, math::constexpr_ceil_div(kNumTopk, kNumScaleoutRanks)>( |
There was a problem hiding this comment.
Interesting idea, as the claim is that we context bounded and not compute bounded and we are not fully utilizing the GPU FLOPS if i got it correctly, and in this specific case we increase it by adding additional 32 threads.
- How much does it actually add to the perf as compare to the other changes added here, as each one by itself can boost it.
- In addition why did not you applied it also for combine reduced, as it should also boost perf?
- why not having the second warp spin here waiting for a "go" signal and then both of them do the reduction, why does it need to go through the entire flow, for me it make more sense if you will have some kind of helper warps that are all waiting here and all are getting a job and you sync them, e.g a pool of 4 extra warp in your case.
|
We ran this branch against Setup. EFA installer 1.50.0, Prefill, 8192 tok/rank (us, all-rank mean)
Decode, 128 tok/rank (us, all-rank mean)
Four observations: 1. The node-variance claim from #8 reproduces at 2 nodes and does not at 4.
Per-node mean At 4 nodes it does not hold. 4N/128 per-node mean 2. 3. 4. Most of the win is at 2 nodes. 2N decode dispatch −27.2% becomes −7.1% at 4N; Two things this does not answer, to be explicit:
And one caveat on reading the tables: since this PR deletes the Separately, for merge ordering: PR #1's |
3 participants
Raise the unordered combine's submission rate three ways: drop the 4-channel/SM cap and let shared memory and the GIN signal budget decide (8/SM at 12 SM) for more parallel submitters, raise the default GIN contexts 11 -> 13 so every SM of the recommended 12-SM launch owns its QP instead of sharing WQE rings, and add cooperative forward warp pairs that split each token's hidden dim across two warps so tokens are reduced faster and therefore submitted faster.