Add KernelStep profiler events for Simple-prims per-slice GPU timing. - #2316
Open
alizmhdi wants to merge 3 commits into
Open
Add KernelStep profiler events for Simple-prims per-slice GPU timing.#2316alizmhdi wants to merge 3 commits into
alizmhdi wants to merge 3 commits into
Conversation
Instrument device stamps, proxy drain, and the example plugin so Coll/P2p parents can nest KernelStep start/stop (v6 API) for intra-host analysis. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
🟡 Not ready to approve
The example profiler’s KernelStep pool never advances its base index on completion, which will exhaust the pool and drop all subsequent KernelStep events.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR adds a new KernelStep profiler event (v6) to capture per-slice GPU globaltimer start/stop timing for Simple-protocol “simple prims”, plumbing device-side stamps through proxy-side draining so Coll/P2p parents can nest these step events for intra-host performance analysis.
Changes:
- Introduces device-side KernelStep dual rings + sequence counter and instruments
prims_simple.hto emit per-slice start/stop stamps. - Adds host-side proxy draining logic to convert completed start/stop pairs into profiler plugin
KernelStepevents. - Extends profiler API surface (event mask/state/descriptor) and updates the example profiler plugin to allocate, nest, and print
KernelStepevents; masks the bit for older v1–v5 plugins.
File summaries
| File | Description |
|---|---|
| src/transport/profiler.cc | Adds proxy-side draining of KernelStep start/stop ring pairs into plugin events. |
| src/proxy.cc | Threads KernelStep ring pointers through proxy op/sub-args when profiler proxy ops are created. |
| src/plugin/profiler/profiler_v5.cc | Masks unsupported KernelStep bit for v5 plugins. |
| src/plugin/profiler/profiler_v4.cc | Masks unsupported KernelStep bit for v4 plugins. |
| src/plugin/profiler/profiler_v3.cc | Masks unsupported KernelStep bit for v3 plugins. |
| src/plugin/profiler/profiler_v2.cc | Masks unsupported KernelStep bit for v2 plugins. |
| src/plugin/profiler/profiler_v1.cc | Masks unsupported KernelStep bit for v1 plugins. |
| src/plugin/profiler.cc | Adds KernelStep to masks/printing and implements start/stop wrappers + proxy requirement update. |
| src/os/windows_stubs.cc | Adds Windows stubs for the new KernelStep wrapper APIs. |
| src/init.cc | Allocates/publishes KernelStep rings, sequence counter buffer, and in-flight handle storage. |
| src/include/proxy.h | Extends proxy op/sub-args to carry KernelStep ring pointers. |
| src/include/profiler.h | Extends ncclProfilerProxy state with KernelStep rings, seq/counters, and handle slots; adds wrapper prototypes. |
| src/include/plugin/profiler/profiler_v6.h | Extends v6 event descriptor union with kernelStep fields. |
| src/include/plugin/profiler/profiler_v5.h | Extends v5 state-args union with kernelStep timing field (used by v6 typedef). |
| src/include/plugin/nccl_profiler.h | Adds ncclProfileKernelStep mask bit and ncclProfilerKernelStepStop state. |
| src/include/device.h | Adds KernelStep ring/event types and work-item profilerStepEnabled flags; extends kernel-comm with KernelStep pointers. |
| src/enqueue.cc | Propagates ncclProfileKernelStep into device work items via profilerStepEnabled. |
| src/device/prims_simple.h | Emits KernelStep start/stop stamps for per-slice timing and wires enable flag into primitives. |
| src/device/common.h | Adds device helpers for KernelStep stamping and ensures work-boundary markers exist for KernelStep-only mode. |
| plugins/profiler/example/README.md | Documents KernelStep mask/pool sizing and trace output fields. |
| plugins/profiler/example/print_event.cc | Adds Chrome-trace printing for nested/orphan KernelStep events. |
| plugins/profiler/example/plugin.cc | Adds KernelStep pool allocation, nesting under task parents, and state handling in the example plugin. |
| plugins/profiler/example/nccl/profiler.h | Mirrors core header updates for the example plugin’s bundled NCCL headers. |
| plugins/profiler/example/nccl/profiler_v6.h | Mirrors v6 descriptor updates for the example plugin’s bundled NCCL headers. |
| plugins/profiler/example/nccl/profiler_v5.h | Mirrors v5 state-args updates for the example plugin’s bundled NCCL headers. |
| plugins/profiler/example/event.h | Adds kernelStep event struct and parent task nesting pointers + context pool fields. |
Review details
- Files reviewed: 26/26 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Comment on lines
+989
to
+991
| struct kernelStep* event = (struct kernelStep *)handle; | ||
| event->stopTs = gettime() - startTime; | ||
| debugEvent(event, "KernelStepStop"); |
| if (isSendNotRecv) ncclShmem.groups[group].kernelStepSeqSend[index] = seq; | ||
| else ncclShmem.groups[group].kernelStepSeqRecv[index] = seq; | ||
| } | ||
| barrier(); // Has couterpart in preceding worker-only loop. |
Keep KernelStep API headers and core NCCL instrumentation; drop chrome-trace example plugin handling so training uses CoMMA only. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Instrument device stamps, proxy drain, and the example plugin so Coll/P2p parents can nest KernelStep start/stop (v6 API) for intra-host analysis.
Description
Related Issues
Changes & Impact
Performance Impact