Add an on-demand Iris vs RCCL CCL benchmark job - #553
nirvedhmeshram wants to merge 6 commits into
Conversation
Iris had this and lost it. comprehensive_sweep.py swept the collectives with a --benchmark_rccl flag and the per-op scripts reported rccl_ms and rccl_bandwidth_gbps; it was deleted in 15bba61 when the benchmarks moved to iris.bench, and the RCCL paths went with it. plot_sweep_results.py survived, still documented as comparing Iris vs RCCL, reading a CSV nothing produced. Rather than restore the old subprocess driver, give each CCL benchmark a "backend" axis with values iris and rccl. Both series then come out of one sweep over identical shapes through the same timing harness, and the framework's own CSV already carries the comparison. The RCCL baseline uses ordinary torch tensors, not Iris heap memory. The symmetric heap is allocated with flags RMA requires, so timing RCCL against it would measure Iris's allocation choice rather than RCCL; someone choosing between them would call RCCL on normal tensors. reduce_scatter is left out. iris.ccl.reduce_scatter is (M, N) -> (M, N) while torch's reduce_scatter_tensor is (W*M, N) -> (M, N), so a side-by-side number would compare two different operations. plot_sweep_results.py now reads the framework CSV and plots against message size instead of comm_sms, and emits a Markdown table for the job summary. The workflow is workflow_dispatch plus push to main, never pull_request: it is a measurement, not a gate, and sweeping four collectives twice over would crowd out the test jobs on a shared 8-GPU pool. Sizes, ranks and repeat count are dispatch inputs, defaulting to a deliberately small sweep.
| @bench.axis("num_ranks", [2, 4, 8]) | ||
| @bench.axis("M", bench.power_of_two(10, 14)) | ||
| @bench.axis("N", bench.power_of_two(10, 14)) | ||
| @bench.axis("dtype", [torch.float16, torch.bfloat16]) |
There was a problem hiding this comment.
there is no RCCL fp8 available so this would not have a baseline, happy to add if we just want to see them by themselves though. Let me know.
… harness
Aggregation. iris.bench reported rank 0's mean, so a straggler on another GPU
was invisible -- a collective finishes when its slowest participant does. Each
rank now summarises its samples with a median (outlier-resistant), the per-rank
medians are all_gathered, and the headline gpu_time_ms is the maximum. min_time_ms
and skew_pct come along so an imbalance shows up rather than being averaged away,
and bandwidth/TFLOPs derive from the headline. This matches the triton-shmem
harness the reviewer pointed at.
Note this changes gpu_time_ms semantics for every iris.bench benchmark, not only
the CCL ones. It does not move the performance-regression gate: none of the 12
examples under examples/*/benchmark.py use iris.bench.
Shapes. Dropped the M=N diagonal, which was my invention and is not
representative. These collectives move (tokens, hidden) tensors, where the token
count varies over orders of magnitude while the hidden dimension stays in a
narrow band, so M and N are now swept independently with N bounded --
M in {1K,4K,16K,64K} x N in {1K,2K,4K,8K}, as triton-shmem does.
Also moved the axes and the RCCL-baseline helper into benchmark/ccl/_common.py
instead of repeating them per file, and the rationale into a README there.
fp8 is deliberately not swept: RCCL implements no fp8 reductions, so all_reduce
and reduce_scatter would have no baseline, and adding it only for the
movement-only collectives would make the table inconsistent. Written down in the
README rather than left as a silent omission.
zizmor reported 7 HIGH and 2 MEDIUM; CodeQL flagged the permissions separately.
Now clean.
template-injection (the one that mattered): dispatch inputs were interpolated
straight into run: blocks. ${{ }} expands before the shell sees the script, so a
crafted num_ranks/sizes/n_repeat could have injected commands. They are bound to
job-level env vars and referenced as shell variables instead.
excessive-permissions: added permissions: contents: read. Nothing here writes to
the repo, and upload-artifact uses its own runtime token rather than
GITHUB_TOKEN.
artipacked: checkout with persist-credentials: false, so the token is not left in
.git/config for a job that uploads artifacts.
unpinned-uses: pinned actions/checkout and actions/upload-artifact to hashes
(v4.4.0, v4.6.2 -- same majors, no version bump). The other workflows in the repo
are unpinned, but the scan only audits *changed* files, so they are never
checked while this one would be.
The rest of this repo's workflows reference actions by major tag. Pinning to commit hashes here made this one file the odd one out, so revert to @v4 and suppress the scan finding explicitly rather than diverging silently.
triton-shmem's plot_bench.py emits latency, speedup and bandwidth; this only emitted bandwidth, so the two figures that show where Iris stands relative to RCCL were missing. Add them, and lay each out as one column per collective and one row per rank count so sweeping num_ranks grows the figure. Bandwidth is left alone: the bench scripts already declare bus bytes via set_bytes, matching nccl-tests conventions, so no bus factor is applied here. Variants within a backend now plot as separate series rather than being median-averaged together, which would have silently mixed one_shot and two_shot all_reduce into a single curve.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 5
Open (6)
This description contradicts the implementation in_grid()(andbenchmark/ccl/README.md), which… · New The table merges all variants for a backend into a singlesize -> pointmap viadict.update(),… · New The Markdown table uses truthiness checks (if il,if rl, etc.). This will render valid0.0… · New This allocatesworld_sizeCUDA tensors plus a fresh input tensor for every benchmark point. Even… · Newgpu_time_msnow represents the slowest rank’s median, whileall_times_msappears to remain the… · New The PR description mentions 'Four collectives across two backends', but the workflow runner… · New
What changed in this PR
Adds an on-demand CI workflow to benchmark Iris CCL collectives against RCCL using a shared iris.bench timing harness, then publishes plots and a Markdown comparison table.
Changes:
- Extend
iris.benchresult aggregation to report cross-rank latency (slowest-rank median), plusmin_time_msandskew_pctin console/JSON/CSV. - Add a
backendsweep axis (irisvsrccl) to CCL benchmarks and a shared_common.pyfor axes and RCCL tensor allocation. - Revive/modernize the CCL plotting pipeline to consume
iris.benchCSV output and emit bandwidth/latency/speedup figures + Markdown summary; add a GitHub Actions workflow and runner script.
| File | Description |
|---|---|
| iris/bench/_runner.py | Adds cross-rank aggregation and emits new CSV/JSON fields (min_time_ms, skew_pct). |
| iris/bench/_core.py | Documents new result semantics and fields on Result. |
| benchmark/ccl/plot_sweep_results.py | Reworks plotting/summary to consume iris.bench CSV and compare Iris vs RCCL. |
| benchmark/ccl/bench_all_reduce.py | Adds backend axis and RCCL baseline path; aligns bytes accounting. |
| benchmark/ccl/bench_all_gather.py | Adds backend axis and RCCL baseline path; aligns bytes accounting. |
| benchmark/ccl/bench_all_to_all.py | Adds backend axis and RCCL baseline path; aligns bytes accounting. |
| benchmark/ccl/_common.py | Centralizes sweep axes and RCCL “plain tensor” helper. |
| benchmark/ccl/README.md | Documents benchmark intent, axes, aggregation, and plotting outputs. |
| .github/workflows/iris-ccl-benchmark.yml | Adds on-demand/post-submit workflow to run sweep and publish artifacts/summary. |
| .github/scripts/run_ccl_benchmark.sh | Implements the sweep + plotting inside the container and writes artifacts. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Aggregate with two scalar all_reduce (MAX/MIN) instead of all_gather. Only the extremes are used, so allocating world_size tensors per benchmark point was wasted work. - Rename Result.all_times_ms to local_times_ms. gpu_time_ms is now a cross-rank figure, so the old name implied the two summarised the same samples. - Emit one table row per variant. Flattening them dropped points whenever two variants shared a message size, with insertion order deciding the winner. - Check 'is not None' rather than truthiness when formatting cells, so a 0.0 timing is not rendered as missing data. - Fix the module docstring's subplot layout, which described the pre-flip orientation, and the workflow's stale 'four collectives' comment.
a3ee7a7 to
2be182d
Compare





Summary
Adds an on-demand job that benchmarks the
iris.cclcollectives against RCCL and publishes a comparison table and graphs. Manual trigger or post-submit onmain— deliberately not on pull requests.This existed before and was lost
benchmark/ccl/comprehensive_sweep.pyswept the collectives with a--benchmark_rcclflag, and the per-op scripts reportedrccl_ms/rccl_bandwidth_gbps/rccl_ratio_percent. It was deleted in 15bba61 when the benchmarks migrated toiris.bench(#486), and the RCCL paths went with it.What survived is
benchmark/ccl/plot_sweep_results.py— still documented as "subplots comparing Iris vs RCCL bandwidth for each collective operation", still reading a CSV that nothing has produced since. This gives it a real input again.Approach
Rather than restore the old subprocess driver, each CCL benchmark gets a
backendaxis:@bench.axis("backend", ["iris", "rccl"])Both series then come out of one sweep over identical shapes through the same timing harness, and the framework's existing CSV already carries the comparison. The process group is already
nccl, so RCCL needs no extra setup.preamble_fnruns outside the timed region, so thecopy_needed to givedist.all_reduceout-of-place semantics does not pollute the measurement.Two judgement calls worth reviewing
The RCCL baseline uses ordinary torch tensors, not Iris heap memory. The symmetric heap is allocated with the flags RMA requires, so timing RCCL against it would measure Iris's allocation choice rather than RCCL. Someone deciding between the two would call RCCL on normal tensors, so that is what is measured.
reduce_scatteris excluded.iris.ccl.reduce_scatteris(M, N) -> (M, N)— "each rank reduces its assigned tiles, stores locally" — whereastorch.distributed.reduce_scatter_tensoris(W*M, N) -> (M, N). Publishing those side by side would compare two different operations. Happy to add it if the intended correspondence is different from how I read it.all_to_allis included with a documented caveat: Iris splits(M, N*W)along dim 1,dist.all_to_all_singlesplits along dim 0, so the RCCL path uses(M*W, N). Identical communication pattern and total bytes —W-1peer messages ofM*Nelements — only the layout differs.Cost
Not on
pull_requestby design. Three collectives across two backends is enough work to crowd out the test jobs on a shared 8-GPU pool. Ranks, sizes and repeat count areworkflow_dispatchinputs; the default sweep is deliberately small (M=N ∈ {1024, 4096, 16384}, fp16, 8 ranks) and can be widened per run without editing anything.Output
The job writes the table into the run summary and uploads CSVs and PNGs as artifacts (30-day retention). Three figures are produced, matching triton-shmem's
plot_bench.py: bus bandwidth, latency, and the Iris/RCCL latency ratio.Results on MI355X
Run on 8x MI355X (gfx950), ROCm 7.2.1, PyTorch 2.10 — 576 measurements: 3 collectives x {2,4,8} ranks x 10 message sizes (2 MiB - 1 GiB) x {fp16, bf16}. Ratios are Iris/RCCL bus bandwidth; above 1.00x means Iris is faster.
Two things worth a reviewer's attention:
The rank-scaling behaviour splits by collective.
all_gatherandall_to_allare close to rank-independent at large messages and sit near parity regardless of world size, withall_to_allcrossing ahead at 8 ranks.all_reducemoves the other way as ranks double: 0.93x -> 0.84x -> 0.68x. Fixed per-launch overhead predicts the first pattern and not the second, which points at thetwo_shotalgorithm rather than at Iris's launch path.Small messages are overhead-dominated across the board (~0.5-0.6x at 2 MiB), converging as size grows. That is the expected shape and is not specific to any one collective.
Test plan
The sweep has been run on hardware: 576 measurements across the full
M x N x dtypegrid at 2, 4 and 8 ranks, with the figures and table above generated by the committed plotting path.The plotting and table path is additionally verified against the real
iris.benchCSV schema, including thatskipped=Truerows are dropped and a missing RCCL point renders as an em dash rather than a bogus ratio.One operational note for anyone sweeping many shapes in a single process: the benchmarks do not free symmetric buffers between shape combinations, so they accumulate.
all_to_allat 8 ranks over the whole grid exhausts a 64 GiB heap, since its output isworld_sizetimes the input. Splitting the sweep byM(one process per value) keeps each working set well inside the heap.