test(rccl): add host microtests for the profiler plugin by aduforest · Pull Request #13597 · ROCm/rocm-systems

aduforest · 2026-10-15T21:27:03Z

JIRA ID: AICOMRCCL-2818

What

New rccl-UnitTestsMicroProfiler (137 tests) for src/plugin/profiler.cc: plugin load/unload, event start/stop/record wrappers, the KernelCh profiler thread.
Also the profiler_v1..v7.cc adapters.

How

Fake v7 plugin and fake v1..v6 plugins behind test-local seams (ncclOpenProfilerPluginLib, ncclOsDlsym, dlsym/dladdr, logging, env); the hipified units are #included.
Thread tests drive the real profiler thread through fake device counter buffers.
Pins 4 production bugs as KnownBug_ tests:
- a failed plugin init() unloads twice, closing the lib under a live comm;
- 256-channel colls are advertised as 0 channels (uint8 truncation);
- the v5 adapter drops GroupStartApiStop/GroupEndApiStart (the core passes null args);
- the v1 adapter maps AlltoAll/AlltoAllv/Gather/Scatter to Send and bf16/fp8 to Float64.
Seen but not pinned, since asserting them would depend on UB; the p2p tests only check channels < 64: the p2p KernelCh post loop shifts 1ull << c for c up to MAXCHANNELS; the v4 adapter never copies p2p.nChannels into its uninitialised descriptor; with RCCL_ENABLE_PROXY_TRACE=1 the plugin name logged after load points at an out-of-scope soPath.

Coverage

profiler.cc 1147/1159 lines (the 12 misses are an allocation-failure path ncclMemoryPoolAlloc can't reach), profiler_v1..v7.cc 100% each.

Mutation Testing

72/72 killed (54 in profiler.cc, 18 across the adapters).

Test Plan

All 137 pass in Debug and Release (-O3) standalone builds, and --gtest_shuffle --gtest_repeat=20 is clean in both.

New rccl-UnitTestsMicroProfiler binary covering src/plugin/profiler.cc (plugin load/unload, the event start/stop/record wrappers and the KernelCh profiler thread) and the profiler_v1..v7.cc adapters, against a fake plugin behind test-local plugin-open and dlsym seams. Pins six production bugs as observed: a failed plugin init() is unloaded twice, 256-channel collectives are advertised with 0 channels, p2p KernelCh posts alias channels >= 64 onto c % 64, the v4 adapter never copies p2p.nChannels, the v5 adapter drops the group-API states, and the v1 adapter reports AlltoAll/Gather/Scatter as Send and bf16/fp8 as Float64. Co-Authored-By: Claude <noreply@anthropic.com>

therock-pr-bot · 2026-10-15T21:28:09Z

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🚫 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?

The p2p post test now asserts only channels below 64 (the post loop shift is undefined above), and the v4 test that read the uninitialised p2p.nChannels is dropped. Co-Authored-By: Claude <noreply@anthropic.com>

aduforest · 2026-10-15T21:31:13Z

@argus review

amd-argus

ARGUS

Findings

5 more finding(s), not posted inline

projects/rccl/test/host/profiler-test.cc:2136 (Low) A failing ASSERT here leaves local child in live_, so TearDown destroys a freed comm and onStart spins on dead locals
projects/rccl/test/host/profiler-test.cc:495 (Low) Asserted 'Successfully loaded ... proxytrace.so' log depends on use-after-scope of soPath in profiler.cc
projects/rccl/test/host/profiler-test.cc:572 (Low) Tests pinning known bugs carry only a // Bug: comment, no KnownBug_ prefix or tracking ID
projects/rccl/test/host/profiler-test.cc:2165 (Low) ProfilerThread_AbortWhileIdle_Exits passes with the abort check removed, since Destroy sets stop and joins anyway
projects/rccl/test/host/profiler-v5-test.cc:290 (Low) v4/v5 tests pin 'dropped event leaves *eHandle untouched', an adapter inconsistency rather than a contract

This PR changes 4,147 lines across 14 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: LGTM once profiler-test.cc:2215 waits for Inflight == 1 instead of reading it once right after the stop-count wait.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

Wait for the inflight count after the stops instead of reading it once, keep child comms and the blocking callback alive through TearDown, drop the proxy-trace success-log check (profiler.cc reads soPath after its scope ends), drop the idle-abort test that the stop path already covered, stop asserting the v4/v5 dropped-event handle, and name the bug-pinning tests KnownBug_. Co-Authored-By: Claude <noreply@anthropic.com>

aduforest · 2026-10-15T22:10:09Z

@argus re-review

amd-argus

ARGUS

This PR changes 4,133 lines across 14 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: LGTM.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

…comrccl-2818-profiler-host-microtests # Conflicts: # projects/rccl/test/host/MICROTEST_README.md # projects/rccl/test/host/run_host_tests.sh # projects/rccl/tools/scripts/test_runner/configs/ci-precheckin.json # projects/rccl/tools/scripts/test_runner/lib/test_executor.py

aduforest · 2026-10-16T17:11:33Z

@argus review

amd-argus

ARGUS

This PR changes 4,133 lines across 14 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: LGTM.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

…one-line pin comments Co-Authored-By: Claude <noreply@anthropic.com>

aduforest · 2026-10-16T20:16:16Z

@argus review

amd-argus

ARGUS

This PR changes 4,130 lines across 14 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: LGTM.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

…comrccl-2818-profiler-host-microtests # Conflicts: # projects/rccl/test/host/MICROTEST_README.md

aduforest · 2026-10-19T17:30:09Z

@argus review

amd-argus

ARGUS

This PR changes 4,168 lines across 14 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: LGTM.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

…comrccl-2818-profiler-host-microtests # Conflicts: # projects/rccl/test/host/MICROTEST_README.md # projects/rccl/tools/scripts/test_runner/lib/test_executor.py

aduforest · 2026-10-19T17:53:51Z

@argus review

amd-argus

ARGUS

This PR changes 4,158 lines across 14 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: LGTM.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

github-actions Bot added the project: rccl label Oct 15, 2026

test(rccl): keep the profiler microtests off undefined behaviour

663e481

The p2p post test now asserts only channels below 64 (the post loop shift is undefined above), and the v4 test that read the uninitialised p2p.nChannels is dropped. Co-Authored-By: Claude <noreply@anthropic.com>

systems-assistant Bot added the organization: ROCm label Oct 15, 2026

amd-argus Bot reviewed Oct 15, 2026

View reviewed changes

Comment thread projects/rccl/test/host/profiler-test.cc Outdated

amd-argus Bot reviewed Oct 15, 2026

View reviewed changes

aduforest marked this pull request as ready for review October 15, 2026 22:33

aduforest requested review from a team October 15, 2026 22:33

pvallem approved these changes Oct 16, 2026

View reviewed changes

amd-argus Bot reviewed Oct 16, 2026

View reviewed changes

dhanavikram approved these changes Oct 16, 2026

View reviewed changes

test(rccl): reset profiler plugin path, env and net lib in TearDown; …

7f44fee

…one-line pin comments Co-Authored-By: Claude <noreply@anthropic.com>

amd-argus Bot reviewed Oct 16, 2026

View reviewed changes

Merge remote-tracking branch 'origin/develop' into users/aduforest/ai…

3007718

…comrccl-2818-profiler-host-microtests # Conflicts: # projects/rccl/test/host/MICROTEST_README.md

amd-argus Bot reviewed Oct 19, 2026

View reviewed changes

Merge remote-tracking branch 'origin/develop' into users/aduforest/ai…

bc18989

…comrccl-2818-profiler-host-microtests # Conflicts: # projects/rccl/test/host/MICROTEST_README.md # projects/rccl/tools/scripts/test_runner/lib/test_executor.py

amd-argus Bot reviewed Oct 19, 2026

View reviewed changes

Conversation

aduforest commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

Coverage

Mutation Testing

Test Plan

Uh oh!

therock-pr-bot Bot commented Oct 15, 2026

✅ All Policy Checks Passed

Uh oh!

aduforest commented Oct 15, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Findings

Uh oh!

Uh oh!

aduforest commented Oct 15, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

aduforest commented Oct 16, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

aduforest commented Oct 16, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

aduforest commented Oct 19, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

aduforest commented Oct 19, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aduforest commented Oct 15, 2026 •

edited

Loading