Skip to content

feat(ds4): add external ROCTX phase markers - #554

Open
cheese-cakee wants to merge 2 commits into
Luce-Org:mainfrom
cheese-cakee:codex/perf-ds4-gpu-profiler
Open

feat(ds4): add external ROCTX phase markers#554
cheese-cakee wants to merge 2 commits into
Luce-Org:mainfrom
cheese-cakee:codex/perf-ds4-gpu-profiler

Conversation

@cheese-cakee

@cheese-cakee cheese-cakee commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add default-off semantic ROCTX ranges for external ROCm tracing
  • cover monolithic and heterogeneous prefill, speculative decode, and leaf layer ranges
  • label exact prefill, decode, verification, and ambiguous remote work explicitly
  • dynamically load ROCTX only when enabled and keep non-HIP builds as no-ops

This replaces the superseded historical in-process HIP-event profiler on current main. The old implementation recorded on a null/default stream and synchronized events, which could fence unrelated streams and distort the two-owner overlap being measured. This rewrite intentionally performs no internal GPU timing.

Behavior contract

DFLASH_DS4_ROCTX=1 enables balanced semantic ranges for correlation with an external rocprof kernel/copy trace. The marker layer:

  • uses no HIP/CUDA events
  • performs no stream or device synchronization
  • calculates no elapsed time
  • remains silent and does not load ROCTX when unset or false
  • reports a one-time diagnostic when explicitly enabled but ROCTX cannot be loaded
  • closes incomplete library handles when required symbols are missing
  • balances successful pushes through RAII, including early-return paths
  • avoids recursive layer-range double counting by marking leaf ranges only

Validation completed

  • strict native C++17 standalone build and tests passed
  • strict HIP-defined Linux standalone build and tests passed with dynamic-loader linkage
  • git diff --check passed
  • fresh independent final-diff review: no remaining actionable findings

Pending before ready-for-review / profiling claims

  • full current-tree CUDA build and CTest
  • full HIP build
  • gfx1151 execution with a real rocprof marker/kernel/copy capture
  • marker-to-kernel alignment inspection for monolithic and heterogeneous exact prefill
  • alternating enabled/disabled runs to quantify instrumentation perturbation

The Lucebox machines are currently unavailable, so this is reopened as a draft. The completed standalone checks do not constitute target-hardware validation, and no performance result is claimed. Historical CI and reviews applied to the superseded event-timing implementation and should not be treated as evidence for this rewrite.

Stack

Independent current-main PR. It does not depend on #548 or the #569/#575/#577 performance stack. #575 may provide a useful external rocprof collection wrapper, but it is not a code prerequisite.

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 10 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread server/src/deepseek4/deepseek4_gpu_profiler.cpp Outdated
Comment thread server/src/deepseek4/deepseek4_graph.cpp Outdated
Comment thread server/docs/ENVIRONMENT.md Outdated
Comment thread server/src/deepseek4/deepseek4_gpu_profiler.cpp Outdated
Comment thread server/src/deepseek4/deepseek4_graph.cpp Outdated
Comment thread server/src/deepseek4/deepseek4_dspark_spec.cpp Outdated

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 6 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread server/src/deepseek4/deepseek4_gpu_profiler.h Outdated

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 5 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread server/src/deepseek4/deepseek4_gpu_profiler.h Outdated
@cheese-cakee cheese-cakee changed the title feat(ds4): add default-off HIP GPU phase profiler feat(ds4): add external ROCTX phase markers Aug 4, 2026
@cheese-cakee cheese-cakee reopened this Aug 4, 2026
@cheese-cakee
cheese-cakee marked this pull request as draft August 4, 2026 20:50
@cheese-cakee
cheese-cakee force-pushed the codex/perf-ds4-gpu-profiler branch from 90ac4e5 to 3aaf9b1 Compare August 4, 2026 20:50
@cheese-cakee
cheese-cakee marked this pull request as ready for review August 4, 2026 21:38

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 10 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread server/src/deepseek4/deepseek4_roctx.cpp Outdated
Comment thread server/src/deepseek4/deepseek4_roctx.h Outdated
Carry semantic phase by value through ROCTX scopes and mixed-layer IPC so
remote Halo shard markers label decode correctly instead of unspecified.
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.

1 participant