Skip to content

[Chore] Cleanup mem_utils.py#31793

Merged
DarkLight1337 merged 3 commits intovllm-project:mainfrom
DarkLight1337:cleanup-mem
Jan 6, 2026
Merged

[Chore] Cleanup mem_utils.py#31793
DarkLight1337 merged 3 commits intovllm-project:mainfrom
DarkLight1337:cleanup-mem

Conversation

@DarkLight1337
Copy link
Member

@DarkLight1337 DarkLight1337 commented Jan 6, 2026

Purpose

  • Ensure MemoryProfilingResult snapshots all have the same device
  • Remove unnecessary noqas

Some of the changes are taken from #22070

Test Plan

Test Result


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.

Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
@DarkLight1337 DarkLight1337 added the ready ONLY add when PR is ready to merge/full CI is needed label Jan 6, 2026
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 aims to clean up mem_utils.py by ensuring memory snapshots use a consistent device and removing unnecessary noqa comments. While the intention to enforce device consistency is good, the implementation is flawed. The MemorySnapshot class stores the actual device in a private device_ attribute, but the changes incorrectly attempt to use the public device attribute, which can be None. This could lead to incorrect device selection in a multi-device environment. I've provided two high-severity comments with suggestions to use the device_ attribute to fix this correctness issue. The other changes, such as refactoring and removing noqa comments, are good improvements.

Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
@DarkLight1337 DarkLight1337 merged commit 14df02b into vllm-project:main Jan 6, 2026
44 checks passed
@DarkLight1337 DarkLight1337 deleted the cleanup-mem branch January 6, 2026 11:56
LucasWilkinson pushed a commit to neuralmagic/vllm that referenced this pull request Jan 6, 2026
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
yugong333 pushed a commit to yugong333/vllm that referenced this pull request Jan 9, 2026
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
akh64bit pushed a commit to akh64bit/vllm that referenced this pull request Jan 16, 2026
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
dsuhinin pushed a commit to dsuhinin/vllm that referenced this pull request Jan 21, 2026
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: dsuhinin <suhinin.dmitriy@gmail.com>
ItzDEXX pushed a commit to ItzDEXX/vllm that referenced this pull request Feb 19, 2026
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants