Skip to content

[Benchmark] Correctly kill vllm process in performance benchamrk #2782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 7, 2025
Merged

[Benchmark] Correctly kill vllm process in performance benchamrk #2782

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
802163b
fix benchmark
Potabk Sep 5, 2025
aa87f77
fix
Potabk Sep 5, 2025
8f84059
add no-stream param to load multi-modal dataset locally
Potabk Sep 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion benchmarks/scripts/run-performance-benchmarks.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ kill_npu_processes() {
ps -aux
lsof -t -i:8000 | xargs -r kill -9
pgrep python3 | xargs -r kill -9

# vLLM now names the process with VLLM prefix after https://github.com/vllm-project/vllm/pull/21445
pgrep VLLM | xargs -r kill -9
Comment on lines 80 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

Killing all processes named python3 is overly broad and poses a significant risk of terminating unrelated user or system processes, which could lead to data loss or system instability. This is a critical issue for script robustness.

Since vLLM processes are now specifically identified by the VLLM prefix, the generic pgrep python3 command is likely redundant and dangerous. It should be removed to make the cleanup process more targeted and safe.

Suggested change
pgrep python3 | xargs -r kill -9
# vLLM now names the process with VLLM prefix after https://github.com/vllm-project/vllm/pull/21445
pgrep VLLM | xargs -r kill -9
# vLLM now names the process with VLLM prefix after https://github.com/vllm-project/vllm/pull/21445
pgrep VLLM | xargs -r kill -9


sleep 4
rm -rf ~/.config/vllm

Expand Down
Loading