Skip to content

feat(ds4): add default-off HIP GPU phase profiler#554

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

feat(ds4): add default-off HIP GPU phase profiler#554
cheese-cakee wants to merge 10 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

Adds an opt-in HIP GPU phase profiler for DeepSeek4 behind DFLASH_DS4_GPU_PROFILE (default off). rocprofv3 does not work on gfx1151, so per-phase GPU timing for the DS4 HIP path did not exist; this provides it with zero cost when disabled.

Stacked on #548: the profiler instruments the exact-versus-approximate verification boundary that #548 established, so this branch carries #548's two commits until it merges. The remaining diff after #548 merges is this feature alone.

Changes

  • deepseek4_gpu_profiler.{h,cpp} (new): env parsing via the existing boolean helpers, reusable HIP event pairs per scope, dynamic libroctx64.so loading for matching ROCTX ranges, stable [ds4-gpu-profile] clock=hip_event records.
  • deepseek4_graph.cpp: scopes around hc_pre, attention, moe_ffn, hc_post, and output_projection for exact verification forwards, plus a whole-graph scope for fused AR decode.
  • deepseek4_dspark_spec.cpp: whole verification step scope (exact batched and sequential paths).
  • deepseek4_fused_verify.inc: scope for the explicitly approximate fused research graph.
  • CMakeLists.txt, docs/DS4.md, docs/ENVIRONMENT.md: build wiring and flag documentation.

How it works

  • Unset and explicit 0 are both disabled: no HIP events created, no synchronization, no ROCTX library loaded, no log lines. The implementation is compiled in on all backends but event creation is #if defined(GGML_USE_HIP) guarded; non-HIP builds are no-ops.
  • Enabled: each scoped forward reuses per-scope start/stop events and emits one record per scope with gpu_ms and calls. Whole-graph paths (fused AR decode, approximate fused verifier) are one captured graph each, so they are honestly timed as whole graphs (fused_decode_graph, approx_fused_verify_graph).
  • ROCTX ranges are emitted only when libroctx64.so is found at runtime; there is no compile-time or link-time requirement.

Performance

Overhead (gfx1151, ROCm 7.2.4, fixed prompt/seed/q=4, 1 warmup + 5 measured requests per arm): profiler unset median 3397.8 ms decode, explicit 0 3429.1 ms (+0.92%, within run noise), enabled 3461.8 ms. All three arms produced the identical 32 generated token IDs.

Example output it produces (exact q=4 verification, mean over 6 enabled requests): whole step 260.711 ms; attention 140.750 ms over 172 calls (71.5% of timed GPU subphases), moe_ffn 53.590 ms over 43 calls (27.2%), output_projection 2.560 ms (1.3%); multi-token HC pre/post report 0 GPU calls because they execute on the host.

Limitations

Verification

  • Host: lucebox3, Radeon 8060S gfx1151, ROCm 7.2.4, HIP 7.2.53211, clang 22.0.0. Target /opt/models/DeepSeek-V4-Flash-IQ2XXS-w2Q2K-AProjQ8-SExpQ8-OutQ8-chat-v2-imatrix.gguf (SHA-256 efc7ed607ff27076e3e501fc3fefefa33c0ed8cf1eff483a2b7fdc0c2e616668), draft ds4-dspark-draft.gguf (SHA-256 48883d35b8a67ecfd2858a90e12a47d04cb5ac581acef868ca0f58544816f746).
  • base fix(ds4): preserve exact speculative verification #548 on 405ab02: fresh Release configure (HIP, gfx1151, GGML_CUDA_GRAPHS=ON, GGML_CUDA_FA=OFF, GGML_HIP_NO_VMM=ON, GGML_HIP_MMQ_MFMA=ON, DFLASH27B_HIP_SM80_EQUIV=OFF, GGML_HIP_ROCWMMA_FATTN=OFF); dflash_server and test_deepseek4_unit build and pass. Unset / =0 / =1 arms each 1 warmup + 5 measured requests: identical 32 IDs in every arm, zero [ds4-gpu-profile] lines when disabled, expected records when enabled. A sequential one-token exact forward baseline was also recorded. Local CUDA 12.6/sm_89 compile of dflash_common and test_deepseek4_unit passed.
  • this branch rebased onto upstream/main 62880e2: incremental HIP rebuild passed, test_deepseek4_unit passed, and a two-arm smoke (unset vs =1, same fixed request) reproduced identical 32 IDs [2337 344 260 2405 3417 14 305 270 3287 344 850 4190 1099 436 6179 16 2454 7367 1664 834 4031 3986 14 790 4562 260 8682 294 5131 8753 305 2883], 0 profile lines unset vs 103 enabled. git diff --check clean.

Review in cubic

DFLASH_DS4_GPU_PROFILE=1 scopes HIP event timing around exact verification phases (hc_pre, attention, moe_ffn, hc_post, output_projection), the whole verification step, fused AR decode, and the explicitly approximate fused research graph. ROCTX ranges are emitted when libroctx64.so is present; it is loaded dynamically, so there is no compile- or link-time dependency. Unset and explicit 0 create no events, synchronize nothing, and emit no records.

@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.

1 issue found across 10 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="server/src/deepseek4/deepseek4_dspark_spec.cpp">

<violation number="1" location="server/src/deepseek4/deepseek4_dspark_spec.cpp:710">
P3: When `DFLASH_DS4_PARITY_TRACE=1`, this scans the full vocab for every verified row on every decode step and emits one stderr line per row; for large vocabularies and long generations this can add non-trivial CPU overhead and log volume. Worth noting as a debug-only cost even though it's opt-in and off by default.</violation>
</file>

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
Comment thread server/src/deepseek4/deepseek4_gpu_profiler.cpp Outdated
Comment thread server/src/deepseek4/deepseek4_graph.cpp
if (parity_trace) {
std::vector<float> trace_logits;
if (target.read_verify_logits(q, trace_logits)) {
for (int row = 0; row < q; ++row) {

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.

P3: When DFLASH_DS4_PARITY_TRACE=1, this scans the full vocab for every verified row on every decode step and emits one stderr line per row; for large vocabularies and long generations this can add non-trivial CPU overhead and log volume. Worth noting as a debug-only cost even though it's opt-in and off by default.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_dspark_spec.cpp, line 710:

<comment>When `DFLASH_DS4_PARITY_TRACE=1`, this scans the full vocab for every verified row on every decode step and emits one stderr line per row; for large vocabularies and long generations this can add non-trivial CPU overhead and log volume. Worth noting as a debug-only cost even though it's opt-in and off by default.</comment>

<file context>
@@ -680,6 +704,31 @@ bool run_deepseek4_dspark_spec_decode(
+        if (parity_trace) {
+            std::vector<float> trace_logits;
+            if (target.read_verify_logits(q, trace_logits)) {
+                for (int row = 0; row < q; ++row) {
+                    const float * logits = trace_logits.data() + (size_t) row * target_w.n_vocab;
+                    int best = 0;
</file context>

end(Count) while idle passed the active-phase check and indexed one past the elapsed/calls arrays. Match the sentinel guard begin() already has.
Records from deepseek4_step_layer_range now carry layers=<begin>-<end> so per-shard timings are distinguishable in layer-split runs. Single-shard and whole-graph scopes omit the field.
emit() no longer infers the exact-verification zero-call policy from the caller's scope/mode strings; the exact verify call site passes emit_zero_core directly.
The documented regeneration grep only matches literal getenv calls; flags read through wrapper helpers such as ds4_env_flag are curated manually.
begin() now runs before the per-phase Ds4TimingClock window and end() after the telemetry accumulation, so enabling DFLASH_DS4_GPU_PROFILE no longer inflates the pre-existing host telemetry counters with event synchronization. GPU event ordering is unchanged. The output-projection block keeps its bracket because tensor_get enqueues device work after compute.
DFLASH_DS4_PARITY_TRACE scans the full vocabulary for every verified row and prints one stderr line per row; document it as a short-run debug tool in the documented table and add it to the wrapper-curated inventory.

@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
Group scope, mode, n_tokens, kv_start, layer range, and emit_zero_core into Ds4GpuProfileOptions. Call sites assign fields by name, so the growing option list can no longer be misordered positionally.

@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.

1 issue found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="server/src/deepseek4/deepseek4_gpu_profiler.h">

<violation number="1" location="server/src/deepseek4/deepseek4_gpu_profiler.h:20">
P3: Ds4GpuProfileOptions defaults scope/mode to nullptr, which were previously required constructor arguments; if a future call site forgets to set them, emit() will format a null pointer with %s (undefined behavior, though glibc happens to print "(null)"). Consider keeping these fields without a safe default (or asserting non-null in the constructor) so the requirement isn't silently lost in the refactor.</violation>
</file>

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

Re-trigger cubic

};

struct Ds4GpuProfileOptions {
const char * scope = nullptr;

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.

P3: Ds4GpuProfileOptions defaults scope/mode to nullptr, which were previously required constructor arguments; if a future call site forgets to set them, emit() will format a null pointer with %s (undefined behavior, though glibc happens to print "(null)"). Consider keeping these fields without a safe default (or asserting non-null in the constructor) so the requirement isn't silently lost in the refactor.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_gpu_profiler.h, line 20:

<comment>Ds4GpuProfileOptions defaults scope/mode to nullptr, which were previously required constructor arguments; if a future call site forgets to set them, emit() will format a null pointer with %s (undefined behavior, though glibc happens to print "(null)"). Consider keeping these fields without a safe default (or asserting non-null in the constructor) so the requirement isn't silently lost in the refactor.</comment>

<file context>
@@ -16,11 +16,19 @@ enum class Ds4GpuPhase : uint8_t {
 };
 
+struct Ds4GpuProfileOptions {
+    const char * scope = nullptr;
+    const char * mode = nullptr;
+    int n_tokens = 0;
</file context>

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