Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 21, 2025

This PR removes graph_pool as a member variable of VllmBackend and as an argument to CUDAGraphWrapper and its base class AbstractStaticGraphWrapper. Instead, CUDAGraphWrapper now always obtains the graph pool directly from the platform using current_platform.get_global_graph_pool().

Changes Made

1. AbstractStaticGraphWrapper Protocol

  • Removed graph_pool: Any parameter from __init__ method signature
  • Updated docstring to remove graph_pool documentation

2. CUDAGraphWrapper

  • Removed graph_pool: Any = None parameter from constructor
  • Simplified initialization to always get graph pool from platform:
    self.graph_pool = current_platform.get_global_graph_pool()
  • Removed conditional logic that previously checked if graph_pool was None

3. VllmBackend

  • Removed self.graph_pool member variable from __init__
  • Removed graph_pool: Any from class type annotations
  • Updated PiecewiseCompileInterpreter to not take or use graph_pool parameter
  • Updated static graph wrapper instantiation to not pass graph_pool argument

Rationale

This change simplifies the architecture by removing an unnecessary parameter that was being passed around when the graph pool can be obtained directly from the platform. The existing fallback logic in CUDAGraphWrapper already called current_platform.get_global_graph_pool() when graph_pool=None, so this change makes that the default behavior while eliminating the parameter passing overhead.

Backward Compatibility

This is a breaking change for any external code that:

  • Instantiates CUDAGraphWrapper with the graph_pool argument
  • Accesses the graph_pool member of VllmBackend
  • Implements AbstractStaticGraphWrapper with the old signature

However, all internal usage has been updated and existing tests continue to work without modification.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@Copilot Copilot AI changed the title [WIP] Remove graph_pool as a member of VllmBackend and as an argument to CUDAGraphWrapper (as well as base class. CUDAGraphWrapper should just obtain the graph pool from the platform directly. Remove graph_pool as member of VllmBackend and argument to CUDAGraphWrapper Aug 21, 2025
@Copilot Copilot AI requested a review from ProExpertProg August 21, 2025 22:33
Copilot finished work on behalf of ProExpertProg August 21, 2025 22:33
@ProExpertProg ProExpertProg marked this pull request as ready for review August 21, 2025 22:55
@zou3519 zou3519 added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 25, 2025
@ProExpertProg ProExpertProg enabled auto-merge (squash) August 25, 2025 16:30
@jikunshang jikunshang disabled auto-merge August 26, 2025 02:17
@jikunshang jikunshang enabled auto-merge (squash) August 26, 2025 02:17
@vllm-bot vllm-bot merged commit 6fad29b into main Aug 26, 2025
48 of 50 checks passed
@vllm-bot vllm-bot deleted the copilot/fix-f1ddac43-4315-4814-a414-33427a5334bc branch August 26, 2025 02:34
tc-mb pushed a commit to tc-mb/vllm that referenced this pull request Aug 27, 2025
…rapper (vllm-project#23385)

Signed-off-by: Luka Govedič <[email protected]>
Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: ProExpertProg <[email protected]>
Co-authored-by: Luka Govedič <[email protected]>
Signed-off-by: tc-mb <[email protected]>
epwalsh pushed a commit to epwalsh/vllm that referenced this pull request Aug 28, 2025
…rapper (vllm-project#23385)

Signed-off-by: Luka Govedič <[email protected]>
Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: ProExpertProg <[email protected]>
Co-authored-by: Luka Govedič <[email protected]>
xiao-llm pushed a commit to xiao-llm/vllm that referenced this pull request Aug 28, 2025
…rapper (vllm-project#23385)

Signed-off-by: Luka Govedič <[email protected]>
Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: ProExpertProg <[email protected]>
Co-authored-by: Luka Govedič <[email protected]>
Signed-off-by: Xiao Yu <[email protected]>
zhewenl pushed a commit to zhewenl/vllm that referenced this pull request Aug 28, 2025
…rapper (vllm-project#23385)

Signed-off-by: Luka Govedič <[email protected]>
Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: ProExpertProg <[email protected]>
Co-authored-by: Luka Govedič <[email protected]>
zhewenl pushed a commit to zhewenl/vllm that referenced this pull request Sep 3, 2025
…rapper (vllm-project#23385)

Signed-off-by: Luka Govedič <[email protected]>
Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: ProExpertProg <[email protected]>
Co-authored-by: Luka Govedič <[email protected]>
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
…rapper (vllm-project#23385)

Signed-off-by: Luka Govedič <[email protected]>
Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: ProExpertProg <[email protected]>
Co-authored-by: Luka Govedič <[email protected]>
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.

5 participants