feat(ds4): add default-off HIP GPU phase profiler#554
Conversation
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.
There was a problem hiding this comment.
6 issues 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/docs/ENVIRONMENT.md">
<violation number="1" location="server/docs/ENVIRONMENT.md:61">
P3: This new inventory line won't be reproduced by the documented regeneration command (`grep -rE 'getenv("[A-Z0-9_]+")' server/src`), since the actual reads go through `ds4_env_flag(name)`/`spec_env_flag(name)` wrappers rather than a literal `getenv("DFLASH_DS4_GPU_PROFILE")` call. Consider noting this is manually curated or adjusting the generation instructions so the 'generated' section stays trustworthy.</violation>
</file>
<file name="server/src/deepseek4/deepseek4_gpu_profiler.cpp">
<violation number="1" location="server/src/deepseek4/deepseek4_gpu_profiler.cpp:159">
P2: end() lacks the same `phase == Ds4GpuPhase::Count` guard that begin() has, so calling end(Ds4GpuPhase::Count) while idle causes an out-of-bounds write into elapsed_ms/calls (index == kPhaseCount), corrupting adjacent struct fields. Add the same sentinel check used in begin() for defense-in-depth.</violation>
<violation number="2" location="server/src/deepseek4/deepseek4_gpu_profiler.cpp:182">
P3: emit() bakes in the literal strings "forward"/"exact_verify" to decide whether to force-emit zero-call core phases, coupling the generic profiler to one specific caller's semantics; if that caller's scope/mode strings ever change, the zero-emit behavior silently breaks with no compile-time signal.</violation>
</file>
<file name="server/src/deepseek4/deepseek4_graph.cpp">
<violation number="1" location="server/src/deepseek4/deepseek4_graph.cpp:5864">
P2: The GPU profiler record emitted per `deepseek4_step_layer_range` call doesn't include `layer_begin`/`layer_end`, so in a layer-sharded (multi-device pipeline) setup, each shard's step produces a profiler line with identical scope/mode/tokens/kv_start, making it impossible to tell which shard a given `hc_pre`/`attention`/`moe_ffn` timing belongs to. Consider including the layer range in the emitted mode/scope string to disambiguate.</violation>
<violation number="2" location="server/src/deepseek4/deepseek4_graph.cpp:6158">
P3: Wrapping each phase's `ggml_backend_graph_compute` with `gpu_profiler.begin()/end()` (which synchronously blocks on `hipEventSynchronize`) inside the same timing window used for existing `telemetry->*_compute_us` counters means enabling the profiler silently skews the pre-existing CPU telemetry, not just adds a separate GPU metric.</violation>
</file>
<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
|
|
||
| void Ds4GpuProfiler::end(Ds4GpuPhase phase) { | ||
| #if defined(GGML_USE_HIP) | ||
| if (!impl_ || impl_->active != phase) return; |
There was a problem hiding this comment.
P2: end() lacks the same phase == Ds4GpuPhase::Count guard that begin() has, so calling end(Ds4GpuPhase::Count) while idle causes an out-of-bounds write into elapsed_ms/calls (index == kPhaseCount), corrupting adjacent struct fields. Add the same sentinel check used in begin() for defense-in-depth.
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.cpp, line 159:
<comment>end() lacks the same `phase == Ds4GpuPhase::Count` guard that begin() has, so calling end(Ds4GpuPhase::Count) while idle causes an out-of-bounds write into elapsed_ms/calls (index == kPhaseCount), corrupting adjacent struct fields. Add the same sentinel check used in begin() for defense-in-depth.</comment>
<file context>
@@ -0,0 +1,197 @@
+
+void Ds4GpuProfiler::end(Ds4GpuPhase phase) {
+#if defined(GGML_USE_HIP)
+ if (!impl_ || impl_->active != phase) return;
+ roctx_api().pop();
+ impl_->active = Ds4GpuPhase::Count;
</file context>
| ds4_backend_is_hip(backend) && ds4_env_flag("DFLASH_DS4_GPU_PROFILE"); | ||
| const char * gpu_profile_mode = verify_hooks ? "exact_verify" : | ||
| (n_tokens == 1 ? "decode" : "prefill"); | ||
| Ds4GpuProfiler gpu_profiler(gpu_profile_enabled, "forward", gpu_profile_mode, |
There was a problem hiding this comment.
P2: The GPU profiler record emitted per deepseek4_step_layer_range call doesn't include layer_begin/layer_end, so in a layer-sharded (multi-device pipeline) setup, each shard's step produces a profiler line with identical scope/mode/tokens/kv_start, making it impossible to tell which shard a given hc_pre/attention/moe_ffn timing belongs to. Consider including the layer range in the emitted mode/scope string to disambiguate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_graph.cpp, line 5864:
<comment>The GPU profiler record emitted per `deepseek4_step_layer_range` call doesn't include `layer_begin`/`layer_end`, so in a layer-sharded (multi-device pipeline) setup, each shard's step produces a profiler line with identical scope/mode/tokens/kv_start, making it impossible to tell which shard a given `hc_pre`/`attention`/`moe_ffn` timing belongs to. Consider including the layer range in the emitted mode/scope string to disambiguate.</comment>
<file context>
@@ -5846,6 +5857,12 @@ bool deepseek4_step_layer_range(
+ ds4_backend_is_hip(backend) && ds4_env_flag("DFLASH_DS4_GPU_PROFILE");
+ const char * gpu_profile_mode = verify_hooks ? "exact_verify" :
+ (n_tokens == 1 ? "decode" : "prefill");
+ Ds4GpuProfiler gpu_profiler(gpu_profile_enabled, "forward", gpu_profile_mode,
+ n_tokens, kv_start);
</file context>
| - `DFLASH_DRAFT_KV` - laguna_backend.cpp, qwen35_backend.cpp | ||
| - `DFLASH_DRAFT_PERSIST` - laguna_backend.cpp | ||
| - `DFLASH_DROP_COLD` - qwen35moe_backend.cpp, qwen35moe_pipelined_decode.cpp | ||
| - `DFLASH_DS4_GPU_PROFILE` - deepseek4_graph.cpp, deepseek4_dspark_spec.cpp |
There was a problem hiding this comment.
P3: This new inventory line won't be reproduced by the documented regeneration command (grep -rE 'getenv("[A-Z0-9_]+")' server/src), since the actual reads go through ds4_env_flag(name)/spec_env_flag(name) wrappers rather than a literal getenv("DFLASH_DS4_GPU_PROFILE") call. Consider noting this is manually curated or adjusting the generation instructions so the 'generated' section stays trustworthy.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/docs/ENVIRONMENT.md, line 61:
<comment>This new inventory line won't be reproduced by the documented regeneration command (`grep -rE 'getenv("[A-Z0-9_]+")' server/src`), since the actual reads go through `ds4_env_flag(name)`/`spec_env_flag(name)` wrappers rather than a literal `getenv("DFLASH_DS4_GPU_PROFILE")` call. Consider noting this is manually curated or adjusting the generation instructions so the 'generated' section stays trustworthy.</comment>
<file context>
@@ -57,6 +58,7 @@ consolidation of this list into CLI flags is tracked as follow-up work.
- `DFLASH_DRAFT_KV` - laguna_backend.cpp, qwen35_backend.cpp
- `DFLASH_DRAFT_PERSIST` - laguna_backend.cpp
- `DFLASH_DROP_COLD` - qwen35moe_backend.cpp, qwen35moe_pipelined_decode.cpp
+- `DFLASH_DS4_GPU_PROFILE` - deepseek4_graph.cpp, deepseek4_dspark_spec.cpp
- `DFLASH_DS4_TIMING` - deepseek4_target_shard_ipc_daemon.cpp
- `DFLASH_EXPERT_BUDGET_MB` - deepseek4_backend.cpp, laguna_backend.cpp, qwen35moe_backend.cpp
</file context>
| uint64_t total_calls = 0; | ||
| for (uint64_t calls : impl_->calls) total_calls += calls; | ||
| if (total_calls == 0) return; | ||
| const bool emit_zero_core = std::strcmp(impl_->scope, "forward") == 0 && |
There was a problem hiding this comment.
P3: emit() bakes in the literal strings "forward"/"exact_verify" to decide whether to force-emit zero-call core phases, coupling the generic profiler to one specific caller's semantics; if that caller's scope/mode strings ever change, the zero-emit behavior silently breaks with no compile-time signal.
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.cpp, line 182:
<comment>emit() bakes in the literal strings "forward"/"exact_verify" to decide whether to force-emit zero-call core phases, coupling the generic profiler to one specific caller's semantics; if that caller's scope/mode strings ever change, the zero-emit behavior silently breaks with no compile-time signal.</comment>
<file context>
@@ -0,0 +1,197 @@
+ uint64_t total_calls = 0;
+ for (uint64_t calls : impl_->calls) total_calls += calls;
+ if (total_calls == 0) return;
+ const bool emit_zero_core = std::strcmp(impl_->scope, "forward") == 0 &&
+ std::strcmp(impl_->mode, "exact_verify") == 0;
+ for (size_t i = 0; i < kPhaseCount; ++i) {
</file context>
| } | ||
| const auto hc_pre_attn_compute_t0 = Ds4TimingClock::now(); | ||
| if (!ds4_try_gpu_hc_pre_device(cached.sg.hidden_states, | ||
| gpu_profiler.begin(Ds4GpuPhase::HcPre); |
There was a problem hiding this comment.
P3: Wrapping each phase's ggml_backend_graph_compute with gpu_profiler.begin()/end() (which synchronously blocks on hipEventSynchronize) inside the same timing window used for existing telemetry->*_compute_us counters means enabling the profiler silently skews the pre-existing CPU telemetry, not just adds a separate GPU metric.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_graph.cpp, line 6158:
<comment>Wrapping each phase's `ggml_backend_graph_compute` with `gpu_profiler.begin()/end()` (which synchronously blocks on `hipEventSynchronize`) inside the same timing window used for existing `telemetry->*_compute_us` counters means enabling the profiler silently skews the pre-existing CPU telemetry, not just adds a separate GPU metric.</comment>
<file context>
@@ -6138,7 +6155,8 @@ bool deepseek4_step_layer_range(
}
const auto hc_pre_attn_compute_t0 = Ds4TimingClock::now();
- if (!ds4_try_gpu_hc_pre_device(cached.sg.hidden_states,
+ gpu_profiler.begin(Ds4GpuPhase::HcPre);
+ const bool hc_pre_ok = ds4_try_gpu_hc_pre_device(cached.sg.hidden_states,
cached.post,
</file context>
| if (parity_trace) { | ||
| std::vector<float> trace_logits; | ||
| if (target.read_verify_logits(q, trace_logits)) { | ||
| for (int row = 0; row < q; ++row) { |
There was a problem hiding this comment.
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>
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, dynamiclibroctx64.soloading for matching ROCTX ranges, stable[ds4-gpu-profile] clock=hip_eventrecords.deepseek4_graph.cpp: scopes aroundhc_pre,attention,moe_ffn,hc_post, andoutput_projectionfor 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
0are 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.gpu_msandcalls. 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).libroctx64.sois 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
03429.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_ffn53.590 ms over 43 calls (27.2%),output_projection2.560 ms (1.3%); multi-token HC pre/post report 0 GPU calls because they execute on the host.Limitations
gpu_ms=0 calls=0;DFLASH_DS4_TIMINGremains the host-wall view for those phases.deepseek4_step_layer_range,ds4_try_fused_decode_step, and the dspark verify path; open PRs touching the same functions (perf(ds4): compute greedy argmax on GPU (monolithic and layer split) #531, feat(ds4): opt-in GPU F16 HC fn mirrors for ROCmFPX #546, feat(ds4): Lucebox heterogeneous expert parallel + DSpark verify #505) may need a textual rebase depending on merge order, but there is no semantic dependency.Verification
/opt/models/DeepSeek-V4-Flash-IQ2XXS-w2Q2K-AProjQ8-SExpQ8-OutQ8-chat-v2-imatrix.gguf(SHA-256efc7ed607ff27076e3e501fc3fefefa33c0ed8cf1eff483a2b7fdc0c2e616668), draftds4-dspark-draft.gguf(SHA-25648883d35b8a67ecfd2858a90e12a47d04cb5ac581acef868ca0f58544816f746).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_serverandtest_deepseek4_unitbuild and pass. Unset /=0/=1arms 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 ofdflash_commonandtest_deepseek4_unitpassed.62880e2: incremental HIP rebuild passed,test_deepseek4_unitpassed, 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 --checkclean.