Skip to content

fix(rccl): Redirect P2P SendRecv protocol logs to NCCL_COLL debug subsystem - #11010

Merged
rahulvaidya20 merged 1 commit into
developfrom
users/rahulvaidya20/rccl-p2p-log-subsys
Sep 1, 2026
Merged

rahulvaidya20 merged 1 commit into
developfrom
users/rahulvaidya20/rccl-p2p-log-subsys

Conversation

@rahulvaidya20

Copy link
Copy Markdown
Contributor

Motivation

PR #10243 added INFO(NCCL_INIT, "RCCL P2P SendRecv protocol=...") in the P2P enqueue path. Because NCCL_INIT is in the default debug subsystem mask, this fires per send/recv direction per op under plain NCCL_DEBUG=INFO. For a 1 GB alltoall across 8 GPUs with -n 1000, this produces ~250k extra log lines (12.9× blowup) per run.

Technical Details

Move the subsystem tag from NCCL_INIT to NCCL_COLL. NCCL_COLL is not in the default mask (NCCL_INIT | NCCL_BOOTSTRAP | NCCL_ENV), so the line becomes opt-in via NCCL_DEBUG_SUBSYS=COLL. The log line itself and the string it emits are unchanged; tests are updated to set NCCL_DEBUG_SUBSYS=COLL instead of INIT.

Issue Tracking

JIRA ID: ROCM-30236

Test Plan

Run SendRecv.* unit tests (the 7 LL128/LegacyLL protocol-scraping tests introduced by #10243) with UT_POW2_GPUS=1 on MI355X gfx950.

Test Result

All 7 tests pass on MI355X (gfx950):

  • SendRecv.LegacyLLIntranode — PASS
  • SendRecv.LL128NetBuffersDisabled — PASS
  • SendRecv.LL128NetBuffersEnabled — PASS
  • SendRecv.Ll128DisabledDoesNotEnableP2pLL128 — PASS
  • SendRecv.Ll128EnabledDoesNotEnableP2pLL128 — PASS
  • SendRecv.SeparateThresholdLL128UsesLL128Knob — PASS
  • SendRecv.SeparateThresholdLegacyLLUsesLLKnob — PASS

Submission Checklist

Signed-off-by: Rahul Vaidya <ravaidya@amd.com>
@therock-pr-bot

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/rccl/src/enqueue.cc, projects/rccl/test/SendRecvTests.cpp; no test file found
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@rahulvaidya20
rahulvaidya20 marked this pull request as ready for review August 31, 2026 23:35
@rahulvaidya20
rahulvaidya20 requested review from a team and a lite review from Copilot August 31, 2026 23:35
@rahulvaidya20 rahulvaidya20 changed the title fix(rccl): redirect SendRecv protocol logs to NCCL_COLL debug subsystem fix(rccl): Redirect P2P SendRecv protocol logs to NCCL_COLL debug subsystem Aug 31, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces default NCCL_DEBUG=INFO log volume by retagging the per-op P2P SendRecv protocol-selection log line from the default-enabled NCCL_INIT subsystem to the opt-in NCCL_COLL subsystem, and updates the associated unit tests accordingly.

Changes:

  • Retag RCCL P2P SendRecv protocol=... logging from INFO(NCCL_INIT, ...) to INFO(NCCL_COLL, ...) in the P2P enqueue path.
  • Update SendRecv unit tests to enable NCCL_DEBUG_SUBSYS=COLL when scraping logs for protocol selection.
  • Adjust related comments to reflect the new debug subsystem requirement.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
projects/rccl/src/enqueue.cc Moves the P2P SendRecv protocol-selection INFO log from NCCL_INIT to NCCL_COLL so it’s no longer emitted under the default debug-subsystem mask.
projects/rccl/test/SendRecvTests.cpp Updates SendRecv protocol-scraping tests to set NCCL_DEBUG_SUBSYS=COLL so they continue to observe the protocol-selection log line.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread projects/rccl/test/SendRecvTests.cpp Resolved
@rahulvaidya20
rahulvaidya20 merged commit 2240f84 into develop Sep 1, 2026
59 of 68 checks passed
@rahulvaidya20
rahulvaidya20 deleted the users/rahulvaidya20/rccl-p2p-log-subsys branch September 1, 2026 01:58
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

corey-derochie-amd pushed a commit that referenced this pull request Oct 14, 2026
…system (#11010)

## Motivation

PR #10243 added `INFO(NCCL_INIT, "RCCL P2P SendRecv protocol=...")` in
the P2P enqueue path. Because `NCCL_INIT` is in the default debug
subsystem mask, this fires per send/recv direction per op under plain
`NCCL_DEBUG=INFO`. For a 1 GB alltoall across 8 GPUs with `-n 1000`,
this produces ~250k extra log lines (12.9× blowup) per run.

## Technical Details

Move the subsystem tag from `NCCL_INIT` to `NCCL_COLL`. `NCCL_COLL` is
not in the default mask (`NCCL_INIT | NCCL_BOOTSTRAP | NCCL_ENV`), so
the line becomes opt-in via `NCCL_DEBUG_SUBSYS=COLL`. The log line
itself and the string it emits are unchanged; tests are updated to set
`NCCL_DEBUG_SUBSYS=COLL` instead of `INIT`.

## Issue Tracking

JIRA ID: ROCM-30236

## Test Plan

Run `SendRecv.*` unit tests (the 7 LL128/LegacyLL protocol-scraping
tests introduced by #10243) with `UT_POW2_GPUS=1` on MI355X gfx950.

## Test Result

All 7 tests pass on MI355X (gfx950):
- `SendRecv.LegacyLLIntranode` — PASS
- `SendRecv.LL128NetBuffersDisabled` — PASS
- `SendRecv.LL128NetBuffersEnabled` — PASS
- `SendRecv.Ll128DisabledDoesNotEnableP2pLL128` — PASS
- `SendRecv.Ll128EnabledDoesNotEnableP2pLL128` — PASS
- `SendRecv.SeparateThresholdLL128UsesLL128Knob` — PASS
- `SendRecv.SeparateThresholdLegacyLLUsesLLKnob` — PASS

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

Signed-off-by: Rahul Vaidya <ravaidya@amd.com>
corey-derochie-amd pushed a commit that referenced this pull request Oct 15, 2026
…system (#11010)

## Motivation

PR #10243 added `INFO(NCCL_INIT, "RCCL P2P SendRecv protocol=...")` in
the P2P enqueue path. Because `NCCL_INIT` is in the default debug
subsystem mask, this fires per send/recv direction per op under plain
`NCCL_DEBUG=INFO`. For a 1 GB alltoall across 8 GPUs with `-n 1000`,
this produces ~250k extra log lines (12.9× blowup) per run.

## Technical Details

Move the subsystem tag from `NCCL_INIT` to `NCCL_COLL`. `NCCL_COLL` is
not in the default mask (`NCCL_INIT | NCCL_BOOTSTRAP | NCCL_ENV`), so
the line becomes opt-in via `NCCL_DEBUG_SUBSYS=COLL`. The log line
itself and the string it emits are unchanged; tests are updated to set
`NCCL_DEBUG_SUBSYS=COLL` instead of `INIT`.

## Issue Tracking

JIRA ID: ROCM-30236

## Test Plan

Run `SendRecv.*` unit tests (the 7 LL128/LegacyLL protocol-scraping
tests introduced by #10243) with `UT_POW2_GPUS=1` on MI355X gfx950.

## Test Result

All 7 tests pass on MI355X (gfx950):
- `SendRecv.LegacyLLIntranode` — PASS
- `SendRecv.LL128NetBuffersDisabled` — PASS
- `SendRecv.LL128NetBuffersEnabled` — PASS
- `SendRecv.Ll128DisabledDoesNotEnableP2pLL128` — PASS
- `SendRecv.Ll128EnabledDoesNotEnableP2pLL128` — PASS
- `SendRecv.SeparateThresholdLL128UsesLL128Knob` — PASS
- `SendRecv.SeparateThresholdLegacyLLUsesLLKnob` — PASS

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

Signed-off-by: Rahul Vaidya <ravaidya@amd.com>
corey-derochie-amd pushed a commit that referenced this pull request Oct 15, 2026
…system (#11010)

## Motivation

PR #10243 added `INFO(NCCL_INIT, "RCCL P2P SendRecv protocol=...")` in
the P2P enqueue path. Because `NCCL_INIT` is in the default debug
subsystem mask, this fires per send/recv direction per op under plain
`NCCL_DEBUG=INFO`. For a 1 GB alltoall across 8 GPUs with `-n 1000`,
this produces ~250k extra log lines (12.9× blowup) per run.

## Technical Details

Move the subsystem tag from `NCCL_INIT` to `NCCL_COLL`. `NCCL_COLL` is
not in the default mask (`NCCL_INIT | NCCL_BOOTSTRAP | NCCL_ENV`), so
the line becomes opt-in via `NCCL_DEBUG_SUBSYS=COLL`. The log line
itself and the string it emits are unchanged; tests are updated to set
`NCCL_DEBUG_SUBSYS=COLL` instead of `INIT`.

## Issue Tracking

JIRA ID: ROCM-30236

## Test Plan

Run `SendRecv.*` unit tests (the 7 LL128/LegacyLL protocol-scraping
tests introduced by #10243) with `UT_POW2_GPUS=1` on MI355X gfx950.

## Test Result

All 7 tests pass on MI355X (gfx950):
- `SendRecv.LegacyLLIntranode` — PASS
- `SendRecv.LL128NetBuffersDisabled` — PASS
- `SendRecv.LL128NetBuffersEnabled` — PASS
- `SendRecv.Ll128DisabledDoesNotEnableP2pLL128` — PASS
- `SendRecv.Ll128EnabledDoesNotEnableP2pLL128` — PASS
- `SendRecv.SeparateThresholdLL128UsesLL128Knob` — PASS
- `SendRecv.SeparateThresholdLegacyLLUsesLLKnob` — PASS

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

Signed-off-by: Rahul Vaidya <ravaidya@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants