perf(dspark): keep the Markov chain graph instead of rebuilding it per call

[maikzz32]

dspark_markov_correct_greedy_chain_fused builds its graph and frees it again on every call. The graph is a pure function of the candidate count, whether the confidence head is wanted, and the weights it closes over — all fixed for the life of a loaded model. The arena and the allocator beside it were already cached (static thread_local); only the graph was not.

This keeps the last one.

Correctness

The key compares the weights by identity (markov_w1, markov_w2, confidence_w, lm_head, backend), so a second model in the same process rebuilds rather than silently reusing the first one's graph. Both failure paths — allocator and compute — invalidate, so a retry rebuilds. DFLASH_DSPARK_NO_CHAIN_GRAPH_CACHE=1 restores the previous behaviour for isolating a suspected stale graph.

The context is freed before build_markov_chain_graph may resize the arena it builds into, which is why invalidate() drops the graph but keeps the arena and the allocator.

Measurement

Radeon 8060S (gfx1151), ROCm 10, DeepSeek V4 Flash, DSpark q=4, 128 tokens at temperature 0, median of 5. Output byte-identical either way.

	head phase
off	4.5 ms/step
on	4.3 ms/step

Modest, and I want to be straight about why I expected more. A candidate-count sweep (head = 2.2 ms at 1 candidate, 4.4 at 3) fits to 1.1 ms fixed plus 1.1 ms per candidate, which looked like graph construction. It is not: most of the constant part is the lm_head matmul, which reads all 129280 × 4096 of its weights whatever the candidate count. This commit removes the construction, which is the part that was genuinely wasted.

Verification

Built against main with -DDFLASH27B_GPU_BACKEND=hip -DDFLASH27B_HIP_ARCHITECTURES=gfx1151 -DDFLASH27B_ROCMFP2_AFFINE=ON -DGGML_HIP_GRAPHS=ON on a gfx1151 host: 379/379, no new warnings. dspark_markov_project_topk shares the builder and is untouched — it keeps its own arena and allocator.

🤖 Generated with Claude Code

[cubic-dev-ai]

1 issue found across 1 file

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/common/dspark_head.cpp">

<violation number="1" location="server/src/common/dspark_head.cpp:212">
P2: The cache key compares only pointer identity (markov_w1/w2, confidence_w, lm_head) and omits the shape parameters the graph actually closes over: dw.n_embd (hdim), dw.dspark.markov_rank, dw.dspark.confidence_dim, dw.dspark.confidence_b, and vocab = lm_head->ne[1]. If a drafter is freed and reloaded (the codebase supports this via free_drafter()/unpark("draft") in qwen35_backend.cpp and deepseek4_backend.cpp) and the allocator reuses the same weight-tensor addresses for a drafter with different dimensions, matches() returns true and the stale graph (which also references the freed weight tensor objects) is silently reused, corrupting inp_hidden/tensor-set sizes. Identity comparison alone does not guarantee the "second model rebuilds" property claimed in the PR. Add the shape fields to the key (or compare them in matches()) so a reload that changes any of them forces a rebuild.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The cache key compares only pointer identity (markov_w1/w2, confidence_w, lm_head) and omits the shape parameters the graph actually closes over: dw.n_embd (hdim), dw.dspark.markov_rank, dw.dspark.confidence_dim, dw.dspark.confidence_b, and vocab = lm_head->ne[1]. If a drafter is freed and reloaded (the codebase supports this via free_drafter()/unpark("draft") in qwen35_backend.cpp and deepseek4_backend.cpp) and the allocator reuses the same weight-tensor addresses for a drafter with different dimensions, matches() returns true and the stale graph (which also references the freed weight tensor objects) is silently reused, corrupting inp_hidden/tensor-set sizes. Identity comparison alone does not guarantee the "second model rebuilds" property claimed in the PR. Add the shape fields to the key (or compare them in matches()) so a reload that changes any of them forces a rebuild.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/src/common/dspark_head.cpp, line 212:

<comment>The cache key compares only pointer identity (markov_w1/w2, confidence_w, lm_head) and omits the shape parameters the graph actually closes over: dw.n_embd (hdim), dw.dspark.markov_rank, dw.dspark.confidence_dim, dw.dspark.confidence_b, and vocab = lm_head->ne[1]. If a drafter is freed and reloaded (the codebase supports this via free_drafter()/unpark("draft") in qwen35_backend.cpp and deepseek4_backend.cpp) and the allocator reuses the same weight-tensor addresses for a drafter with different dimensions, matches() returns true and the stale graph (which also references the freed weight tensor objects) is silently reused, corrupting inp_hidden/tensor-set sizes. Identity comparison alone does not guarantee the "second model rebuilds" property claimed in the PR. Add the shape fields to the key (or compare them in matches()) so a reload that changes any of them forces a rebuild.</comment>

<file context>
@@ -186,6 +186,56 @@ struct MarkovChainGraph {
+
+    bool matches(const DraftWeights & dw, ggml_tensor * head, ggml_backend_t be,
+                 int cand, bool conf) const {
+        return built && lm_head == head && backend == be && n_cand == cand &&
+               want_confidence == conf &&
+               markov_w1 == dw.dspark.markov_w1 &&
</file context>

[Graffioh]

maybe you can also double-check and in case address:

---

The cached graph contains pointers to the model’s weights. Those weights must remain alive whenever the graph is used.

Currently, the cache belongs to the thread, which can outlive the loaded model:

Load model A → cache graph pointing to A’s weights
Unload A     → free A’s weights, but keep cached graph
Load model B → cached graph still refers to A’s old memory

The PR checks pointer addresses to detect a different model, but the memory allocator can reuse the same addresses, making that check unreliable.

The lifetime fix means making the model own the cache: destroy the cached graph before freeing that model’s weights and backend. A newly loaded model then starts with a fresh cache.

It still reuses the graph throughout generation; it just stops keeping it after the resources it depends on are gone.

---

i think it would also simplify a bit the cache handling if that's actually the case