From 3e5072d4d87bd1c858913b173194450a29b4319c Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 8 Aug 2026 22:04:06 +0000 Subject: [PATCH] perf(vulkan): drain the batch only on a real dependency -- decode 2.74 -> 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] --- .agents/benchmark-record.md | 50 ++++++++++++++++++++++++++++++ docs/BENCHMARKS.md | 2 +- docs/STATUS.md | 2 +- src/vt/vulkan/vulkan_backend.cpp | 37 ++++++++++++++++++++--- src/vt/vulkan/vulkan_buffers.h | 4 +++ src/vt/vulkan/vulkan_context.cpp | 52 +++++++++++++++++++++++++++++--- src/vt/vulkan/vulkan_context.h | 13 ++++++-- src/vt/vulkan/vulkan_ops.cpp | 23 ++++++++++++++ 8 files changed, 171 insertions(+), 12 deletions(-) diff --git a/.agents/benchmark-record.md b/.agents/benchmark-record.md index fc8a8896d..b9bad610d 100644 --- a/.agents/benchmark-record.md +++ b/.agents/benchmark-record.md @@ -16054,3 +16054,53 @@ Rollback is 1728 calls / 718.704016 ms / 415.917 us; exact is 1728 / 27-commit main advance; against the sealed vLLM conv trace the residual is **1.60881x**. Trace SHA-256: rollback `6a5dde18e...f97c47`, exact `f47fb9cc...7aecf9`; both token files `83fcdc45...453545`. +### Decode: the batch was being drained by every host memcpy (2026-08-08, GB10) + +Goal is llama.cpp Vulkan's **4.35 tok/s** on Qwen3.6-27B decode. We were at 2.74, +with **GPU busy only 56% of wall** and both decode GEMMs already near the +bandwidth roof (`vt_matmul_vec` 90%, lm_head 74%) — so the entire gap is host. + +**MEASURED, not guessed: 212 flushes per TOKEN**, most carrying 1-2 dispatches. +Command-buffer batching was almost entirely defeated. Attributing each flush to +its trigger settled why in one run: + +| trigger | flushes | avg dispatches | +|---|---:|---:| +| `copy` | **2081** | 3.2 | +| `reference-tier` | 112 | 10.0 | +| ring-full / batch-cap | **0** | — | + +The ring depth and batch cap — the two things previously tuned — **never fired +once**. `Backend::Copy` drained the batch unconditionally, so every host memcpy +paid a full submit-plus-blocking-fence round trip. + +**A drain is required only on a real dependency:** when the copy touches memory the +OPEN batch bound. Reading a buffer the batch writes would see bytes the GPU has not +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 can never alias a bound buffer, so it never drains. + +**Result: flushes 212 -> 114 per token, decode 2.74 -> 2.99 tok/s (1.09x)**, with +opt-125m STRICT 6/6 token-exact and 26/26 on GB10. + +**What remains, and it is architectural.** `copy-src` (3201) and `copy-dst` (3072) +still account for 98 of the 114 flushes: the 27B forward moves activations through +HOST vectors between ops, so it round-trips host<->device ~100 times per token. +llama.cpp never does this — its whole graph runs and then one readback happens, so +its mid-graph submits carry no fence at all and a single wait lands at step end +(`ggml-vulkan.cpp:15709`, `:16344`). Closing that is a model-level change to +`qwen3_5.cpp` (a device-resident forward), not a backend one, and it is the +remaining 1.45x. + +`reference-tier` is a further 16 flushes/token from `kCausalConv1dFwd` and +`kAttnQkNormRopeGate` still on the host — worth ~14% of flushes. + +**Also ruled out by the sweep of llama.cpp's backend:** pre-recorded/replayed +command buffers do not exist upstream (`graph_plan_* = NULL`, `:17708-17711`; +`eOneTimeSubmit`, `:8086`), so there is no CUDA-graph analogue to port. They submit +MORE often than us, not fewer — every ~100 nodes, deliberately, to overlap host +recording with GPU execution — and their descriptor sets are fungible across +pipelines (one global layout, bump-allocated, `:6996-7009`, `:8137`), so ring +exhaustion cannot force a flush at all. Our submit-then-block idiom is, in their +code, a debug-only path behind `GGML_VK_SERIALIZE_SUBMISSIONS` (`:17016-17035`). diff --git a/docs/BENCHMARKS.md b/docs/BENCHMARKS.md index 09d11314c..3b0a7b4cb 100644 --- a/docs/BENCHMARKS.md +++ b/docs/BENCHMARKS.md @@ -361,7 +361,7 @@ built on it rather than keeping the flattering one. | Memory footprint vs declared workload (`ROAD-V1-MEM`, #83) | **Never measured, and not measurable today**: there is no auto-sizing to compare against, because the KV pool is a hand-typed `--num-blocks`, so "what the run actually needed" has no number | Once M1's `MemoryBudget` lands: predicted-vs-actual bytes per allocation class, then peak footprint ours-auto vs vLLM at its 0.9 default on the same model and config | | Startup latency (cold to first `/health`) | **36.51 s vs vLLM 0.25.0's 221.51 s = 6.07x** (medians of 3, 27B-NVFP4, GB10). PROVISIONAL: 3 of 6 legs contended, repeat killed by a host reboot. [Detail](../.agents/benchmark-record.md) | Uncontended 3-rep re-run on a quiet box | | Speculation depth (`ROAD-V1-D3-SPEC-K`, #81) | **Never measured, MTP is k=1** (our port covers vLLM's k=1 branch only), so no acceptance-vs-depth curve exists | k=2..4 three-way greedy gate, then the c1/c>1 A/B + the per-workload (prose vs code) acceptance-vs-depth curve any dynamic or adaptive depth policy needs | -| Vulkan vs llama.cpp Vulkan (`BENCH-VK-LLAMA`) | 24 NATIVE (+8 GDN, BOTH recurrences); 63 host-tier. **27B prefill 21.5x on GB10** (ragged-M reached coopmat). opt-125m e2e token-exact. [Detail](../.agents/specs/vulkan-full-support.md) | `VK-C` coopmat A/B on Thor (`VT_VULKAN_COOPMAT=0` A/Bs it): **11.1x-32.9x** vs our UNTILED scalar kernel, not vs a competent GEMM. `VK-E`: llama.cpp `-DGGML_VULKAN=ON` at `237ad9b96` on dgx, same GGUF, three columns | +| Vulkan vs llama.cpp Vulkan (`BENCH-VK-LLAMA`) | 24 NATIVE (+8 GDN, BOTH recurrences); 63 host-tier. **27B prefill 21.5x**, decode 2.74->2.99 (drain only on real dependency; target 4.35). opt-125m e2e token-exact. [Detail](../.agents/specs/vulkan-full-support.md) | `VK-C` coopmat A/B on Thor (`VT_VULKAN_COOPMAT=0` A/Bs it): **11.1x-32.9x** vs our UNTILED scalar kernel, not vs a competent GEMM. `VK-E`: llama.cpp `-DGGML_VULKAN=ON` at `237ad9b96` on dgx, same GGUF, three columns | | ROCm (`BACKEND-GATE-ROCM-VLLM` / `-SGLANG`) | **NOT APPLICABLE: no number measured, claimed or owed.** W0 ctest-green on 4 gfx archs (#41); gfx1201 hipBLAS + Gemma-4 MoE (#140, contributor) ran M0/M1 on 2× R9700, our side CPU-link-verified only. No AMD HW here | The approach-(b) fix (PENDING community) unblocks the first APU model run (M2); the gate becomes a same-box vLLM-ROCm oracle once a model runs ([#41](https://github.com/mudler/vllm.cpp/issues/41)); floor: vLLM | | SGLang floor arms | Never ran | Both arms of the SGLang comparison | | Embeddings on the ONE surface (ROW 6, `LlamaModel` + `vllm_embed` + `/v1/embeddings`) | **NO number measured, claimed or owed.** Correctness-gated only, CPU: the 2026-08-08 fold (engine path == direct registry path, f64 LAST+normalize reference on the committed fixture) is plumbing, no speed claim | A REAL embedding checkpoint (e5-mistral class) + a same-box `vllm.LLM(task="embed")` oracle; only then does an embed-throughput bar exist | diff --git a/docs/STATUS.md b/docs/STATUS.md index f2cbe0dc4..cdd3e32f9 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -421,7 +421,7 @@ Parakeet ASR (2026-08-07): *CPU-correct, ON THE ONE SURFACE (ROW 1)*. Ids exact LoRA (W1 CPU runtime brick landed; not yet usable end-to-end), multi-GPU, Vulkan (opt-125m exact, GEMV 1.8x; 24 native, +8 GDN incl. BOTH -recurrences; 27B prefill 21.5x on GB10; qwen3_5 #125 VERIFIED; CUDA build repaired +recurrences; 27B prefill 21.5x, decode 2.99 vs llama.cpp 4.35; #125 VERIFIED; CUDA build fixed [campaign](../.agents/specs/vulkan-full-support.md)), ROCm (W0 community-green on 4 gfx archs (#41); the ratified (b) APU unified-memory fix is in — **blind-written, unverified** — M2 unblocks on verification; gfx1201 hipBLAS + diff --git a/src/vt/vulkan/vulkan_backend.cpp b/src/vt/vulkan/vulkan_backend.cpp index e5361b87d..c08464376 100644 --- a/src/vt/vulkan/vulkan_backend.cpp +++ b/src/vt/vulkan/vulkan_backend.cpp @@ -82,17 +82,32 @@ class VulkanBackend final : public Backend { // error and no crash. The flush is a no-op when nothing is pending, and when // batching is off there is never anything pending. void Memset(Queue&, void* p, int value, size_t bytes) override { - VulkanContext::Get().FlushBatch(); + VulkanContext::Get().FlushIfBatchTouches(TryResolve(p).buffer, "memset"); std::memset(p, value, bytes); } + // DRAIN ONLY ON A REAL DEPENDENCY. MEASURED: this unconditional flush was 2,081 + // of 2,193 flushes in a 27B decode -- 95% -- averaging 3.2 dispatches each, so + // command-buffer batching was almost entirely defeated and every host memcpy + // paid a full submit-plus-blocking-fence round trip. Ring exhaustion and the + // batch cap never fired once. + // + // A drain is required exactly when the copy touches memory the OPEN batch is + // using: reading a buffer the batch writes would see bytes the GPU has not + // written, and writing one the batch 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 and the + // batch can keep accumulating. A pointer outside every Vulkan allocation is + // plain host memory and can never alias a bound buffer. void Copy(Queue&, void* dst, const void* src, size_t bytes) override { - VulkanContext::Get().FlushBatch(); + auto& ctx = VulkanContext::Get(); + ctx.FlushIfBatchTouches(TryResolve(dst).buffer, "copy-dst"); + ctx.FlushIfBatchTouches(TryResolve(src).buffer, "copy-src"); std::memcpy(dst, src, bytes); } // Synchronize was a no-op while every dispatch waited on its own fence. With // batching it is the caller's explicit "make results readable" point, and it is // what the tests and the engine use before touching device memory. - void Synchronize(Queue&) override { VulkanContext::Get().FlushBatch(); } + void Synchronize(Queue&) override { VulkanContext::Get().FlushBatch("synchronize"); } // THE REFERENCE-TIER SAFETY HOOK (backend.h:44-49, the seam Metal already // implements for M3c-1). op_provider.cpp calls this before running a PORTABLE // CPU kernel, which reads and writes this backend's device memory DIRECTLY @@ -100,7 +115,7 @@ class VulkanBackend final : public Backend { // dispatch's writes would be invisible to that host kernel -- stale bytes, no // error, no crash. This is what lets command-buffer batching be the DEFAULT // rather than an opt-in lever. - void FlushPending() override { VulkanContext::Get().FlushBatch(); } + void FlushPending() override { VulkanContext::Get().FlushBatch("reference-tier"); } // One process-wide VkQueue is shared by every vt::Queue: the queue handle is // the ORDERING domain and, with synchronous dispatch, every op is already @@ -147,6 +162,20 @@ bool UnregisterAllocation(void* base, void** out_buffer, void** out_memory) { return true; } +Resolved TryResolve(const void* ptr) { + const auto addr = reinterpret_cast(ptr); + std::lock_guard g(AllocMutex()); + auto& m = AllocMap(); + auto it = m.upper_bound(addr); + if (it != m.begin()) { + --it; + if (addr >= it->first && addr < it->first + it->second.bytes) { + return Resolved{it->second.buffer, static_cast(addr - it->first)}; + } + } + return Resolved{}; +} + Resolved Resolve(const void* ptr, const char* what) { const auto addr = reinterpret_cast(ptr); std::lock_guard g(AllocMutex()); diff --git a/src/vt/vulkan/vulkan_buffers.h b/src/vt/vulkan/vulkan_buffers.h index d2cafdc79..7ff43499a 100644 --- a/src/vt/vulkan/vulkan_buffers.h +++ b/src/vt/vulkan/vulkan_buffers.h @@ -50,6 +50,10 @@ struct Resolved { uint32_t offset = 0; // BYTE offset from the buffer's start }; Resolved Resolve(const void* ptr, const char* what); +// Non-throwing: returns {nullptr, 0} when `ptr` is not inside any Vulkan +// allocation. Needed to ask "is this pointer device memory?" of an arbitrary +// host pointer without treating the answer NO as an error. +Resolved TryResolve(const void* ptr); } // namespace vt::vulkan diff --git a/src/vt/vulkan/vulkan_context.cpp b/src/vt/vulkan/vulkan_context.cpp index 406e74dbf..bf973de8d 100644 --- a/src/vt/vulkan/vulkan_context.cpp +++ b/src/vt/vulkan/vulkan_context.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -545,6 +546,7 @@ VulkanContext::VulkanContext() { dispatch_hist_ = new std::map(); dispatch_ms_ = new std::map(); batch_names_ = new std::vector(); + batch_buffers_ = new std::set(); // TWO timestamps per dispatch (before and after), for a whole batch. Created // only under the stats flag: a query pool is cheap, but writing timestamps adds // commands to every dispatch and production must not pay for a diagnostic. @@ -897,17 +899,54 @@ uint32_t VulkanContext::pending_batch() const { return batch_count_; } -void VulkanContext::FlushBatch() { +void VulkanContext::FlushIfBatchTouches(void* buffer, const char* why) { std::lock_guard guard(*static_cast(mutex_)); - FlushBatchLocked(); + if (!batch_open_) return; + // A host pointer (nullptr) cannot alias a bound VkBuffer, so it never forces a + // drain. Otherwise drain only on a genuine intersection with the open batch. + if (buffer == nullptr) return; + const auto& bound = *static_cast*>(batch_buffers_); + if (bound.find(buffer) == bound.end()) return; + FlushBatchLocked(why); +} + +void VulkanContext::FlushBatch(const char* why) { + std::lock_guard guard(*static_cast(mutex_)); + FlushBatchLocked(why); } // Ends the open command buffer, submits it, and WAITS. The wait is what makes // every descriptor set in the batch free to reuse and every write visible to the // host, so it is not an optimisation to drop: without it the reset below would // race the GPU. -void VulkanContext::FlushBatchLocked() { +void VulkanContext::FlushBatchLocked(const char* why) { if (!batch_open_) return; + if (kDispatchStats) { + static std::mutex fw_mu; + static std::map> fw; // reason -> (count, dispatches) + static bool reg = false; + std::lock_guard g(fw_mu); + auto& e = fw[why]; + e.first += 1; + e.second += batch_count_; + if (!reg) { + reg = true; + static auto* snap = &fw; + static auto* snap_mu = &fw_mu; + std::atexit([] { + std::lock_guard g2(*snap_mu); + std::fprintf(stderr, "[vt vulkan] FLUSH TRIGGERS %-16s %10s %12s %8s\n", + "reason", "flushes", "dispatches", "avg"); + for (const auto& kv : *snap) { + std::fprintf(stderr, "[vt vulkan] %-16s %10llu %12llu %8.1f\n", + kv.first.c_str(), + static_cast(kv.second.first), + static_cast(kv.second.second), + kv.second.first ? double(kv.second.second) / double(kv.second.first) : 0.0); + } + }); + } + } const VulkanApi& vk = Api(); auto device = Unpack(device_); auto cmd = Unpack(command_buffer_); @@ -951,6 +990,7 @@ void VulkanContext::FlushBatchLocked() { for (auto& kv : *static_cast*>(pipelines_)) { kv.second.used_this_batch = 0; } + static_cast*>(batch_buffers_)->clear(); batch_open_ = false; batch_count_ = 0; } @@ -996,7 +1036,7 @@ void VulkanContext::Dispatch(const std::string& name, const void* const* buffers // can reuse set 0, because the GPU has not necessarily read the earlier ones // yet. Flushing also resets every pipeline's counter. if (kBatchDispatch && (p.used_this_batch >= kRingDepth || batch_count_ >= kMaxBatch)) { - FlushBatchLocked(); + FlushBatchLocked(p.used_this_batch >= kRingDepth ? "ring-full" : "batch-cap"); } VkDescriptorSet set = kBatchDispatch ? p.sets[p.used_this_batch] : p.sets[0]; @@ -1072,6 +1112,10 @@ void VulkanContext::Dispatch(const std::string& name, const void* const* buffers } if (kBatchDispatch) { + auto& bound = *static_cast*>(batch_buffers_); + for (uint32_t i = 0; i < buffer_count; ++i) { + bound.insert(const_cast(buffers[i])); + } ++p.used_this_batch; ++batch_count_; return; // submitted by FlushBatch, at the next host read or Synchronize diff --git a/src/vt/vulkan/vulkan_context.h b/src/vt/vulkan/vulkan_context.h index 7e6ff2c39..199a1503c 100644 --- a/src/vt/vulkan/vulkan_context.h +++ b/src/vt/vulkan/vulkan_context.h @@ -127,7 +127,15 @@ class VulkanContext { // Copy/Memset are plain memcpy over the persistently mapped, host-coherent // allocation, so a pending batch means the host reads STALE bytes -- silently, // with no error. Backend::Copy, Memset and Synchronize all flush. - void FlushBatch(); + // `why` attributes the flush to its TRIGGER. Measured: a 27B decode does 212 + // flushes per TOKEN, most carrying only 1-2 dispatches, so batching is being + // defeated by something other than the ring. Which trigger fires decides the + // fix, and guessing has been wrong five times this session. + void FlushBatch(const char* why = "explicit"); + // Drains only if `buffer` (a packed VkBuffer, or nullptr for host memory) was + // bound by a dispatch in the currently open batch. See Backend::Copy for why + // that is the exact condition. + void FlushIfBatchTouches(void* buffer, const char* why); // Whether dispatch batching is active. Exposed so a test never has to restate // the default: the VK-A2 gate originally re-derived it from the environment // variable and silently asserted the wrong branch the moment the default @@ -212,7 +220,7 @@ class VulkanContext { void* scratch_buffer_ = nullptr; // VkBuffer void* scratch_memory_ = nullptr; // VkDeviceMemory void* scratch_mapped_ = nullptr; // host pointer - void FlushBatchLocked(); // caller holds mutex_ + void FlushBatchLocked(const char* why = "explicit"); // caller holds mutex_ // GPU TIMESTAMP PROFILING. Batching submits many dispatches under ONE fence, // so the per-dispatch fence wait that used to attribute time to a shader no // longer exists. Timestamps written into the command buffer are the only way to @@ -227,6 +235,7 @@ class VulkanContext { void* batch_names_ = nullptr; // std::vector*, one per recorded dispatch bool batch_open_ = false; // a command buffer is recording uint32_t batch_count_ = 0; // dispatches recorded into it + void* batch_buffers_ = nullptr; // std::set*, buffers the open batch bound void* dispatch_hist_ = nullptr; // std::map* void* dispatch_ms_ = nullptr; // std::map* uint64_t dispatch_total_ = 0; diff --git a/src/vt/vulkan/vulkan_ops.cpp b/src/vt/vulkan/vulkan_ops.cpp index 66fcce503..605749e11 100644 --- a/src/vt/vulkan/vulkan_ops.cpp +++ b/src/vt/vulkan/vulkan_ops.cpp @@ -585,6 +585,29 @@ bool GemvMatmulUsable(bool bt, int64_t k, int64_t m) { return v != nullptr && std::strcmp(v, "0") == 0; }(); if (kDisabled) return false; + + // WHY IT DECLINED, once per distinct reason, under VT_VULKAN_DISPATCH_STATS. + // Same reasoning as the coopmat predicate above: a 27B decode profile showed the + // UNTILED SCALAR kernel still taking 256 calls at 12.53 ms -- one per output + // token, and the largest single per-call cost in decode -- and no amount of + // reading the source says WHICH clause sent it there. + if (kCoopMatWhy) { + const char* why = nullptr; + if (!bt) why = "not MatmulBT (b is [K,N]; that layout is already coalesced)"; + else if (m != 1) why = "M is not 1 (not a decode-shaped GEMV)"; + else if (k < static_cast(kWorkgroupSize)) why = "K is below one workgroup width"; + if (why != nullptr) { + static std::mutex gseen_mu; + static std::set gseen; + std::lock_guard g(gseen_mu); + if (gseen.insert(std::string(why)).second) { + std::fprintf(stderr, "[vt vulkan] gemv DECLINED: %s (bt=%d m=%lld k=%lld)\n", + why, bt ? 1 : 0, (long long)m, (long long)k); + std::fflush(stderr); + } + } + } + if (!bt || m != 1) return false; return k >= static_cast(kWorkgroupSize); }