Skip to content

Conversation

aarnphm
Copy link
Collaborator

@aarnphm aarnphm commented Sep 29, 2025

This PR removes the vllm/worker directory and migrate relevant class to vllm/v1

Some change behaviours include:

  • Remove the support for loading dynamic class from cloudpickle in init_worker (cc @russellb)
  • Move all worker initialisation to v1's gpu workers

I haven't touched the ray executor part yet.

Signed-off-by: Aaron Pham contact@aarnphm.xyz

Signed-off-by: Aaron Pham <contact@aarnphm.xyz>
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 effectively removes the deprecated vllm.worker directory and migrates its functionality to vllm.v1, successfully deprecating the V0 worker implementation. The changes are clean and well-executed. Key improvements include:

  • Consolidating worker base classes into vllm/v1/worker/worker_base.py.
  • Removing the insecure and deprecated usage of cloudpickle for dynamic worker class loading, replacing it with a clear error for unsupported configurations.
  • Simplifying the worker selection logic in platform-specific files (cuda.py, rocm.py) to always use the V1 worker.
  • Updating all relevant imports across the codebase to point to the new vllm.v1.worker path.

The refactoring makes the codebase cleaner, more secure, and easier to maintain by removing the old V0 logic. I have reviewed the changes and found no issues. The implementation is solid.

Signed-off-by: Aaron Pham <contact@aarnphm.xyz>
@aarnphm aarnphm added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 29, 2025
@aarnphm
Copy link
Collaborator Author

aarnphm commented Sep 29, 2025

Will wait to trigger CI to make sure there aren't any failures

@aarnphm aarnphm enabled auto-merge (squash) September 29, 2025 22:52
@aarnphm aarnphm merged commit 6a113d9 into vllm-project:main Sep 29, 2025
47 checks passed
Comment on lines +115 to +119
def execute_model(
self,
execute_model_req: Optional[ExecuteModelRequest] = None
) -> Optional[list[SamplerOutput]]:
raise NotImplementedError
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI, this method is also outdated. V1 doesn't use ExecuteModelRequest at all.

@hmellor hmellor moved this to Done in V0 Deprecation Sep 30, 2025
yewentao256 pushed a commit that referenced this pull request Oct 3, 2025
…25901)

Signed-off-by: yewentao256 <zhyanwentao@126.com>
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 rocm Related to AMD ROCm v1
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants