ggml : backport persistent view initialization after buffer-size splits - #37
Open
arc-uri-el wants to merge 2 commits into
Open
arc-uri-el wants to merge 2 commits into
arc-uri-el wants to merge 2 commits into
Conversation
When ggml_backend_alloc_ctx_tensors_from_buft splits allocation on buft max_size, a view-only tail at the end of the context could skip the final alloc_tensor_range. Persistent views (e.g. KV k_stream / v_stream) were then left without ggml_backend_view_init. Allocate only parent tensors in alloc_tensor_range and initialize all views in a final pass over the context after all splits complete. (cherry picked from commit 335c098)
Cover the buft max_size split path where a view-only tail would skip ggml_backend_view_init without the finalize pass. (cherry picked from commit 0074731)
Collaborator
|
Wouldn’t this be picked up by an upstream rebase? |
dzannotti
requested changes
Sep 15, 2026
Collaborator
There was a problem hiding this comment.
Validation: Release test-alloc passed, including test_view_init_after_max_size_split.
Scope: this is a general allocator correctness backport, not a gfx1151 change, and the branch is behind master. Please land/stage it in halo-box/llama.cpp/upstream (or rebase if master already contains it), then sync the Strix fork instead of carrying a permanent local divergence.
comment generated by my clanker Codex
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.
Overview
Backport Yoshinao Kadowaki's allocator fix and regression from ggml-org#25584. The original authors and cherry-pick provenance are retained:
335c0989ed71e0207b212da063a40a5845a00582->14aa71aa144e5c51660ecf3cf75de375cd568e330074731c9a0ee2b453fe222d59a4aa24ad9cb696->72dbbbd207c3f5ceaaff3177bac821212f612682The test insertion conflicted with newer Halo tests; both sets are retained. No state-I/O skip guards, new checkpoint APIs or unrelated scheduling changes are included. This is not a claim to have authored the upstream fix.
On the current tested Halo base, a large KV parent exceeds the Vulkan buffer type's maximum allocation chunk. A trailing view-only range is left without
ggml_backend_view_init. Saving a displaced conversation then aborts on that nonempty view. The change allocates parents within ranges and initializes views after all ranges have been allocated.Fresh diagnosis on the Radeon 8060S identified:
The view represents live KV data. A
!tensor->dataguard in state I/O would conceal the allocation failure and omit that data; it is not the repair submitted here. The diagnostic interposer only printed these fields and then called the original reader, preserving the abort.Measurements
The baseline and backport were built independently in this session with identical compiler/backend settings and no compiler warnings. Mapped libraries were verified for each test. The subsequent master merge
99a40a3e6changes speculative replay, not this allocation path; it is not included in these measured builds.Baseline / after:
test-allocexecutable with view-tail regressionThe official witness uses the unmodified files in
ggml-org/Qwen3.8-27B-GGUF:Qwen3.8-27B-Q4_K_M.gguf: SHA-25631629f53165ab6a7dad8c9847dcfd1fdf55829dac1e6e748f4a68581b0033d34mtp-Qwen3.8-27B-Q8_0.gguf: SHA-256cbf60a0c48b431bb61f1d49b8948dc88ac29c398d6dbdbbb2e6e89ef77eacc9aThe server tuple was one unified slot with 1,000,000 configured tokens (1,000,192 physical padding), Q8_0 target KV, Q4_0 draft KV, MTP depth 3, batch 2048/512, 32 checkpoints and 16 GiB RAM cache. The requests were a 512-token shallow prompt with 256 generated tokens, a different 5,000-token prefix, then the original shallow prompt. The 256-token cap is an output-identity control, not a completed-task claim.
Relevant server arguments for the allocation and prompt-cache boundary:
Correctness:
test-allocincludes the upstream small dummy-backend reproducer, so reaching the view-tail bug does not require a large model. All 17 tests pass on the backport; the same executable aborts at the new view check when linked to the baseline libraries.A separate official-Qwen Vulkan control used each of the repository's prose, code, structured and numeric corpora, repeated/truncated to 512 tokens, followed by eight greedy steps. All nine complete logit rows, prompt IDs and selected IDs were byte-identical before and after for each corpus, with Q8_0 KV and identical settings. The model processes exited normally and were confirmed gone.
This backport makes no throughput claim.
llama-bench, perplexity and the full backend-op suite were not run. The evidence establishes the allocator view-init regression, actual affected Vulkan state save/recovery and bounded output preservation, not global backend qualification.Requirements