Avoid repeated reallocation of the TopK buffer - #1011
Open
Magnushst wants to merge 1 commit into
Open
Conversation
`TopK` fills a `std::vector<double>` with one packed entry per logit, but starts from an empty vector. `logits.size()` is the vocabulary size, so for Gemma 3's 256K vocabulary libstdc++ reallocates 19 times and copies 2.1 MB before the buffer reaches its final size. This happens on every sampled token whenever `--top_k` is greater than 1. Reserve the full size up front, and hoist the `accept_token` test out of the loop so that the common case (no constrained decoding) is a straight-line pack-and-append. Peak transient memory also falls, because the final doubling no longer holds the old and new buffers at the same time. Sampling results are unchanged: the same entries are appended in the same order, so the subsequent VQSelect/VQSort see identical input. Measured on an i9-13905H (AVX2, GCC 14.2, -O3 -DNDEBUG), best of 30 calls, median of three interleaved baseline/patched runs: vocab 256K, k=50 367 us -> 284 us (-23%) vocab 32K, k=50 43.4 us -> 33.3 us (-23%) Add `TestTopK`, which covers both the filtered and the unfiltered path.
jan-wassenberg
approved these changes
Sep 3, 2026
jan-wassenberg
left a comment
Member
There was a problem hiding this comment.
Nice improvements, thank you!
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.
Summary
TopKfills astd::vector<double>with one packed entry per logit but starts from an empty vector.logits.size()is the vocabulary size, so for Gemma 3's 256K vocabulary libstdc++ reallocates 19 times and copies 2.1 MB before the buffer reaches its final size. This runs once per sampled token viaFusedSoftmaxAndSampleTopK, whichChooseSampleFuncselects whenevertop_kis greater than 1 or anaccept_tokenfilter is set.This reserves the full size up front and hoists the
accept_tokentest out of the loop, so the common case (no constrained decoding) is a straight-line pack-and-append rather than astd::functiontest per logit. Peak transient memory also falls, because the final doubling no longer holds the old and new buffers at the same time.Sampling results are unchanged: the same entries are appended in the same order, so the subsequent
VQSelect/VQSortsee identical input.Performance
Deterministic reduction, for a 256K vocabulary:
Wall time, interleaved baseline/patched runs, best of 30 calls (300 for the small cases), median of three runs:
The
accept_tokencase is the control: it keeps the per-element filter call, which dominates, so it is unaffected.Environment:
This is a sampling-path improvement, not an end-to-end decode speedup; on a memory-bound decode step the saving is small next to the output-head MatMul.
Testing
TestTopKcovers the filtered and unfiltered paths, asserting exact tokens and probabilities. Its size exceeds the vector's initial capacity, so it exercises the reserved-capacity path.ops_testpasses 40/40 across AVX2 and EMU128;compress_testandsfp_testunaffected.ops_test.ccbuilt with-fsanitize=addressand run withdetect_leaks=1: 40/40 pass, no errors and no leaks.Notes
Only x86-64 (AVX2) was measured; ARM was not benchmarked. The change is allocation behaviour rather than SIMD, so it should carry over, but I have not verified that.