Skip to content

vulkan : reuse descriptor sets when bindings are constant - #29280

Merged
0cc4m merged 2 commits into
ggml-org:masterfrom
realrasengan:vulkan-descriptor-reuse
Sep 29, 2026
Merged

0cc4m merged 2 commits into
ggml-org:masterfrom
realrasengan:vulkan-descriptor-reuse

Conversation

@realrasengan

Copy link
Copy Markdown
Contributor

Overview

Descriptor sets come from a per-context ring whose cursor resets for every graph, so when the same graph is recorded again it already lands on the same set with the same buffers, offsets and ranges as last time. This keeps the last binding list written to each set and skips vkUpdateDescriptorSets when the new list is identical giving a 20-39% improved decode performance in translation layers and would be interesting to see how much it improves discrete GPU architectures broadly.

For the record, a skip keyed on VkBuffer handles alone is unsafe since a destroyed buffer's handle can be handed to a new buffer, and the set would still point at freed memory. As a result, the device counts buffer destroys and a context drops all of its cached bindings whenever that count has moved. This is coarse on purpose, since destroys are rare in steady-state decode. Without it, test-backend-ops -b Vulkan0 fails 63 IM2COL_3D cases with ERR = inf and then crashes in TOP_K but with it, 18933 / 18933 pass.

To be clear, the gain depends on how expensive descriptor updates are for the driver. On MoltenVK on an Apple machine there was minimal to no benefit with 0 regression.

Perplexity on wikitext-2 is identical to master, all 569 chunks.

GGML_VK_DISABLE_DESCRIPTOR_REUSE=1 is a flag that can disable this patch and is read once on device creation.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: Yes. AI was used to assist with efficiently writing the patch, and subsequently human reviewed and tested.

@realrasengan
realrasengan requested a review from a team as a code owner September 22, 2026 17:46
@github-actions github-actions Bot added Vulkan Issues specific to the Vulkan backend ggml changes relating to the ggml tensor library for machine learning labels Sep 22, 2026
@jeffbolznv

Copy link
Copy Markdown
Contributor

Where is the 20-39% benefit?

I don't expect any gains from this change and I think it is just a risk for bugs.

@realrasengan

Copy link
Copy Markdown
Contributor Author

Where is the 20-39% benefit?

I don't expect any gains from this change and I think it is just a risk for bugs.

The numbers are from a layered Vulkan implementation running gemma-2-2b Q4_K_M, same build with the feature on and off:
+38.8% decode on a short prompt
+20.1% on a long prompt
+32.3% on a long prompt with graph replay off

In a layered implementation, the descriptor update crosses a guest-to-host boundary, so its slower than a local write. I agree that on a discrete GPU there may be minimal gains.

An earlier version of this was unsafe, and llama.cpp's test suite caught it with 63 IM2COL_3D cases failing with ERR = inf and then a crash in TOP_K.

That is why the device counts buffer destroys and a context drops its cached bindings when that count moves. With that guard, all tests pass [1] and perplexity on wikitext-2 is identical to master across all 569 chunks.

[1] 18,933 of 18,933 in test-backend-opsand ci/run.sh 52/52

@jeffbolznv

Copy link
Copy Markdown
Contributor

If we're going to optimize for this case I'd rather do something more like #24720 and reuse the whole command buffer.

@realrasengan

Copy link
Copy Markdown
Contributor Author

If we're going to optimize for this case I'd rather do something more like #24720 and reuse the whole command buffer.

24720 is likely not thread-safe according to its author. This one is a maintainable 40 lines with an on/off flag. To be clear, though, this does become redundant if 24720 is merged.

We do actually do graph-replay in our own stack, at the host layer, and it gives us about 2.15x on decode.

That said, 24720 has been in draft for a few months whereas this is mergable.

@jeffbolznv

Copy link
Copy Markdown
Contributor

Ok, I don't see any bugs in the logic so I don't object.

@0cc4m

0cc4m commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Please rebase onto latest master to fix the CI issue (needs #29279)

abraham-imre pushed a commit to abraham-imre/llama.cpp that referenced this pull request Sep 23, 2026
Cherry-pick of ggml-org/llama.cpp PR ggml-org#29280 (ggml-org#29280)
Author: Andrew Lee (@realrasengan)

Reason for our setup: Cuts CPU-side overhead by reusing descriptor sets when bindings are constant
@realrasengan
realrasengan force-pushed the vulkan-descriptor-reuse branch from b99d497 to d2b0b6d Compare September 24, 2026 20:40
@realrasengan

Copy link
Copy Markdown
Contributor Author

Please rebase onto latest master to fix the CI issue (needs #29279)

My apologies for the delay. It's been rebased now, and thank you!

@0cc4m

0cc4m commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

What does "layered Vulkan implementation" mean specifically? #24720 was not pursued further because I couldn't find a system where it provided a measurable benefit. If you have a case we can look into reopening it.

@realrasengan

Copy link
Copy Markdown
Contributor Author

What does "layered Vulkan implementation" mean specifically?

We run VMs without direct GPU access on Apple's hypervisor, using venus in the guest to serialize Vulkan commands over virtio-gpu to the host, wherein they're translated to Metal. This means every vkAllocateDescriptorSets / vkUpdateDescriptorSets costs a guest-host barrier round trip. Reusing descriptor sets when the bindings are constant avoids most of those round trips, like a cache, and gives a significant speedup!

Thank you again!

@realrasengan

Copy link
Copy Markdown
Contributor Author

By the way it looks like the 2 failed tests were pre-existing (not relating to this PR)!

@tczank

tczank commented Sep 26, 2026

Copy link
Copy Markdown

Discrete-GPU data point, as requested: RX 7900 XTX (gfx1100, 24 GB, PCIe 3.0 x16), Linux Mesa 26.2 RADV, llama-bench, dense 27B IQ3_S, -r 3, interleaved A1/B1/A2/B2 on the same machine:

build pp512 (t/s) tg128 (t/s)
master a02c7f5 980.2 ± 1.3 50.98 ± 0.08
+ d2b0b6d (this PR) 976.8 ± 2.6 50.92 ± 0.09

−0.35% pp / −0.1% tg — within run-to-run noise (leg spread ~1% on pp). No functional issues across the runs.

So the expectation holds for discrete RDNA3: descriptor re-recording is not a measurable cost when the driver isn't serializing over virtio — the Venus numbers don't transfer. Neutral change here: not a perf win on RADV, but also no regression or observed instability.

Comment thread ggml/src/ggml-vulkan/ggml-vulkan-types.h Outdated
@0cc4m
0cc4m merged commit 0bc845d into ggml-org:master Sep 29, 2026
21 checks passed
pierreguillot pushed a commit to Ircam-Partiels/llama.cpp that referenced this pull request Oct 1, 2026
…9280)

* vulkan : reuse descriptor sets when bindings are constant

* vulkan : bump buffer_destroy_count before destroying the buffer
frostyautumnleaf pushed a commit to frostyautumnleaf/llama.cpp that referenced this pull request Oct 5, 2026
…9280)

* vulkan : reuse descriptor sets when bindings are constant

* vulkan : bump buffer_destroy_count before destroying the buffer
edwardyoon pushed a commit to edwardyoon/focus-llama that referenced this pull request Oct 7, 2026
…9280)

* vulkan : reuse descriptor sets when bindings are constant

* vulkan : bump buffer_destroy_count before destroying the buffer

(cherry picked from commit 0bc845d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ggml changes relating to the ggml tensor library for machine learning Vulkan Issues specific to the Vulkan backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants