perf(vulkan): drain the batch only on a real dependency — decode 2.74 → 2.99 - #174
Merged
Conversation
…4 -> 2.99 Goal is llama.cpp Vulkan's 4.35 tok/s on Qwen3.6-27B decode. Both decode GEMMs are already near the bandwidth roof (vt_matmul_vec 90%, lm_head 74%), and GPU busy was only 56% of wall, so the whole gap is host-side. MEASURED, NOT GUESSED: 212 flushes per TOKEN, most carrying 1-2 dispatches. Attributing each flush to its trigger settled it in one run: copy 2081 flushes avg 3.2 dispatches reference-tier 112 flushes avg 10.0 ring-full 0 batch-cap 0 The ring depth and batch cap -- the two things previously tuned -- never fired once. Backend::Copy drained unconditionally, so every host memcpy paid a full submit-plus-blocking-fence round trip and command-buffer batching was almost entirely defeated. A drain is required only when the copy touches memory the OPEN batch bound: reading a buffer the batch writes would see bytes the GPU has not written, and writing one it reads would change operands mid-flight. If the batch never bound the buffer -- the common case, since activations flow forward into freshly allocated ones -- neither hazard exists. A pointer outside every Vulkan allocation is plain host memory and cannot alias a bound buffer, so it never drains; TryResolve is added for that question because Resolve treats a host pointer as an error. Flushes 212 -> 114 per token, decode 2.74 -> 2.99 tok/s. opt-125m STRICT 6/6 token-exact, 26/26 on GB10. WHAT REMAINS IS ARCHITECTURAL. copy-src and copy-dst still account for 98 of the 114 flushes, because the 27B forward moves activations through HOST vectors between ops and round-trips ~100 times per token. llama.cpp never does this: its whole graph runs and one readback happens at the end, so its mid-graph submits carry no fence at all. Closing that is a device-resident forward in qwen3_5.cpp, a model-level change, and it is the remaining 1.45x. The flush-trigger attribution ships behind VT_VULKAN_DISPATCH_STATS. It is the third diagnostic this session that answered in one run what reading the source had gotten wrong. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude-Code:claude-opus-5 [Claude Code]
Collaborator
Author
|
LANDED on Rebased over 21 commits of main and re-gated on a clean rebuild before merging, not on the pre-rebase build: Pre-existing red, not from this branch: Closing per POL-PR-DISPOSITION. |
mudler
added a commit
that referenced
this pull request
Aug 8, 2026
Landing a PR with `gh pr merge --merge` breaks two gates, and it has now happened three times in a row on main: c3db780 (#172, mine), 3e5072d (#174), and the trailer half on da0a9d8 (#157). check-role-discipline.py inspects every commit in the pushed range and wants each to name its row/<ROW-ID> branch or its PR as (#N). GitHub's squash-merge writes (#N) into the subject for free. A --merge landing leaves the content commit with the subject it had on the branch, which names neither, so POL-PR-REQUIRED reports "reached main without a reviewed row/* PR" about a commit that came from exactly such a PR. check-commit-trailers.py reads the same range, and GitHub's generated "Merge pull request #N from ..." message carries no FOLLOWING_AGENTS_PROTOCOL paragraph and no trailers, so POL-COMMIT-TRAILERS fails on the merge commit itself. Both gates are scoped over github.event.before..github.sha and each run's before is the previous run's sha, so no later run re-covers a range that already went red. That is the part worth writing down: the failure is not repairable after the push except by rewriting published history or waiving it, and it is invisible at PR time because the PR is checked against its own base. Documented in CONTRIBUTING.md rather than .agents/workflow.md because that file is 7 bytes under its 12288-byte procedure budget, and every paragraph of its non-generated prose is load-bearing; buying room by trimming it would cost more than this note is worth. CONTRIBUTING.md is also where a contributor with merge rights actually looks. This is documentation, not a guard. The permanent fix is a repository setting - allow squash only, disable merge commits - which needs admin and is left to the owner. No record row, no capability claim, no code. Every tree-scoped gate on main was already green before this change; main's red runs come entirely from the diff-scoped range gates described above. Gates: check-policy, check-protocol-consistency, check-agent-record, check-readme-structure, check-public-doc-tables, check-role-discipline, check-doc-checkpoint and check-commit-trailers over the range - all OK on a worktree pinned at 80b0880. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude-Code:claude-opus-5 [Claude Code]
mudler
added a commit
that referenced
this pull request
Aug 8, 2026
test_agent_role.py::test_landed_detached_commit_remains_strict_without_ pending_evidence failed on PR #176 and would fail on many PRs that have nothing to do with what it asserts. The test's subject is main()'s DECISION: a violation on a commit that has landed, with no --pending-pr-head evidence, is strict (return 1) rather than a REPORT (return 0). But it obtained that violation by relying on the REAL HEAD to be one, and that coupling is live under CI. On a pull_request event GitHub checks out the SYNTHETIC merge commit for refs/pull/N/merge. inspect() sees two parents, so arrives_via_row_pr() switches to the merged_messages branch and scans the PR's own commit bodies with PR_REFERENCE = \(#\d+\)|#\d+, which matches a bare #123 anywhere. Any PR whose commit message cites an issue or PR number therefore stops being a violation, main() returns 0, and the assertion fails with 0 != 1. #176's message quotes (#157) and (#174) while explaining these very gates, which is how it tripped. Fixed by feeding main() a fixed violation through inspect/enforced rather than depending on HEAD's message, plus a regression case pinning that a message mentioning (#157) and #174 cannot flip the decision again. Proven on a reconstructed synthetic merge, the exact shape CI checks out (git merge --no-ff of the PR head into the base, detached): RED old test on that checkout: AssertionError: 0 != 1, 41 tests, FAILED (failures=1) -- the same failure CI reported GREEN new test on the SAME checkout: 42 tests, OK Not changed: PR_REFERENCE matching a bare #123 anywhere in a body also means a direct push whose message merely mentions an issue satisfies POL-PR-REQUIRED. That is a real hole, but tightening a policy gate's strictness is a separate reviewed decision, not a CI repair. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude-Code:claude-opus-5 [Claude Code]
localai-bot
added a commit
that referenced
this pull request
Aug 8, 2026
Two fixes for a main branch that has been red for several landings. 1) CONTRIBUTING.md documents the landing rule. Landing a PR with `gh pr merge --merge` breaks two gates, and it happened three times in a row: c3db780 (#172, mine), 3e5072d (#174), and the trailer half on da0a9d8 (#157). check-role-discipline.py inspects every commit in the pushed range and wants each to name its row branch or its PR number, which squash-merge writes into the subject for free and --merge does not. check-commit-trailers.py reads the same range, and GitHub's generated merge-commit message carries no FOLLOWING_AGENTS_PROTOCOL paragraph and no trailers. Both gates are scoped over before..sha, and each run's before is the previous run's sha, so no later run re-covers a range that already went red: the failure is not repairable after the push, and it is invisible at PR time because a PR is checked against its own base. Documented in CONTRIBUTING.md rather than .agents/workflow.md because that file sits 7 bytes under its 12288-byte procedure budget. 2) tests/scripts/test_agent_role.py is unbroken under a pull_request checkout. test_landed_detached_commit_remains_strict_without_pending_ evidence asserts a decision in main() -- a violation on a landed commit with no pending evidence is strict rather than a REPORT -- but it obtained that violation by relying on the real HEAD to be one. Under a pull_request event GitHub checks out the synthetic refs/pull/N/merge commit; inspect() then sees two parents, so arrives_via_row_pr() scans the PR's own commit bodies with a pattern that matches a bare #123 anywhere. Any PR whose message cites an issue or PR number stopped being a violation, main() returned 0, and the test failed 0 != 1. It now feeds main() a fixed violation, with a regression case pinning that a message mentioning a PR number cannot flip the decision again. Proven on a reconstructed synthetic merge, the exact shape CI checks out: RED, the old test on that checkout gave AssertionError: 0 != 1 across 41 tests, the same failure CI reported; GREEN, the new test on the SAME checkout gave 42 tests OK. Confirmed in real CI on this PR: agent-record now passes. Not changed: the reference pattern matching a bare #123 anywhere also means a direct push whose message merely mentions an issue satisfies POL-PR-REQUIRED. That is a real hole, but tightening a policy gate is a separate reviewed decision, not a CI repair. The permanent fix for the merge-method half is a repository setting -- allow squash only, disable merge commits -- which needs admin. Gates: check-agent-record (ENGINE=144), check-role-discipline, check-policy, check-protocol-consistency, check-readme-structure, check-public-doc-tables, test_agent_role 42/42, test_agent_onboard, plus check-commit-trailers and check-doc-checkpoint over the range. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude-Code:claude-opus-5 [Claude Code]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Decode 2.74 → 2.99 tok/s on Qwen3.6-27B, toward llama.cpp Vulkan's 4.35.
The bottleneck, measured rather than guessed
Both decode GEMMs are already near the bandwidth roof (
vt_matmul_vec90%, lm_head 74%) and GPU busy was only 56% of wall — so the whole gap is host-side.Attributing every command-buffer flush to its trigger found it in one run: 212 flushes per token, most carrying 1-2 dispatches.
copyreference-tierThe ring depth and batch cap — the two knobs previously tuned, including the one that won 1.51× on the 0.6B — never fired once.
Backend::Copydrained unconditionally, so every host memcpy paid a submit-plus-blocking-fence round trip and batching was almost entirely defeated.The fix
A drain is required only when the copy touches memory the open batch bound: reading a buffer the batch writes would see bytes the GPU hasn't written; writing one it reads would change operands mid-flight. If the batch never bound the buffer — the common case, since activations flow forward into freshly allocated ones — neither hazard exists.
A pointer outside every Vulkan allocation is plain host memory and cannot alias a bound buffer, so it never drains.
TryResolveis added for that question, becauseResolvetreats a host pointer as an error.Flushes 212 → 114 per token; decode 2.74 → 2.99.
What remains, and it is architectural
copy-srcandcopy-dststill account for 98 of the 114 flushes: the 27B forward moves activations through host vectors between ops, round-tripping ~100 times per token. llama.cpp never does this — its whole graph runs and one readback happens at the end, so its mid-graph submits carry no fence at all (ggml-vulkan.cpp:15709,:16344). Closing that is a device-resident forward inqwen3_5.cpp, a model-level change, and it is the remaining 1.45×.Gates
test_vulkan_backend26/26 (1020 assertions),opt-125mSTRICT 6/6 token-exact, 0 declines, clean-Werror, full preflight green — all re-run on a clean rebuild after rebasing over 21 commits of main.The flush-trigger attribution ships behind
VT_VULKAN_DISPATCH_STATS. It is the third diagnostic this session that answered in one run what reading the source had gotten wrong.🤖 Generated with Claude Code