[https://nvbugs/6523785][fix] Added Backend.get_graph_pool_handle() / retire_graph_pool_handle() and… - #16968
[https://nvbugs/6523785][fix] Added Backend.get_graph_pool_handle() / retire_graph_pool_handle() and…#16968trtllm-agent wants to merge 1 commit into
Backend.get_graph_pool_handle() / retire_graph_pool_handle() and…#16968Conversation
…eleased Backend caches one private CUDA graph pool handle per process and hands it to every CUDAGraphRunner. When the graphs captured into that pool are reset (KV cache estimation shutdown, executor shutdown), the pool's use_count drops to zero, but the handle stayed cached on the class. A second LLM in the same worker process then reused the retired handle, and the first torch.cuda.graph(pool=...) tripped the CUDACachingAllocator assertion 'it->second->use_count > 0'. Retire the handle whenever the graphs are released and recreate it lazily, mirroring the handle rotation clear_piecewise_cuda_graphs() already does for PiecewiseRunner pools. Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
WalkthroughThe change makes CUDA graph pool handle creation lazy at the backend level and adds explicit retirement. ChangesCUDA graph pool lifecycle
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
#16952 Fixed the issue. |
5523ee2 to
4b90dc1
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tensorrt_llm/_torch/pyexecutor/model_engine.py (1)
2750-2754: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftAdd a regression test for pool rotation.
Exercise first-engine capture →
_release_cuda_graphs()→ second-engine capture in the same MPI worker, verifying that the second capture uses a fresh pool and does not hit the allocator assertion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tensorrt_llm/_torch/pyexecutor/model_engine.py` around lines 2750 - 2754, Add a regression test covering two engines in the same MPI worker: capture CUDA graphs with the first engine, call its _release_cuda_graphs() method, then capture with the second engine. Assert that the second capture uses a different/fresh graph memory pool and completes without the allocator assertion, using the existing engine and CUDA-graph test helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tensorrt_llm/_torch/pyexecutor/model_engine.py`:
- Around line 2750-2754: Add a regression test covering two engines in the same
MPI worker: capture CUDA graphs with the first engine, call its
_release_cuda_graphs() method, then capture with the second engine. Assert that
the second capture uses a different/fresh graph memory pool and completes
without the allocator assertion, using the existing engine and CUDA-graph test
helpers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7ec5762d-16e0-41d1-826b-9475ba998939
📒 Files selected for processing (2)
tensorrt_llm/_torch/compilation/backend.pytensorrt_llm/_torch/pyexecutor/model_engine.py
Summary
Backend._graph_pool_handleis a process-wide class attribute that was kept after_release_cuda_graphs()reset every graph captured into that private pool, so a secondLLMin the same reused MPI worker got a retired handle whoseuse_countwas already 0, trippingCUDACachingAllocator'sensureExistsAndIncrefPoolassert.Backend.get_graph_pool_handle()/retire_graph_pool_handle()and retire the shared handle (plus the engine's cached copy) in_release_cuda_graphs(), so the next capture lazily allocates a fresh pool.Test plan
Links
Dev Engineer Review
Backendto manage the process-wide CUDA graph pool handle via two new lifecycle APIs:Backend.get_graph_pool_handle()for lazy creation andBackend.retire_graph_pool_handle()for clearing the cached handle after graph teardown.Backend.optimize()’spiecewise_cuda_graphpath to passself.get_graph_pool_handle()intopiecewise_optimizer(instead of using a pre-initialized cached field).PyTorchModelEngine.__init__to initialize_cuda_graph_mem_poolfromBackend.get_graph_pool_handle()when torch.compile is enabled, and updatedPyTorchModelEngine._release_cuda_graphs()to retire the shared pool handle (Backend.retire_graph_pool_handle()) and set_cuda_graph_mem_pooltoNoneafter clearing runners.QA Engineer Review
No test changes.