Add ComfyUI Serving Benchmarks#13595
Conversation
This reverts commit ac85d78.
…ading image or model is already there, if it is, skip. 2. remove prompt-file arg, when generating a new request, roundrobin the generated prompts
…under input folder
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive benchmarking system for ComfyUI server performance measurement. It adds a new benchmark script that manages a model/task registry, downloads model weights, generates prompt JSONs from workflow templates, submits concurrent requests with configurable arrival patterns, and collects latency and throughput metrics. A new 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
benchmarks/README.md (1)
6-10: Consider documenting the optional Pillow dependency.The
benchmark_comfyui_serving.pyscript requires Pillow for synthetic image generation fallback (when VBench download fails). Adding it to the dependencies section would help users who encounter that code path.## Dependencies ```bash -pip install aiohttp tqdm gdown +pip install aiohttp tqdm gdown Pillow<details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@benchmarks/README.mdaround lines 6 - 10, Update the Dependencies section in
the README to include the optional Pillow package used by
benchmark_comfyui_serving.py for synthetic image generation fallback; modify the
pip install line to add "Pillow" so users know to install it when the VBench
download fails and the script falls back to Pillow-based image creation.</details> </blockquote></details> <details> <summary>benchmarks/benchmark_comfyui_serving.py (1)</summary><blockquote> `389-418`: **Unreachable sleep statement at line 416.** The `await asyncio.sleep(poll_interval_s)` at line 416 can never be reached — all preceding code paths either `continue` (with their own sleep) or `return`. Consider removing it for clarity. ```diff benchmark = history_item.get("benchmark", {}) return ( benchmark.get("execution_ms"), benchmark.get("nodes"), ) - - await asyncio.sleep(poll_interval_s) raise TimeoutError(f"timed out waiting for prompt_id={prompt_id}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmarks/benchmark_comfyui_serving.py` around lines 389 - 418, The final await asyncio.sleep(poll_interval_s) after the async with block is unreachable because every path inside the with either continues (after sleeping) or returns; remove that redundant sleep to simplify the loop. Locate the loop that uses session.get(history_url, timeout=timeout_s) and references prompt_id, history_item, status, and benchmark, then delete the solitary await asyncio.sleep(poll_interval_s) that follows the async with so control flows correctly to the next iteration via the existing sleeps or return paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@benchmarks/benchmark_comfyui_serving.py`:
- Around line 244-285: Update the function docstring to reflect that prompt JSON
files are always regenerated (not skipped) by the implementation: change the
line that currently says prompts are "skipped if prompt files already exist" to
state that prompt files are unconditionally (re)generated for each input image;
reference the behavior implemented with generate_prompt_file being called in the
loop that writes to output_dir and the generated list, and mention that this
forced regeneration occurs regardless of existing files.
---
Nitpick comments:
In `@benchmarks/benchmark_comfyui_serving.py`:
- Around line 389-418: The final await asyncio.sleep(poll_interval_s) after the
async with block is unreachable because every path inside the with either
continues (after sleeping) or returns; remove that redundant sleep to simplify
the loop. Locate the loop that uses session.get(history_url, timeout=timeout_s)
and references prompt_id, history_item, status, and benchmark, then delete the
solitary await asyncio.sleep(poll_interval_s) that follows the async with so
control flows correctly to the next iteration via the existing sleeps or return
paths.
In `@benchmarks/README.md`:
- Around line 6-10: Update the Dependencies section in the README to include the
optional Pillow package used by benchmark_comfyui_serving.py for synthetic image
generation fallback; modify the pip install line to add "Pillow" so users know
to install it when the VBench download fails and the script falls back to
Pillow-based image creation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 13cc4200-c265-4f2b-9b75-6254b7369278
📒 Files selected for processing (6)
benchmarks/README.mdbenchmarks/benchmark_comfyui_serving.pybenchmarks/workflows/wan22_i2v.jsoncomfy/cli_args.pyexecution.pymain.py
| Full benchmark setup for a given *model*/*task*: | ||
|
|
||
| 1. Optionally download model weights into *comfyui_base_dir*. | ||
| 2. Prepare input images in *input_dir* (skipped if images already exist). | ||
| 3. Generate one prompt JSON per input image in *output_dir* | ||
| (skipped if prompt files already exist). | ||
|
|
||
| Returns the list of prompt file paths. | ||
| """ | ||
| key = (model, task) | ||
| if key not in _MODEL_REGISTRY: | ||
| available = ", ".join(f"{m}/{t}" for m, t in _MODEL_REGISTRY) | ||
| raise ValueError(f"Unknown --model {model!r} --task {task!r}. Available: {available}") | ||
|
|
||
| cfg = _MODEL_REGISTRY[key] | ||
|
|
||
| if download_model_weights: | ||
| if comfyui_base_dir is None: | ||
| raise ValueError("--comfyui-base-dir is required when --download-models is set") | ||
| download_models(comfyui_base_dir, model, task) | ||
|
|
||
| image_filenames = prepare_input_images( | ||
| input_dir, | ||
| num_images=num_images, | ||
| image_source=cfg.get("image_source", "synthetic"), | ||
| ) | ||
| if not image_filenames: | ||
| raise RuntimeError(f"No input images available in {input_dir}") | ||
|
|
||
| workflow_path = _WORKFLOWS_DIR / cfg["workflow_file"] | ||
| if not workflow_path.exists(): | ||
| raise FileNotFoundError(f"Workflow file not found: {workflow_path}") | ||
|
|
||
| output_dir.mkdir(parents=True, exist_ok=True) | ||
| generated: list[Path] = [] | ||
| for i, image_name in enumerate(image_filenames): | ||
| prompt_path = output_dir / f"{model}_{task}_prompt_{i:04d}.json" | ||
| generate_prompt_file(prompt_path, workflow_path, image_name) | ||
| generated.append(prompt_path) | ||
|
|
||
| print(f"[setup] generated {len(generated)} prompt files in {output_dir}") | ||
| return generated |
There was a problem hiding this comment.
Docstring contradicts implementation: prompts are always regenerated.
The docstring states prompts are "skipped if prompt files already exist," but the implementation at lines 278-282 unconditionally regenerates all prompt files on every run. Based on the PR summary mentioning "forced prompt regeneration on each run," this appears intentional — please update the docstring to match.
"""
Full benchmark setup for a given *model*/*task*:
1. Optionally download model weights into *comfyui_base_dir*.
2. Prepare input images in *input_dir* (skipped if images already exist).
- 3. Generate one prompt JSON per input image in *output_dir*
- (skipped if prompt files already exist).
+ 3. Generate one prompt JSON per input image in *output_dir*.
Returns the list of prompt file paths.
"""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@benchmarks/benchmark_comfyui_serving.py` around lines 244 - 285, Update the
function docstring to reflect that prompt JSON files are always regenerated (not
skipped) by the implementation: change the line that currently says prompts are
"skipped if prompt files already exist" to state that prompt files are
unconditionally (re)generated for each input image; reference the behavior
implemented with generate_prompt_file being called in the loop that writes to
output_dir and the generated list, and mention that this forced regeneration
occurs regardless of existing files.
Measures latency and throughput of a running ComfyUI server by submitting concurrent prompt requests and collecting results from the history API.