perf(vulkan): barrier only on a PROVEN dependency, and the llama.cpp gate is already MET - #198
Merged
Merged
Conversation
…gate is already MET The batched dispatch path recorded an unconditional COMPUTE->COMPUTE VkMemoryBarrier before every dispatch, about 900 per 27B decode token. llama.cpp instead syncs at 42 explicit ggml_vk_sync_buffers call sites with per-scratch-buffer need_sync flags (ggml-vulkan.cpp:3193, :2108, :8174, :8687-8748 @ 237ad9b96). VT_VULKAN_SMART_BARRIERS=1 generalizes that: a barrier is recorded only on a true read-after-write, write-after-write or write-after-read hazard against everything recorded since the previous barrier. The read and write sets are the hard part, and they are neither plumbed from the op layer nor guessed from binding index. Dispatch() gets one flat VkBuffer array with no roles attached, and a wrong guess costs a MISSING barrier, i.e. silently wrong numbers. The GLSL already states the roles and glslang enforces them: writing a `readonly buffer` is a compile error and the promise survives into the module as a NonWritable decoration. gen-vulkan-spirv.py now reflects each module's binding count and writable_mask out of the compiled SPIR-V, the same way it already reflects SpecIds, and GetPipeline hard-checks the declared binding count against what the host binds. The analysis defaults to emitting. Buffers are compared whole, never by byte range; an operand that is both read and written counts as a write; a binding whose read-only-ness is not positively proven counts as writable. The access sets are cleared ONLY when a barrier is recorded, never at a flush or a drain, which is what carries the invariant across the command-buffer boundary that pipelined submission creates, using the same submission-order scope the unconditional barrier relied on. vkCmdDispatch is the only device command in this backend that touches a storage buffer, so the analysis sees every device reader and writer. MEASURED ON GB10. Current main is 4.36 tok/s (7 clean legs, median TPOT 229.54 ms) against llama.cpp Vulkan's 4.35: the campaign gate is MET before this lever, which the 4.285 of record predated. The lever removes 178 of 900 barriers per decode token (19.8%) and moves the GPU-timestamp span 231.38 -> 230.29 ms/token; e2e over 12 order-alternated AB/BA pairs it wins 8, mean paired delta -0.94 ms, sign test p = 0.19, so the e2e instrument does NOT establish it. It ships DEFAULT-OFF for that reason and because this is the hazard class that produced a measured-faster arm computing garbage on GB10 two days ago. Correctness ran before any timing, in all 12 combinations of barrier arm x VT_VULKAN_INFLIGHT x gate: test_vulkan_backend 35/35, test_backend_cross_device 11/11, opt-125m 6/6 prompts token-exact (96/96 tokens) with 0 declines. A 256-token decode gives byte-identical token ids across all four arms. RED-BEFORE-GREEN by mutation: forcing the hazard predicate to report "independent" fails 330 VALUE assertions on GB10 (0 == -150, a dispatch reading memory before its producer wrote it); removing only the write-after-read half fails exactly the two new structural barrier counters and NO value assertion. A gate that checked only numbers would have passed the second one. 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 9, 2026
fix(vulkan): the 27B load held the model TWICE on a unified box (#203) Fixes #203. On a unified-memory box the Vulkan backend held **two full copies of the weights**. For Qwen3.6-27B that is an extra 50 GiB, and it hard-rebooted this GB10 twice in one session with `NVRM ... NV_ERR_NO_MEMORY @ mem_desc.c:1359`. ## Measured Qwen3.6-27B bf16 (50.89 GiB), GB10, one binary, `VT_ADOPT_DEVICE_BYTES` A/B: | arm | Vulkan live | buffers | VmRSS | MemAvailable floor | outcome | |---|---|---|---|---|---| | OFF (old behaviour) | 50.755 GiB | 863 of 894 | **100.759 GiB** | 13.85 | watchdog killed it, **still allocating** | | ON (now default) | 50.756 GiB | 894 | **53.413 GiB** | 47.33 | completed | The gap is a flat **50.003 GiB** across the last four high-water lines — one whole extra copy of the model. Qwen3-4B: VmHWM 16.392 → 9.607 GiB. The unfixed 27B never completed, so 100.759 GiB is a **lower bound**. ## Cause `ResidentWeight` uploaded each weight and kept `OwnedTensor.bytes` as well. Vulkan allocates every buffer `HOST_VISIBLE | HOST_COHERENT | DEVICE_LOCAL` and persistently mapped, so on unified memory both copies are the same RAM. `src/vllm/platforms/vulkan.cpp` had *reasoned* "there is exactly one copy of the bytes"; `dense_attn_block.h:190` made a second one. ## The investigation refuted my own first diagnosis I briefed this row with "the Vulkan loader lacks the CUDA path's shard release". That was **wrong**, and the row was told not to build on it. Windowed release is not the defect and it does fire: during load the RSS holds one copy, not the mmap as well. The other candidates were measured and cleared too: - **No allocator waste.** `requested == committed` exactly, every run — 50.756 GiB over 894 buffers. Nothing to win from suballocation, and `maxMemoryAllocationCount` is nowhere near 894. - **No transient held too long.** A staging or dequant scratch would show as a bump; the excess is a constant equal to the model. - **Page cache is not the trigger.** It tracks copied bytes 1:1 and is reclaimable. (Worth recording: `MADV_DONTNEED` on a private file mapping does not evict page cache — that needs `POSIX_FADV_DONTNEED`.) - **The 89.72 GiB Vulkan heap was never the binding constraint.** The machine was. A first watchdog keyed on `MemFree` killed a healthy load at 11.46 GiB free while `MemAvailable` was 60.65 — recorded as a negative result. ## Fix `AdoptDeviceBytesAsHost` re-points `bytes` **at** the device allocation via the existing `OwnedBytes` borrow, keyed alive by `d_dev`'s control block. An adoption, not a release: every `.bytes` reader sees the same bytes, so unlike `ReleaseHost` it needs no "device path committed" proof. Gated on a new `Backend::DeviceMemoryIsHostAddressable()`, default **false**, deliberately narrower than `UnifiedMemory()` — CUDA on GB10 is unified yet `cudaMalloc` is not host-dereferenceable. Discrete and non-Vulkan paths are byte-identical. ## Gates, operator-verified on GB10 with the real driver All four arms (`VT_ADOPT_DEVICE_BYTES` x `VT_VULKAN_INFLIGHT`), re-run by me rather than taken on report: - `test_vulkan_backend` **2650/2650 assertions** in all four - `test_opt_paged_engine` **6/6 prompts token-exact (96/96)**, 0 declines, in all four, on **device type 3** - `test_backend_cross_device` **11/11 (132)** On llvmpipe from a clean build: `test_vulkan_backend` 35/35 (2107), `test_backend_cross_device` 11/11, `test_opt_paged_engine` 6/6 token-exact on device type 3, and the three new mechanism tests 7/7 (45 assertions). `gen-vulkan-spirv.py --check` clean. The row's three mechanism tests each go **red under mutation** — no-op body (3 assertions), dropped guard (3), dropped keep-alive (2, a real use-after-free reading 128 where 167 was written). ## A trap this uncovered, worth knowing repo-wide **`VLLM_CPP_DEVICE` is read nowhere in the tree.** The engine selects via `CurrentPlatform()`. The campaign's recorded gate command `VLLM_CPP_DEVICE=vulkan test_opt_paged_engine` has been selecting Vulkan only because those builds had no CUDA compiler; with CUDA on PATH the identical command reports `device type 1` and passes 6/6 on **CUDA**. Every Vulkan gate in this PR was therefore checked for `device type 3` explicitly, and so were the barrier gates in #198 (confirmed type 3 in all four arms). The env var gives false confidence and the docs should stop implying it selects anything. ## Left open, named Peak is now the **load** phase: the host build reaches ~51 GiB before the first upload. Copying from the mmap straight into the device buffer would cut that too. ## Not tested No CUDA or Metal run — argued byte-identical from the default-false gate, not measured. Only Qwen3-4B and Qwen3.6-27B. `test_qwen36_weights` has one pre-existing failure on a CUDA-less build (`in_proj_qkv_fp8` behind `#ifdef VT_CUTLASS_FP8`) that is unrelated and touches no Backend. 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
2 participants
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.
Merge pull request #192 from mudler/row/BACKEND-VULKAN-BARRIERS
perf(vulkan): barrier only on a PROVEN dependency, and the llama.cpp gate is already MET
THE HEADLINE IS NOT THE LEVER. No absolute number existed for main after #191
landed pipelined submission, so this row measured one first: 7 clean legs, page
cache dropped per leg, GPU serialized, 27B bf16 32/32 c1, TPOT 228.66 to 230.25 ms
= MEDIAN 4.36 tok/s against llama.cpp Vulkan's 4.35. One further leg was discarded
for a NAMED cause -- an overlapping compile job the row had started itself -- not
the unexplained 1-in-16 collapse. The 4.285 of record predates #191. Clean-leg
spread is 0.69%, which IS the noise floor, so this is a NARROW pass and is written
down as one.
OPERATOR VERIFICATION, at its real strength. I re-measured independently on a
different main SHA with its own fresh configure and read 4.58 tok/s -- but n=1, on
a freshly rebooted box with clean memory, so I treat it as OPTIMISTIC and keep the
7-leg median as the binding figure. It corroborates the direction, not the value.
I abandoned further legs rather than reboot the box a third time (below).
THE BRIEFED PREMISE WAS WRONG, AGAIN MINE. I sized this lever at "~2.3 ms of GPU
time outside any timestamped dispatch". It measures 0.56 ms/token and does NOT
move when 178 barriers/token are removed. TOP_OF_PIPE is stamped when a command
reaches top of pipe, which a preceding barrier does not hold back, so the drain is
billed INSIDE the following dispatch's interval. Span minus sum understates
barrier cost; the per-command-buffer span is the honest probe.
READ/WRITE SETS: NEITHER PLUMBED NOR GUESSED. I offered two options and the row
took a third and better one -- the GLSL already states it and glslang ENFORCES it,
since writing a
readonly bufferis a compile error and the promise survives as aNonWritable decoration in the SPIR-V. gen-vulkan-spirv.py now reflects each
module's binding_count and writable_mask out of the compiled blob (the same
technique as the existing spec_ids reflection) and GetPipeline hard-checks the
declared binding count against what the host binds. Zero op-layer edits, and the
source of truth is machine-checked. Also verified: vkCmdDispatch is the ONLY
device command in this backend that touches a storage buffer, and AllocBuffer
gives each VkBuffer its own dedicated VkDeviceMemory, so distinct buffers cannot
alias. Access sets clear ONLY when a barrier is recorded, never at a flush, which
is what carries the invariant across the pipelined command-buffer boundary.
MEASURED: barriers 900.0 -> 722.0 per token (-19.8%); GPU span 231.38 -> 230.29 ms
(-1.09); span-minus-sum 0.556 -> 0.563 (unmoved, as above). e2e 12 order-alternated
pairs, 8 to smart, mean paired delta -0.94 ms, sign test p = 0.19 -- NOT
ESTABLISHED against a box that drifts 8.6 ms across four pairs of its own base arm.
DEFAULT OFF. Gate-clean everywhere tried and the mechanism is directly measured,
but the e2e effect is not resolved past this box's drift, the goal is already met
without it, and this is the exact hazard class that shipped garbage two days ago.
One env var away with all evidence attached.
THE MUTATION RESULTS ARE THE MOST VALUABLE PART.
failures (0 == -150: a dispatch reading memory before its producer wrote it).
The unconditional barrier is load-bearing, proven on the real driver.
BOTH the new structural counters, and EVERY value assertion passed. A partial
implementation that handles RAW and WAW but forgets WAR computes correct
numbers on this hardware today. That is the whole argument for asserting on
counters rather than on tolerances, demonstrated instead of asserted.
OPERATOR-VERIFIED GATES, on GB10, all four combinations of the barrier arm and the
pipelining depth, because llvmpipe is largely serial and hides exactly this class
of bug -- this campaign already has a change that passed 33/33 on llvmpipe while
computing garbage on the real driver:
smart=0 inflight=1 test_vulkan_backend 2647/2647 opt-125m 6/6 token-exact
smart=0 inflight=2 test_vulkan_backend 2650/2650 opt-125m 6/6 token-exact
smart=1 inflight=1 test_vulkan_backend 2647/2647 opt-125m 6/6 token-exact
smart=1 inflight=2 test_vulkan_backend 2650/2650 opt-125m 6/6 token-exact
plus test_backend_cross_device 132/132 and, on llvmpipe from a clean build,
test_vulkan_backend 35/35 (2107) and opt-125m 6/6 with 0 declines.
gen-vulkan-spirv.py --check reproduces the committed SPIR-V byte-for-byte. The row
additionally reports a 256-token decode byte-identical across all four arms.
INCIDENTS, DISCLOSED. The row hit a stale-binary FALSE GREEN first:
cp -aof abuild directory silently rebuilds the ORIGINAL sources because CMake caches
absolute paths, and produced a byte-identical binary. Redone in place with source
and binary md5s verified. Separately the box OOM-rebooted twice today
(NVRM NV_ERR_NO_MEMORY, mem_desc.c:1359) -- once here, once during my own
verification, on the FIRST leg with nothing else running. Root cause is now
understood and is NOT this change: the 27B holds ~50 GB of safetensors in page
cache AND ~50 GB of Vulkan buffers, both host RAM on a 119 GB unified box, and
after a leg exits that memory is not promptly reclaimed, so leg N+1 allocates
while leg N is still outstanding. The CUDA NVFP4 path already solves this by
releasing the mmap'd shards after upload (114 -> 67 GiB RSS); the Vulkan loader
does not. That is a real robustness bug for any 27B user on a Spark and is left
NAMED, not fixed here.
Also updates the headline benchmark source and the two README claims it licenses,
both of which still said 98.5% and understated the current state.
FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: Claude-Code:claude-opus-5 [Claude Code]