docs(skills): add vllm-bench multi-turn serving benchmark skill - #424
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8de8090291
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -159,6 +159,7 @@ Organized by domain (model line / subsystem / playbook / lesson) instead of by l | |||
|
|
|||
| | Path | TL;DR | | |||
| | --- | --- | | |||
| | `benchmarks/qwen3-4b-multiturn-vllm-bench-rtx5090.md` | Qwen3-4B multi-turn `openai-chat` load on RTX 5090 with `vllm-bench` 0.1.0: 200 conversations x 5 turns, concurrency 16, 1.408M input tokens and 128k output tokens. OpenInfer completed `1000/1000` turns at 1122.68 output tok/s vs vLLM 0.22.1 at 1013.73 (+10.75%), with lower mean TTFT (114.29 vs 249.95 ms) and p99 E2EL (2346.97 vs 2802.64 ms). | | |||
There was a problem hiding this comment.
Scope the benchmark claim to the measured branch
This headline reads as a Qwen3/OpenInfer baseline, but the benchmark doc itself says the OpenInfer run used feat/green-ctx-sm-partition @ 65834e8 with untracked speculative-decoding files and should not be read as clean-main. In contexts where people scan docs/index.md for benchmark baselines or regression thresholds, this can turn a development-branch snapshot into a misleading mainline performance claim; please scope the row and the doc TL;DR to that worktree or remove the +10.75% headline.
Useful? React with 👍 / 👎.
| Prefix sharing works only with `--dataset-name random`; the two ratios must sum | ||
| to less than `1.0`. |
There was a problem hiding this comment.
Warn that prefix sharing disables history accumulation
When either prefix-sharing flag is set, vllm-bench switches multi-turn random generation into fixed-length prompts with no growing chat history; I checked the upstream CLI/source, which documents this as no history accumulation. Users following this skill to benchmark realistic multi-turn cache behavior could otherwise enable these flags thinking they are additive prefix sharing while measuring a different workload, so add that caveat here and in the duplicated .claude skill.
Useful? React with 👍 / 👎.
Add the vllm-bench skill (mirrored under .agents and .claude) for benchmarking OpenAI-compatible / vLLM serving endpoints with multi-turn chat load tests, concurrency sweeps, and TTFT/TPOT/throughput metrics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0b6231e to
baf4891
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baf4891255
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| --multi-turn | ||
| --multi-turn-num-turns <N> | ||
| --num-prompts <CONVERSATIONS> | ||
| --multi-turn-concurrency <CONCURRENT_CONVERSATIONS> |
There was a problem hiding this comment.
Clarify multi-turn concurrency units
When the run uses --multi-turn-delay-ms or --num-prompts exceeds this value, calling this <CONCURRENT_CONVERSATIONS> misstates what the flag limits. I checked upstream src/multi_turn.rs: it spawns one task per conversation and the semaphore “controls max in-flight requests (not conversations)” and is released before inter-turn delay, so this flag caps simultaneous turn requests rather than active conversations; users sizing session-load experiments from this skill can measure the wrong workload. The duplicated .claude skill has the same wording.
Useful? React with 👍 / 👎.
| this repo. Start OpenInfer: | ||
|
|
||
| ```bash | ||
| cd /data/code/workspace-rustllm/pegainfer-2 |
There was a problem hiding this comment.
Replace local-only smoke paths
This smoke test fails immediately for anyone whose checkout is not the author's /data/code/workspace-rustllm/pegainfer-2 path, and the same block later hard-codes a separate /data/code/workspace-rustllm/vllm-bench checkout. Since this skill is meant to guide agents/users from arbitrary OpenInfer worktrees, these should be placeholders or commands rooted at the current repo/binary location; the duplicated .claude skill has the same local paths.
Useful? React with 👍 / 👎.
The merged vllm-bench skill (#424) hardcoded machine-specific paths (`/data/models/Qwen3-4B`, `/data/code/workspace-rustllm/{pegainfer-2,vllm-bench}`). Replace them with `<model-path>`, `<openinfer-repo>`, and `<vllm-bench-repo>` placeholders, matching the path-sanitization convention used in the model docs. Applied identically to both `.agents` and `.claude` mirrors. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
vllm-benchskill, mirrored under both.agents/skills/and.claude/skills/.Scope
Skill only. The earlier RTX 5090 Qwen3-4B benchmark snapshot doc, its assets, and the profiling-guide / model-doc edits were dropped from this branch to keep the PR focused.
Validation
uv run --no-project --with pyyaml python .../skill-creator/scripts/quick_validate.py .agents/skills/vllm-benchuv run --no-project --with pyyaml python .../skill-creator/scripts/quick_validate.py .claude/skills/vllm-bench