Skip to content

[GG] Calibrate lossless PCIe DMA dispatch crossover - #184

Merged
lukealonso merged 2 commits into
dev/gilded-gnosisfrom
feat/gg-pcie-dma-calibration-20260726
Jul 29, 2026
Merged

[GG] Calibrate lossless PCIe DMA dispatch crossover#184
lukealonso merged 2 commits into
dev/gilded-gnosisfrom
feat/gg-pcie-dma-calibration-20260726

Conversation

@voipmonitor

@voipmonitor voipmonitor commented Jul 26, 2026

Copy link
Copy Markdown

Summary

  • replace the fixed 6 MiB SparkInfer DMA crossover with VLLM_PCIE_DMA_MIN_BYTES
  • accept byte-size values and an explicit off state
  • avoid constructing the DMA collective when calibration determines that lossless DMA never beats NCCL
  • preserve the existing 6 MiB behavior when the variable is unset

Why

The crossover depends on GPU order, PCIe/root-complex topology, CPU/NUMA placement, and collective configuration. A fixed threshold can route large all-reduces to a slower path on cross-root or PCIe Gen4 systems. A pre-model probe can now provide the measured threshold without changing compressed wire modes.

Validation

  • git diff --check dev/gilded-gnosis...HEAD
  • runtime contract exercised in the pinned v20 CUDA 13.2 image for default, 24MB, off, disabled, and invalid negative values
  • focused unit coverage added in test_b12x_fused_all_reduce.py

This PR does not select FP8/INT8/MXFP8 DMA modes. Those remain explicit quality choices.

Summary by CodeRabbit

  • New Features
    • Added configuration for the minimum PCIe DMA transfer size via VLLM_PCIE_DMA_MIN_BYTES (default 6MB).
    • Accepts raw byte values or KB/MB suffixes (whitespace ignored) and can disable DMA using off, disabled, or none (case-insensitive).
    • When disabled, large all-reduce operations continue on the standard communication path.
  • Bug Fixes
    • Negative DMA threshold values are now rejected with a clear error.
  • Tests
    • Added unit coverage for defaults, custom values, disabled modes, and invalid negatives.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d958eb3e-3f42-4e5c-9af1-771e71c23384

📥 Commits

Reviewing files that changed from the base of the PR and between 9508e96 and c4022e8.

📒 Files selected for processing (2)
  • tests/distributed/test_b12x_fused_all_reduce.py
  • vllm/envs.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/distributed/test_b12x_fused_all_reduce.py
  • vllm/envs.py

📝 Walkthrough

Walkthrough

Adds VLLM_PCIE_DMA_MIN_BYTES configuration for b12x PCIe DMA thresholds, including disabled modes and validation. The all-reduce initialization path conditionally loads DMA and reports the configured value. Tests cover defaults, MB parsing, disabled values, and negative input.

Changes

b12x DMA configuration

Layer / File(s) Summary
DMA configuration contract and parser
vllm/envs.py, vllm/distributed/device_communicators/custom_all_reduce.py, tests/distributed/test_b12x_fused_all_reduce.py
Registers VLLM_PCIE_DMA_MIN_BYTES with a 6MB default, parses byte-size values, maps off/disabled/none to disabled DMA, rejects negative values, and tests these behaviors.
Conditional DMA initialization
vllm/distributed/device_communicators/custom_all_reduce.py
Uses the configured threshold to skip or initialize the PCIe DMA backend and logs either the byte threshold or off.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Environment
  participant CustomAllreduce
  participant DmaAllReduce
  Environment->>CustomAllreduce: provide VLLM_PCIE_DMA_MIN_BYTES
  CustomAllreduce->>CustomAllreduce: parse threshold or disabled value
  CustomAllreduce->>DmaAllReduce: load and configure DMA when enabled
  CustomAllreduce-->>CustomAllreduce: log threshold or off
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: making the PCIe DMA dispatch crossover configurable and calibrated instead of fixed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/gg-pcie-dma-calibration-20260726

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@vllm/envs.py`:
- Around line 1819-1821: Complete the DMA threshold contract at vllm/envs.py
lines 1819-1821 by documenting the 6MB default, accepted byte-size syntax, and
case-insensitive disabled spellings off, disabled, and none; update
tests/distributed/test_b12x_fused_all_reduce.py lines 176-190 to cover
whitespace-normalized input and disabled/none values while preserving existing
threshold behavior.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d8342f19-3b52-4795-8113-ffafa418a2ee

📥 Commits

Reviewing files that changed from the base of the PR and between 89b4a98 and 9508e96.

📒 Files selected for processing (3)
  • tests/distributed/test_b12x_fused_all_reduce.py
  • vllm/distributed/device_communicators/custom_all_reduce.py
  • vllm/envs.py

Comment thread vllm/envs.py Outdated
@voipmonitor

Copy link
Copy Markdown
Author

Validated in the final v20 release candidate: voipmonitor/vllm:gilded-gnosis-v20-vllm0c79e41-sie603f74-fi801d57a-cu132-20260726.

The helper consumed the calibrated lossless cutoff as VLLM_PCIE_DMA_MIN_BYTES=25165824; TP8/DCP1 decode remained at 87.50 tok/s aggregate / 87.49 tok/s server, and TP8/DCP4 64k prefill reached 5,835 tok/s. Unit/manual parsing checks and image runtime contracts passed. No compressed wire mode was selected automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants