perf(dflash): split-chain fast rollback with F32 SSM checkpoints (+29% decode)#506
Conversation
There was a problem hiding this comment.
4 issues found across 13 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/qwen35/layer_split_forward.cpp">
<violation number="1" location="server/src/qwen35/layer_split_forward.cpp:631">
P1: Feature-capture IPC requests now fail whenever the shard daemon runs without DFLASH: `captures_out` enables rollback capture even though its prefill-only cache has no rollback storage. Keep feature-slice capture separate from `capture_ssm_intermediates`; only an explicit rollback request should enable it.</violation>
</file>
<file name="server/CMakeLists.txt">
<violation number="1" location="server/CMakeLists.txt:969">
P2: The new `test_qwen35_split_tree_guard` is registered with CTest but never added to `_check_deps`, so `make check` will not build or run iun it. The "CPU-only root-inclusive pure-chain guard regression test" described in the PR is effectively excluded from the standard test workflow.</violation>
</file>
<file name="server/src/qwen35/qwen35_layer_split_dflash_target.cpp">
<violation number="1" location="server/src/qwen35/qwen35_layer_split_dflash_target.cpp:710">
P2: The feature ring validation loop after `std::copy` can never detect an error. `std::copy(source_stage.begin(), source_stage.end(), dest_stage.begin())` is a byte-exact element-wise copy — when both vectors have the same size it always produces identical content. The subsequent 30-line element-by-element equality check (`if (a[j] != b[j])`) is therefore dead code: it will always pass. This gives a misleading impression of safety; remove the validation loop or replace it with a check that actually compares against expected data (e.g., what the destination _should_ contain after reordering, rather than what was just copied from the source).</violation>
</file>
<file name="server/src/qwen35/layer_split_types.h">
<violation number="1" location="server/src/qwen35/layer_split_types.h:43">
P2: The `non_owner_write_count` field in `Qwen35SplitCaptureStats` is declared, reset, logged, and checked in a gate condition (`non_owner_write_count == 0`), but it is never incremented. This means the gate always passes (every capture is logged as zero non-owner writes), masking the intended safety check. The field's name suggests it was meant to catch a non-owning shard writing into another shard's SSM rollback storage — a correctness concern for layer-split mode. Either add the increment at the appropriate write site, or remove the field (and the gate/log that depend on it) to avoid dead-code confusion.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| shards, acts, base_pos, n_tokens_total, ubatch, kq_stride_pad, | ||
| fa_window, captures_out, nullptr, nullptr, kvflash, | ||
| kvflash_preallocated)) { | ||
| /*capture_ssm_intermediates=*/captures_out != nullptr, |
There was a problem hiding this comment.
P1: Feature-capture IPC requests now fail whenever the shard daemon runs without DFLASH: captures_out enables rollback capture even though its prefill-only cache has no rollback storage. Keep feature-slice capture separate from capture_ssm_intermediates; only an explicit rollback request should enable it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/qwen35/layer_split_forward.cpp, line 631:
<comment>Feature-capture IPC requests now fail whenever the shard daemon runs without DFLASH: `captures_out` enables rollback capture even though its prefill-only cache has no rollback storage. Keep feature-slice capture separate from `capture_ssm_intermediates`; only an explicit rollback request should enable it.</comment>
<file context>
@@ -516,11 +623,13 @@ bool run_qwen35_layer_split_forward_from_activation(
shards, acts, base_pos, n_tokens_total, ubatch, kq_stride_pad,
fa_window, captures_out, nullptr, nullptr, kvflash,
- kvflash_preallocated)) {
+ /*capture_ssm_intermediates=*/captures_out != nullptr,
+ /*capture_stats=*/nullptr, kvflash_preallocated, tree_inputs)) {
return false;
</file context>
| test/test_qwen35_split_tree_guard.cpp) | ||
| target_include_directories(test_qwen35_split_tree_guard PRIVATE | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/src/qwen35) | ||
| add_test(NAME qwen35_split_tree_guard COMMAND test_qwen35_split_tree_guard) |
There was a problem hiding this comment.
P2: The new test_qwen35_split_tree_guard is registered with CTest but never added to _check_deps, so make check will not build or run iun it. The "CPU-only root-inclusive pure-chain guard regression test" described in the PR is effectively excluded from the standard test workflow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/CMakeLists.txt, line 969:
<comment>The new `test_qwen35_split_tree_guard` is registered with CTest but never added to `_check_deps`, so `make check` will not build or run iun it. The "CPU-only root-inclusive pure-chain guard regression test" described in the PR is effectively excluded from the standard test workflow.</comment>
<file context>
@@ -961,6 +961,13 @@ if(DFLASH27B_TESTS)
+ test/test_qwen35_split_tree_guard.cpp)
+ target_include_directories(test_qwen35_split_tree_guard PRIVATE
+ ${CMAKE_CURRENT_SOURCE_DIR}/src/qwen35)
+ add_test(NAME qwen35_split_tree_guard COMMAND test_qwen35_split_tree_guard)
+
if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/test/test_server_unit.cpp")
</file context>
| std::copy(one_row.begin(), one_row.end(), | ||
| source_stage.begin() + (size_t)d * (size_t)fc_in); | ||
| } | ||
| std::copy(source_stage.begin(), source_stage.end(), dest_stage.begin()); |
There was a problem hiding this comment.
P2: The feature ring validation loop after std::copy can never detect an error. std::copy(source_stage.begin(), source_stage.end(), dest_stage.begin()) is a byte-exact element-wise copy — when both vectors have the same size it always produces identical content. The subsequent 30-line element-by-element equality check (if (a[j] != b[j])) is therefore dead code: it will always pass. This gives a misleading impression of safety; remove the validation loop or replace it with a check that actually compares against expected data (e.g., what the destination should contain after reordering, rather than what was just copied from the source).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/qwen35/qwen35_layer_split_dflash_target.cpp, line 710:
<comment>The feature ring validation loop after `std::copy` can never detect an error. `std::copy(source_stage.begin(), source_stage.end(), dest_stage.begin())` is a byte-exact element-wise copy — when both vectors have the same size it always produces identical content. The subsequent 30-line element-by-element equality check (`if (a[j] != b[j])`) is therefore dead code: it will always pass. This gives a misleading impression of safety; remove the validation loop or replace it with a check that actually compares against expected data (e.g., what the destination _should_ contain after reordering, rather than what was just copied from the source).</comment>
<file context>
@@ -72,6 +169,599 @@ bool Qwen35LayerSplitDFlashTarget::restore_kv() {
+ std::copy(one_row.begin(), one_row.end(),
+ source_stage.begin() + (size_t)d * (size_t)fc_in);
+ }
+ std::copy(source_stage.begin(), source_stage.end(), dest_stage.begin());
+ for (int d = 0; d < stage_rows; ++d) {
+ const float * a = source_stage.data() + (size_t)d * (size_t)fc_in;
</file context>
| uint64_t requested = 0; | ||
| uint64_t enabled = 0; | ||
| uint64_t missing_owner_count = 0; | ||
| uint64_t non_owner_write_count = 0; |
There was a problem hiding this comment.
P2: The non_owner_write_count field in Qwen35SplitCaptureStats is declared, reset, logged, and checked in a gate condition (non_owner_write_count == 0), but it is never incremented. This means the gate always passes (every capture is logged as zero non-owner writes), masking the intended safety check. The field's name suggests it was meant to catch a non-owning shard writing into another shard's SSM rollback storage — a correctness concern for layer-split mode. Either add the increment at the appropriate write site, or remove the field (and the gate/log that depend on it) to avoid dead-code confusion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/qwen35/layer_split_types.h, line 43:
<comment>The `non_owner_write_count` field in `Qwen35SplitCaptureStats` is declared, reset, logged, and checked in a gate condition (`non_owner_write_count == 0`), but it is never incremented. This means the gate always passes (every capture is logged as zero non-owner writes), masking the intended safety check. The field's name suggests it was meant to catch a non-owning shard writing into another shard's SSM rollback storage — a correctness concern for layer-split mode. Either add the increment at the appropriate write site, or remove the field (and the gate/log that depend on it) to avoid dead-code confusion.</comment>
<file context>
@@ -27,4 +29,33 @@ struct Qwen35LayerSplitShard : LayerSplitShardMeta {
+ uint64_t requested = 0;
+ uint64_t enabled = 0;
+ uint64_t missing_owner_count = 0;
+ uint64_t non_owner_write_count = 0;
+ std::vector<uint64_t> layers_owned_per_shard;
+ std::vector<uint64_t> slots_written_per_shard;
</file context>
Summary
Benchmark
Controlled dual-GPU A/B/A benchmark on two RTX 3090 24 GB cards using the same pinned models, prompt, generation length, power/thermal conditions, and layer split (
--target-gpus 0,1 --target-layer-split 12,11). Each block used one discarded warmup and five measured runs.All 15 measured runs produced byte-identical 256-token output:
1653a412811d3b87e26aab900e801b411fd68f9214b512ae984db6404b82204fA1/A2 baseline drift was 0.48%. Maximum observed GPU temperature was 64 C.
Memory trade-off
The F32 SSM checkpoint storage adds exactly 1,774,190,592 bytes (~1.65 GiB) of persistent GPU memory in the tested split configuration. Both GPUs are required for the tested 27B configuration; a single RTX 3090 is not sufficient.
Safety boundary
supports_tree_verify()remains unconditionally false in production.DFLASH_SPLIT_FAST_ROLLBACKand requires successful capture validation.Verification
mainat5e302cbb483819cd21e72f5dd8becaa609eca8cfsrc[7]Scope
Measured on the exact dual-RTX-3090 configuration above with a 1,024-token prompt and 256 generated tokens. No performance claim is made for single-GPU execution, sibling-tree verification, other models, or other hardware.