Skip to content

llama : synchronize before writing the inputs of a reused graph - #217

Open
gaetan-puleo wants to merge 1 commit into
masterfrom
fix/reused-graph-input-sync
Open

gaetan-puleo wants to merge 1 commit into
masterfrom
fix/reused-graph-input-sync

Conversation

@gaetan-puleo

Copy link
Copy Markdown
Collaborator

Problem

When llama_context::process_ubatch reuses the previous graph, it calls res->set_inputs() without synchronizing the scheduler. A rebuilt graph synchronizes first (inside ggml_backend_sched_alloc_graph / reset).

On an integrated GPU (gfx1151, unified memory), the device reads the host input buffers in place: they are not copied to device memory. So the host can overwrite inp_pos, inp_tokens, masks, etc. while the previous ubatch's kernels are still reading them. This is a race. It shows up only when graph reuse kicks in (consecutive ubatches with identical topology, e.g. decode).

Fix

Call ggml_backend_sched_synchronize() in the reuse branch before the inputs are written, as the rebuild path already does.

Testing

Strix Halo (gfx1151), Qwen3.8 Flash Next Q4_K_XL, llama-bench -n 128:

tg128 (t/s)
master 25.58 ± 1.06
this PR 26.27 ± 0.21

The decode-speed difference is within noise.

On an integrated GPU the device reads host input buffers in place, so
writing them while the previous ubatch is still running is a race. The
rebuild path already synchronizes; do the same when the graph is reused.
@Nathanw1014

Copy link
Copy Markdown

This is already on master: c5863db (merged with #213) synchronizes just before res->set_inputs(), which covers the reused graph, the rebuilt graph and the K-shift. With this PR on top the reused path would synchronize twice (the second is a no-op). Suggest closing as superseded.

This branch has not been deployed

No deployments
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.

2 participants