nixl_ep: Optimize low-latency NVLink path - #1751
Conversation
|
👋 Hi itayalroy! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a device-side per-rank P2P pointer cache (device array + kernel + host launcher), wires allocation/refresh/free in Buffer, updates kernels to use cached pointers and adjust mask-load semantics, and tweaks PTX opcode selection and Meson/config defaults (TOPK_IDX_BITS → 32, DISABLE_AGGRESSIVE_PTX_INSTRS). ChangesP2P Pointer Cache System
PTX Instructions and Build Configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/device/ep/csrc/kernels/utils.cuh`:
- Around line 179-183: The LD_NC_FUNC macro currently expands to
"ld.volatile.global.L2::256B" when DISABLE_AGGRESSIVE_PTX_INSTRS is defined,
which requires PTX ISA 7.4+/SM80+ assembler support; update the build
configuration to target PTX 7.4+ / compute capability SM80+ (e.g., set the
appropriate -arch / -gpu-architecture / CUDA_NVCC_FLAGS) so the assembler
accepts the ".L2::256B" cache-hint, or add a fallback macro value for older
PTX/SM targets; locate and adjust the LD_NC_FUNC definition and any build
scripts that influence PTX/SM targets to ensure compatibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: aac73b69-891d-42e2-a161-a8806cda6330
📒 Files selected for processing (6)
examples/device/ep/csrc/kernels/api.cuhexamples/device/ep/csrc/kernels/nixl_ep_ll.cuexamples/device/ep/csrc/kernels/utils.cuhexamples/device/ep/csrc/nixl_ep.cppexamples/device/ep/csrc/nixl_ep.hppexamples/device/ep/meson.build
252b191 to
604dbb2
Compare
Resolve peer base pointers once when NIXL memory views are created. The low-latency kernels then index the cache instead of calling the NIXL device API on the datapath. Signed-off-by: Itay Alroy <ialroy@nvidia.com>
Disable the aggressive DeepEP PTX copy path by default. This matches DeepEP PR 605 and improves dispatch performance without affecting combine. Signed-off-by: Itay Alroy <ialroy@nvidia.com>
Use the same TMA store wait instruction as DeepEP before issuing the next store in the low-latency combine path. Signed-off-by: Itay Alroy <ialroy@nvidia.com>
Default TOPK_IDX_BITS to 32. The low-latency kernels only need int32 expert ids for practical EP configurations, which reduces metadata size in the datapath. Signed-off-by: Itay Alroy <ialroy@nvidia.com>
Combine receive checks the rank mask for each top-k entry of each token after cg::this_grid().sync(). At that point, the timeout phase is complete and the active-rank mask cannot be modified, so these reads do not need acquire semantics. Use regular mask-buffer loads instead, which performs much better in the low-latency combine datapath. This relies on the CUDA synchronization guarantee: "All memory accesses (e.g., reads and writes) made by threads in the group before the synchronization point are visible to all threads in the group after the synchronization point." Signed-off-by: Itay Alroy <ialroy@nvidia.com>
604dbb2 to
a7a3bf3
Compare
|
Changed P2P pointer caching to launch one small kernel per newly connected rank.
The alternative is adding and populating a I think we should consider making UCX return |
|
/build |
NIXL EP can be built with different top-k index types. Query the dtype reported by the installed nixl_ep instead of hardcoding int64. ai-dynamo/nixl#1751 changes the default NIXL EP top-k index type from 64-bit to 32-bit. Following nixl_ep.topk_idx_t keeps vLLM compatible with both the upstream default and custom NIXL EP builds. Signed-off-by: Itay Alroy <ialroy@nvidia.com>
This PR adds a set of optimizations for NIXL EP low-latency NVLink path. Each optimization is in a separate commit, with the explanation included in the commit message. On the TRT-LLM MoE communication benchmark ([bench_moe_comm.py](https://github.com/NVIDIA/TensorRT-LLM/blob/main/tests/microbenchmarks/bench_moe_comm.py)) over single-node NVLink, 8xH100, these changes bring NIXL EP dispatch to parity with DeepEP low latency and reduce the remaining combine gaps. <details> <summary>Full dispatch benchmark table</summary> | Batch | deepep low latency | nixl ep main | main vs deepep | nixl ep PR 1749 | PR vs deepep | |---:|---:|---:|---:|---:|---:| | 16 | 32.06 µs | 38.34 µs | 0.836x | 32.55 µs | **0.985x** | | 32 | 37.61 µs | 45.49 µs | 0.827x | 37.98 µs | **0.990x** | | 64 | 49.38 µs | 61.12 µs | 0.808x | 49.69 µs | **0.994x** | | 128 | 75.21 µs | 92.18 µs | 0.816x | 74.76 µs | **1.006x** | | 256 | 128.85 µs | 147.32 µs | 0.875x | 128.20 µs | **1.005x** | | 512 | 229.28 µs | 248.60 µs | 0.922x | 228.14 µs | **1.005x** | </details> <details> <summary>Combine active rank mask optimization</summary> | Batch | disable mask / speed-of-light | current: load acq | current vs speed-of-light | this PR: plain load | PR vs speed-of-light | |---:|---:|---:|---:|---:|---:| | 16 | 25.03 µs | 27.24 µs | 0.919x | 26.26 µs | **0.953x** | | 32 | 31.74 µs | 33.79 µs | 0.939x | 33.06 µs | **0.960x** | | 64 | 45.65 µs | 48.48 µs | 0.942x | 46.94 µs | **0.973x** | | 128 | 73.58 µs | 76.75 µs | 0.959x | 74.37 µs | **0.989x** | | 256 | 137.18 µs | 141.01 µs | 0.973x | 137.34 µs | **0.999x** | | 512 | 258.57 µs | 266.23 µs | 0.971x | 258.68 µs | **1.000x** | </details> **Note that there is no visible performance impact on NIXL EP’s [elastic.py](https://github.com/ai-dynamo/nixl/blob/main/examples/device/ep/tests/elastic/elastic.py).** Follow-up PRs will continue optimizing NIXL EP combine, with the goal of matching and then exceeding DeepEP low-latency performance. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * GPU-side P2P pointer caching to improve peer-to-peer GPU memory access performance. * **Chores** * Reduced default index width from 64-bit to 32-bit. * Added a CUDA compile flag to alter PTX instruction selection for targeted memory operations. * Improved allocation, initialization, population, and cleanup of the GPU pointer cache to ensure consistent behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Itay Alroy <ialroy@nvidia.com>
This PR adds a set of optimizations for NIXL EP low-latency NVLink path.
Each optimization is in a separate commit, with the explanation included in the commit message.
On the TRT-LLM MoE communication benchmark (bench_moe_comm.py) over single-node NVLink, 8xH100, these changes bring NIXL EP dispatch to parity with DeepEP low latency and reduce the remaining combine gaps.
Full dispatch benchmark table
Combine active rank mask optimization
Note that there is no visible performance impact on NIXL EP’s elastic.py.
Follow-up PRs will continue optimizing NIXL EP combine, with the goal of matching and then exceeding DeepEP low-latency performance.
Summary by CodeRabbit
New Features
Chores