mudler / mudler/vllm.cpp

fix(ENG-RUNNER-MODELSHAPE): block-table width aligns by 128/block_size, not 128/gcd(128, block_size)

Open
#2,656 0 comments 0 reactions 0 assignees View on GitHub

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 the BlockTable constructor,
    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, and SlotMappingMode has zero hits in this tree. It
    needs that enum ported first and is a separate, larger unit.
  • Also surface-absent: requires_block_table_width on 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.