fix(ENG-RUNNER-MODELSHAPE): block-table width aligns by 128/block_size, not 128/gcd(128, block_size)
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 423
- Forks
- 53
- Avg merge
- 20h 26m
- Merged PRs (30d)
- 310
Description
Row: ENG-RUNNER-MODELSHAPE
Found by wave PORTQ-2 while re-deriving PORT-NOW entry 47 of
5559679229..e126687a9a against this tree (#2646).
Upstream: a0cd2b69b3 vllm#50302, "Universally align block table width to 128 tokens".
What is here
src/vllm/v1/worker/gpu/block_table.cpp:210-216 carries the PRE-commit alignment
verbatim, cited to the same upstream issue (#39324):
// Align to a multiple of (128 / block_size) for block_size <= 128 (#39324).
for (size_t i = 0; i < num_blocks.size(); ++i) {
const int bs = block_sizes[i];
if (bs <= 128) {
const int mult = 128 / bs;
num_blocks[i] = cdiv(num_blocks[i], mult) * mult;
}
}
It is production code: MultiGroupBlockTable is constructed from InputBatch
(src/vllm/v1/worker/gpu/input_batch.cpp:105,123), and
include/vllm/v1/worker/gpu/block_table.h:161-163 states the alignment as a
contract.
What upstream now does
block_alignment = token_alignment // gcd(token_alignment, block_size), folded
into a get_block_table_width helper that also applies the
block_size / kernel_block_size virtual-block split.
Integer division and the gcd form agree for every block size that DIVIDES 128
(16, 32, 64, 128), and diverge otherwise. block_size = 48: the current form
computes mult = 2 and aligns to 96 tokens, which is not 128-aligned at all;
the gcd form computes 128 / gcd(128,48) = 8 and aligns to 384 tokens.
Scope, and what is NOT in it
- In scope: the gcd correction, and folding the
blocks_per_kv_block
multiply (today done separately in theBlockTableconstructor,
block_table.cpp:49) into one width function. Roughly 25-40 lines over
block_table.h/.cpp, plus a red-first test on a non-divisor block size. - Not in scope, and surface-absent: the commit's Mamba opt-out keys on
SlotMappingMode.NONE, andSlotMappingModehas zero hits in this tree. It
needs that enum ported first and is a separate, larger unit. - Also surface-absent:
requires_block_table_widthon the metadata builder,
because the builder factory is recorded as unported at
include/vllm/v1/attention/backend.h:15,283.
Honest weight
No configuration this tree builds today diverges. Every registered backend
declares kernel block sizes {16} or {1}, all of which divide 128. This is a
robustness repair against a block size nobody currently selects, and the owning
row is entitled to rank it accordingly.
Nothing was executed for this finding: no build, no test, no GPU. It is a static
reading of the two sources.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the alignment loop at src/vllm/v1/worker/gpu/block_table.cpp:210-216, then inspect the BlockTable constructor at block_table.cpp:49 and the contract in include/vllm/v1/worker/gpu/block_table.h:161-163. Trace construction from input_batch.cpp:105,123 and add a red-first test using a non-divisor block size. Done means the gcd-based width and blocks_per_kv_block handling are covered without adding the out-of-scope Mamba or metadata-builder work.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, performance
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100