Repository navigation
feat: add random serving benchmark workload - #5036
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a synthetic prompt generation feature (similar to vLLM's random benchmark mode) to the serving benchmark tool, allowing users to generate random prompts with reproducible token lengths. It adds new command-line arguments, updates the benchmark runner to support ignoring EOS tokens, and implements random request sampling in the utility module. The review feedback highlights opportunities to prevent special tokens from being introduced during random prompt generation, add validation for input and output lengths, and safely access the API response's usage statistics to avoid potential KeyErrors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
cb138d1 to
163b31f
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for generating synthetic, random benchmark requests (similar to vLLM's random benchmark mode) without requiring a dataset. It updates the documentation, adds new command-line arguments to benchmark_serving.py, and implements robust synthetic request generation in utils.py. Additionally, it introduces an --ignore-eos option and improves token usage tracking in benchmark_runner.py. The review feedback suggests enhancing error handling when parsing usage statistics from API responses and adding input validation to ensure args.request_rate is positive to prevent runtime errors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
rogercloud
left a comment
There was a problem hiding this comment.
Summary
Overall the feature is well-implemented — clean separation between CLI parsing (benchmark_serving.py), benchmark orchestration (benchmark_runner.py), and prompt generation (utils.py). The ignore_eos threading and completion_tokens defensive handling are good improvements. The --num-prompts spelling fix in README is also appreciated.
Issues
- Major —
_num_special_tokens_to_addreturns 0 for non-callable attributes (see inline) - Minor —
request_ratevalidation happens after tokenizer loading (see inline)
Positive Observations
ignore_eosis cleanly threaded through all three constructor chains (BenchmarkRunner→ConcurrentBenchmarkRunner→ServingBenchmarkRunner)completion_tokensfallback logic insend_requesthandles missing usage data gracefully in both streaming and non-streaming pathsparse_random_range_ratioprovides good error messages for CLI misconfiguration_gen_prompt_decode_to_target_lenretry loop correctly handles tokenizer decode-re-encode mismatches- Reproducibility is preserved via
np.random.default_rng(seed)local RNG for prompt generation alongside globalnp.random.seedfor request timing
Validation
py_compilepasses on all three changed Python files
rogercloud
left a comment
There was a problem hiding this comment.
Re-review
All 9 prior findings (Gemini ×7, rogercloud ×2) are fully addressed. Three new minor issues found via inline comments below.
rogercloud
left a comment
There was a problem hiding this comment.
All three follow-up findings are addressed:
benchmark_runner.py—logger.warningadded for non-streaming missing-usage fallback. ✓benchmark_serving.py—_validate_random_range_ratiohelper validates[0, 1)for both the scalar and JSON dict paths (also catches invalidinput/outputvalues individually). ✓utils.py—np.setdiff1dreplaces the Python set subtraction. ✓
LGTM.
Summary
--dataset-name randomtobenchmark_serving.pyfor vLLM-style synthetic request generation without ShareGPT.ignore_eosthrough the benchmark runner so synthetic output lengths are more stable.--num-promptsflag spelling.Validation
python -m black --check benchmark/utils.py benchmark/benchmark_runner.py benchmark/benchmark_serving.pypython -m isort --check-only benchmark/utils.py benchmark/benchmark_runner.py benchmark/benchmark_serving.pypython -m flake8 benchmark/utils.py benchmark/benchmark_runner.py benchmark/benchmark_serving.pypython -m py_compile benchmark/utils.py benchmark/benchmark_runner.py benchmark/benchmark_serving.pypython benchmark/benchmark_serving.py --help