Skip to content

[Runner] Never pad a step past max_num_batched_tokens - #3466

Open
gutianyu-google wants to merge 2 commits into
vllm-project:mainfrom
gutianyu-google:fix/token-paddings-include-max-batched
Open

[Runner] Never pad a step past max_num_batched_tokens#3466
gutianyu-google wants to merge 2 commits into
vllm-project:mainfrom
gutianyu-google:fix/token-paddings-include-max-batched

Conversation

@gutianyu-google

@gutianyu-google gutianyu-google commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

[Runner] Never pad a step past max_num_batched_tokens

Problem

TPUModelRunner builds num_tokens_paddings with get_token_paddings(min, max_num_batched_tokens * dp_size, gap).
With the default VLLM_TPU_BUCKET_PADDING_GAP=0 the buckets double until they cover the budget, so a full
chunked-prefill step is padded to the next power of two
whenever the budget is not one itself. Example
(Qwen3-0.6B, TP8 x DP16, max_num_batched_tokens=16640): a 16640-token prefill chunk runs at the 32768-token
shape per rank — 97% padding.

On that shape we also observe a silent correctness problem: the last-position logits of the padded prefill step
are wrong, so the first sampled token of ~90% of requests is garbage ( tắc, iquement, 人力, ...) before
the model recovers on the next decode step. Reproduced on TPU7x with page_size 128, 1 KV head/device
(tpu-inference @ be9cdf0): first token correct for 0/2048 requests at the 32768 shape, 2048/2048 when the
same chunk is padded to 18432 (VLLM_TPU_BUCKET_PADDING_GAP=32768), 16384 (max_num_batched_tokens=12544),
or 4096. The kernel-level cause at the 32768 shape is not localized here; this PR removes the trigger
and the padding waste.

Change

  • runner/utils.py::build_token_paddings(min, max, gap, additional_sizes): the runner's bucket list is now the generated
    buckets capped at the budget (rounded up to a multiple of min_token_size), the budget itself, and the user's
    additional_config.compilation_sizes, merged and deduplicated in one place. The budget enters through the same merge
    that already handled compilation_sizes; no new flag on get_token_paddings, which is unchanged.
  • Buckets above the budget are dropped: the scheduler never schedules more than max_num_batched_tokens per rank, so
    they are unreachable and only cost a compile each and oversize the preallocated token buffers (max_num_tokens is
    derived from the last bucket). With a 16640 budget the list ends ..., 8192, 16384, 16640 instead of
    ..., 8192, 16384, 32768. User-supplied compilation_sizes are kept as given, even above the budget.
  • runner/tpu_runner.py: builds the list through the helper. A budget that is already a bucket (e.g. 8192, 16384)
    yields the stock list, so those configurations see no change.
  • tests/runner/test_utils.py: capped list for the exponential and bucketed gaps, stock list for a power-of-two budget,
    rounding of the budget, and compilation_sizes preserved and deduplicated.

Validation

  • Unit tests for get_token_paddings.
  • End-to-end (MaxText/tunix GRPO, Qwen3-0.6B, TP8xDP16, budget 16640): with the patch and no env override,
    first token correct for 2048/2048 requests (LENDIST census), generation distribution identical to the
    VLLM_TPU_BUCKET_PADDING_GAP=32768 workaround; without it 0-256/2048. Run 51 (this patch, no env): first token correct 2048/2048 at steps 0 and 1; 29.3% / osl 4477 at step 0 (workaround run: 27.5% / 4391 — same distribution, different samples); step-1 engine_generate 84.5 s vs 88.5 s with the 18432 workaround and 82.9 s on the corrupted 32768 shape, i.e. the exact-budget bucket also pads less than the gap workaround.
  • No behaviour change when the budget is already a bucket (e.g. 8192, 16384).

@gutianyu-google
gutianyu-google force-pushed the fix/token-paddings-include-max-batched branch 2 times, most recently from 12b3330 to dfb2979 Compare September 4, 2026 18:49
@gutianyu-google
gutianyu-google marked this pull request as ready for review September 4, 2026 18:55
@sixiang-google sixiang-google added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 4, 2026
Comment thread tpu_inference/runner/tpu_runner.py Outdated
@wenxindongwork

Copy link
Copy Markdown
Collaborator

btw, you can also just use additional_config.compilation_sizes to pass in custom compilation sizes.

@gutianyu-google
gutianyu-google force-pushed the fix/token-paddings-include-max-batched branch from dfb2979 to 6549988 Compare September 4, 2026 23:18
get_token_paddings() builds the token buckets by doubling (or by
VLLM_TPU_BUCKET_PADDING_GAP steps) until the budget is covered, so a full
chunked-prefill step is padded to the next bucket above
max_num_batched_tokens * dp whenever the budget is not itself a bucket.
With max_num_batched_tokens=16640 and dp=16 a 16640-token chunk runs at the
32768-token shape per rank (97% padding).

On TPU7x with Qwen3-0.6B (TP8 x DP16, page_size 128) that padded shape also
returns wrong last-position logits: the first sampled token of ~90% of
requests is garbage before generation recovers on the next decode step. The
same chunk padded to 18432, 16384 or 4096 is correct.

Add include_max_token_size to get_token_paddings() and enable it for the
runner's token-bucket table so the exact budget is always a bucket. Larger
buckets are kept, so max_num_tokens and pre-allocation are unchanged.

Validated end-to-end (MaxText/tunix GRPO, 2048 requests): first token
correct 2048/2048 with the patch vs 0-256/2048 without, and the prefill step
is ~4 s/step faster than the VLLM_TPU_BUCKET_PADDING_GAP=32768 workaround.

Signed-off-by: Tianyu Gu <tianyugworker@gmail.com>
@gutianyu-google
gutianyu-google force-pushed the fix/token-paddings-include-max-batched branch from 6549988 to 83a4e61 Compare September 7, 2026 05:22
…ets above it

Instead of a new flag on get_token_paddings, the runner now builds its
bucket list with build_token_paddings: the generated buckets capped at the
budget, the budget itself (rounded up to a multiple of the minimum bucket),
and the user's compilation_sizes, merged and deduplicated in one place.
Buckets above the budget are unreachable (the scheduler never exceeds it)
and only cost a compile each and oversize the preallocated token buffers.
A budget that is already a bucket yields the stock list.
@gutianyu-google

Copy link
Copy Markdown
Contributor Author

Done, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants