Skip to content

Conversation

@jr-shen
Copy link

@jr-shen jr-shen commented Nov 28, 2025

Purpose

Fix a bug in vllm bench throughput where --dataset-name prefix_repetition is incorrectly routed to RandomDataset when --dataset-path is not provided, due to:

if args.dataset_path is None or args.dataset_name == "random":
    ...
    dataset_cls = RandomDataset

This causes prefix-repetition benchmarks to ignore --prefix-repetition-* arguments and fall back to random dataset defaults (e.g. input-len=1024, output-len=128).

The fix makes prefix_repetition explicitly use PrefixRepetitionRandomDataset with its own CLI args, and only uses RandomDataset for true random/default cases.

Test Plan

  1. prefix_repetition dataset
vllm bench throughput \
  --model facebook/opt-125m \
  --backend vllm \
  --dataset-name prefix_repetition \
  --prefix-repetition-prefix-len 60 \
  --prefix-repetition-suffix-len 40 \
  --prefix-repetition-num-prefixes 1 \
  --prefix-repetition-output-len 1 \
  --max-model-len 2048 \
  --max-num-batched-tokens 2048 \
  --max-num-seqs 1 \
  --num-prompts 1 \
  --dtype float16 \
  --seed 42
  1. random dataset regression
vllm bench throughput \
  --model facebook/opt-125m \
  --backend vllm \
  --dataset-name random \
  --input-len 100 \
  --output-len 1 \
  --max-model-len 2048 \
  --max-num-batched-tokens 2048 \
  --max-num-seqs 1 \
  --num-prompts 1 \
  --dtype float16 \
  --seed 42

Test Result

1. prefix_repetition test

  • Before fix:
INFO ... Sampling input_len from [1023, 1023] and output_len from [128, 128]
...
Total num prompt tokens:  1024
Total num output tokens:  128
  • After fix:
...
Total num prompt tokens:  101
Total num output tokens:  1

(≈ 60 + 40 input tokens and 1 output token, as configured.)

2. random regression test

  • Before / after fix (unchanged):
INFO ... Sampling input_len from [99, 99] and output_len from [1, 1]
...
Total num prompt tokens:  100
Total num output tokens:  1

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

@mergify mergify bot added the performance Performance-related issues label Nov 28, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly fixes a bug where vllm bench throughput with --dataset-name prefix_repetition would incorrectly use RandomDataset. The change ensures that prefix_repetition is routed to its specific dataset implementation. However, the fix is specific to prefix_repetition and doesn't address the same issue for other pathless synthetic datasets like random-mm and random-rerank. I've suggested a more comprehensive fix to prevent similar bugs with other datasets.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Junru Shen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Performance-related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant