Backend sampling multi output - #25532
Conversation
f5e3f22 to
5897112
Compare
a1b62dd to
466696c
Compare
|
Backend sampling shows ~12% improvement on Windows + RTX 5090: CPU Sampling: Backend sampling: |
762a06e to
89cd866
Compare
|
Backend sampling shows ~4% improvement on Linux + Tesla P40 (sm_61, Pascal): CPU Sampling: Backend sampling: Acceptance ratio with both backend and CPU sampling is exactly same. The improvement is smaller than on RTX 5090 (4% vs 12%), which is expected — the P40 is memory-bandwidth-bound (sm_61, 580 GB/s vs RTX 5090's 1,792 GB/s), so the CPU↔GPU logits round-trip is a smaller fraction of total decode time. However, still the largest improvement in tok/s I have seen in a while. (~+2 t/s). |
89cd866 to
4cbb71d
Compare
|
Any explanation for docs/ ? |
|
Trying to reproduce the claimed speedup, finding some different results. Setup:
MTP-only, controlled test (temp=0.01, top-k=64, top-p=0.95).(Avoided temp=0 since it reduces the entire sampling pipeline to a single argmax, which wouldn't test the backend properly). Full sampling pipeline, 8/9 prompts produce identical sequences.
Small overhead (~0.75%) on single-sequence workloads. weird. MTP-only, default temperature8/9 prompts produce different token sequences between baseline and patched.
Appears faster, but throughput difference is confounded by different speculative paths (different text generated). MTP + ngram-mod, default temperature
Sequences diverge heavily. MTP + ngram-mod, controlled test (temp=0.01)
Still divergent on 3/9 prompts. Where sequences match, patched is slightly slower (same pattern as MTP-only). The overall delta is driven by divergent prompts hitting different ngram-mod cache paths. How can I avoid prompt divergence for proper apples-to-apples benchmarking without resorting to temp=0? Since the seed is fixed, I assume the divergence at temp=0.01 is caused by floating-point arithmetic differences (e.g. softmax/cumsum) between CPU and GPU architectures. Is there a recommended way to enforce exact deterministic sampling across both paths? |
|
Here is what I see on my Linux (CachyOS) machine + 5090. I could not find the mtp-bench.py script so I had Claude reverse-engineer it. CPU sampling: llama-server -m Qwen3.6-35B-A3B-UD-Q4_K_M.gguf --spec-type draft-mtp --seed 42 Backend sampling: llama-server.exe -m Qwen3.6-35B-A3B-UD-Q4_K_M.gguf --spec-type draft-mtp --seed 42 -bs
|
|
Great improvements! TG increased by about 10%. Test environment: 4xP100 0.00.066.293 I cmn common_param: common_params_print_info: verbosity = 3 (adjust with the 0.10.283.512 W set_sampler: backend sampling not supported with SPLIT_MODE_TENSOR; using CPU 0.11.758.080 I common_speculative_init_result: creating MTP draft context against the target model '/var/model/gguf/Qwen3.6-27B/Qwen3.6-27B-Q4_0_bartowski_8c66.gguf' 0.13.839.650 W spec common_specu: backend offload failed for seq_id=0; using CPU sampler 0.13.959.036 I srv init: chat template supports preserving reasoning, consider enabling it via --reasoning-preserve |
There was a problem hiding this comment.
I think we can avoid the slot.backend_sampling flag and the related changes by adding an API llama_sampler_copy to llama.h like so:
void llama_sampler_copy(const struct llama_sampler * src, struct llama_sampler * dst) {
if (!src || !dst) {
return;
}
GGML_ASSERT(src->iface == dst->iface && "llama_sampler_copy: cannot copy between different sampler types");
// build a temporary sampler carrying src's current state
llama_sampler * tmp = llama_sampler_clone(src);
// free dst's old state (frees dst->ctx, including children for a chain)
if (dst->iface->free) {
dst->iface->free(dst);
}
// transplant tmp's state into dst, then destroy the (now empty) temp shell
dst->ctx = tmp->ctx;
tmp->ctx = nullptr;
delete tmp;
}This is similar to llama_sampler_clone, but the idea is to keep the pointer intact and just copy the internal context.
Unless I missed something, you should be able to replace this logic in server-context.cpp:
slot.prompt.tokens.keep_first(ckpt.n_tokens);
slot.smpl = std::move(smpl_save);
if (slot.backend_sampling) {
slot.backend_sampling = llama_set_sampler(
slot.ctx_tgt, slot.id, common_sampler_get(slot.smpl.get()));
}with simple unconditional:
slot.prompt.tokens.keep_first(ckpt.n_tokens);
llama_sampler_copy(smpl_save.get(), slot.smpl.get());This will avoid re-setting the sampler of the llama_context (avoiding sched reserve) and avoiding the extra slot.backend_sampling tracking in the server logic.
LMK if this makes sense.
4cbb71d to
8d16326
Compare
This makes sense as it can help avoid expensive sched reserves. However, the change is going to be more complicated. A simple clone-and-swap won't be enough. We will have to copy the backend sampler state and child samplers explicitly. I have attempted this change in the latest commit along the same lines. Please take a look. |
9fcc911 to
9f5d059
Compare
Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
…ped PR The declaration alone would have been orphaned: its implementation depends on a llama_sampler_i::copy_state interface field that is part of upstream PR ggml-org#25532 ('multi-output backend sampling', 24 files, ~1000 lines) - a squashed feature that predates this fork's real base and is out of scope for a 'zero TQ3 impact' sync. copy_state does not exist anywhere in this tree; a half-port would not compile. Nothing in this codebase currently calls llama_sampler_copy, so dropping the declaration is safe. The full feature belongs in the larger upstream content-catch-up, tracked separately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
28 upstream commits since 687e778. One touches ggml-opencl: 689e227 (ggml-org#26428, FA prefill K-tile transpose) -- our own PR, merged upstream this window, so its four files conflicted against the x2ue originals they were carved from. Resolved to ours in all four: x2ue is a strict superset (FA_Q_HALF / FA_O_HALF, the FA*_PROBE_NO_LDS diagnostics and FA_V_LDS_T all landed on top). Verified line-by-line that no ggml-org#26428 content was lost -- the seven upstream lines absent from the merged tree are the same code reshaped by our later commits (#elif rather than #if because our probe branch precedes, mad() rather than *, Q_PRIV_TO_ACC4() around q_priv), all still reading through FA_LK/FA_LK_PAIR, plus one reworded comment. Also verified the automerge kept upstream intact: of the 659 files upstream touched, the merged tree is byte-identical to upstream/master on all but the eight we also modified, and all 247 upstream-added lines in the four auto-merged files (clip.cpp, clip-model.h, llama-context.cpp, test-backend-ops.cpp -- Muse Glimmer ggml-org#26841, multi-output sampling ggml-org#25532, Granite-Switch ggml-org#25107) are present.
…, multi-output backend sampling, pocket-tts Merges 45 upstream commits (0865990..ebb546b, b10362-16-gebb546b7e) into synori/llama-update-mtp-fit. Zero conflicts; all vendored patches carried over untouched. Primary motivation — new Meta architecture: * 62bf73d model: Muse Glimmer Support (ggml-org#26841) LLM_ARCH_MUSE_GLIMMER + src/models/muse-glimmer.cpp + the mtmd vision tower in tools/mtmd/models/muse-glimmer.cpp and conversion/muse_glimmer.py. Other notable changes that touch our public API surface: * dd1ea52 llama : support multi-output backend sampling (ggml-org#25532) llama_context_params gains n_outputs_max_per_seq; llama_sampler_i.backend_init takes it as a third argument; new backend_reset / copy_state vtable slots and llama_sampler_copy(). * 153d324 llama : default load-mode auto, avoids mmap on iGPUs (ggml-org#26081) llama_load_mode gains LLAMA_LOAD_MODE_AUTO = -1 (enum is now signed). * 6e62ba5 mtmd: support pocket-tts (ggml-org#26871) mtmd_gen_inp/mtmd_gen_out gain seed/temp/feats/is_eos; new mtmd_gen_inp_default(); mtmd_helper_gen_audio_step_gen() takes out_stop. * 157b81f model : Granite-Switch Architecture (ggml-org#25107) * 7a20b41 model: MTP support for Nemotron (ggml-org#26725) and cc078b4 Dflash support for nemotron-3.5 (ggml-org#26905) * e23e944 vendor : cpp-httplib 0.53.0, 4c6766f vendor : subprocess.h sync Vendored patches preserved: * 919fde3 feat(rpc): thread-local last_error accessor — intact, upstream touched ggml-rpc.cpp by one unrelated line. * 3679b23 Fixes (RPC) — intact. * 2aa76c7 fix(metal): drop stray kernel_pad_f32 — still applies; upstream has since refactored pad into a templated kernel_pad_impl<T>, so the duplicate definition that referenced the nonexistent kargs_pad.s0..s3 is gone on both sides and nothing had to be re-applied.
* Enable backend sampling with token speculation * Clamp the mask sum before converting it into the sampled index * Add a numeric context parameter declaring the maximum outputs one sequence * More fixes * Don't reuse memory for output views. * Match dist between CPU and GPU * Fix CPU and backend sampling mismatches * Simpify some of the changes * Fix tests on Vulkan * More test fixes * Rebase changes * Rebase and address review comments * Address review comments * Address review comments * Update src/llama-sampler.cpp Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> --------- Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
Overview
This PR extends backend sampling from one output per sequence to multiple outputs, enabling backend sampling during speculative decoding.
n_sampling_outputs_per_seq_maxto context parameters, separating the total output budget from the per-sequence backend-sampling limit.backend_resethook to clear graph-owned tensor references before graph reconstruction.Additional information
Performance: I see a perf improvement of ~8% on RTX 5090 with the Qwen-3.6-35B Q4_K_M model. Acceptance ratio with both backend and CPU sampling is exactly same.
CPU sampling:
llama-server -m Qwen3.6-35B-A3B-UD-Q4_K_M.gguf --spec-type draft-mtp --seed 42Backend sampling:
llama-server -m Qwen3.6-35B-A3B-UD-Q4_K_M.gguf --spec-type draft-mtp --seed 42 -bsRequirements