-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[#19527][fix] Account for beam width when sizing CUDA graph warmup KV requests #19568
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1347,6 +1347,25 @@ def add_dummy_requests( | |
| _populate_dummy_mrope_config(req, token_num, is_gen) | ||
| requests.append(req) | ||
|
|
||
| # Beam search allocates most blocks once per beam, so the single-block | ||
| # check above does not guarantee the dummy requests fit. Skip padding | ||
| # instead of failing inside the block manager. VSWA pools are sized | ||
| # per window, which this full-attention count does not model. | ||
| if beam_width > 1 and batch_request_infos and not self.is_vswa: | ||
| num_appended_tokens = self.num_extra_kv_tokens + (_kv_draft | ||
| if is_gen else 0) | ||
| num_required_blocks = sum( | ||
| self._get_num_blocks_for_dummy_request( | ||
| token_num, num_appended_tokens, beam_width) | ||
| for _, token_num, _ in batch_request_infos) | ||
| if num_required_blocks > available_blocks: | ||
| logger.debug( | ||
| f"[add_dummy_requests] {len(batch_request_infos)} dummy " | ||
| f"requests with beam_width={beam_width} need " | ||
| f"{num_required_blocks} blocks, only {available_blocks} " | ||
| f"free; skipping.") | ||
| return None | ||
|
|
||
| try: | ||
| # Use add_sequence_batch for all dummy requests, then add extra tokens. | ||
| # This must happen before is_gen state modifications below, which may | ||
|
|
@@ -1839,18 +1858,62 @@ def get_num_kv_blocks(self, num_tokens: int) -> int: | |
| def get_num_available_tokens(self, | ||
| token_num_upper_bound: int, | ||
| max_num_draft_tokens: int = 0, | ||
| max_beam_width: int = 1, | ||
| **kwargs) -> int: | ||
| """Return a token count such that one sequence of any length up to it | ||
| fits in the free blocks. | ||
|
|
||
| Args: | ||
| token_num_upper_bound: Upper bound on the returned token count. | ||
| max_num_draft_tokens: Draft tokens appended after the sequence. | ||
| max_beam_width: Beam width of the sequence. With beam search, only | ||
| blocks fully covered by the prompt are shared among beams; the | ||
| rest are allocated once per beam. | ||
| """ | ||
| free_blocks = self.get_num_free_blocks() | ||
| result = min( | ||
| token_num_upper_bound, free_blocks * self.tokens_per_block - | ||
| self.num_extra_kv_tokens - max_num_draft_tokens) | ||
| num_appended_tokens = self.num_extra_kv_tokens + max_num_draft_tokens | ||
| if max_beam_width > 1 and self.kv_cache_type != CacheTypeCpp.CROSS: | ||
| # Block usage is not monotonic in the sequence length (a | ||
| # block-aligned prompt shares all of its blocks), so bound it by | ||
| # the worst case: a partially filled last prompt block followed by | ||
| # the appended tokens, all allocated per beam. | ||
| max_blocks_per_beam = math.ceil( | ||
| (self.tokens_per_block - 1 + num_appended_tokens) / | ||
| self.tokens_per_block) | ||
| num_shared_blocks = free_blocks - max_beam_width * max_blocks_per_beam | ||
| capacity = (num_shared_blocks + 1) * self.tokens_per_block - 1 | ||
|
Comment on lines
+1874
to
+1884
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: rg -n "max_num_draft_tokens|_kv_draft|num_draft|draft_tokens|beam_width" tests/unittest/_torch/executor/test_resource_manager.py
sed -n '1050,1155p' tests/unittest/_torch/executor/test_resource_manager.py
sed -n '1335,1375p' tensorrt_llm/_torch/pyexecutor/resource_manager.py
sed -n '1850,1925p' tensorrt_llm/_torch/pyexecutor/resource_manager.pyRepository: NVIDIA/TensorRT-LLM Length of output: 15401 Add coverage for beam search with positive draft-token reservations. The existing beam-search tests use 🤖 Prompt for AI Agents |
||
| else: | ||
|
Comment on lines
+1874
to
+1885
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1850,1925p' tensorrt_llm/_torch/pyexecutor/resource_manager.py
sed -n '3315,3370p' tensorrt_llm/_torch/pyexecutor/model_engine.py
rg -n "get_num_available_tokens\\(" tensorrt_llm/_torch/pyexecutor | head -80Repository: NVIDIA/TensorRT-LLM Length of output: 8487 🏁 Script executed: #!/bin/bash
sed -n '1925,2025p' tensorrt_llm/_torch/pyexecutor/resource_manager.py
sed -n '3290,3425p' tensorrt_llm/_torch/pyexecutor/model_engine.py
rg -n -C 8 "def add_dummy_requests|available_tokens|max_num_tokens|ENC_DEC_CUDA_GRAPH_DUMMY_TOKEN_NUM|num_tokens" tensorrt_llm/_torch/pyexecutor/resource_manager.py tensorrt_llm/_torch/pyexecutor/model_engine.pyRepository: NVIDIA/TensorRT-LLM Length of output: 43669 🏁 Script executed: #!/bin/bash
rg -n "^ def add_dummy_requests|^ def _get_num_blocks_for_dummy_request|blocks_to_use|available_tokens|num_tokens > self.max_num_tokens|exceeds available" tensorrt_llm/_torch/pyexecutor/resource_manager.py tensorrt_llm/_torch/pyexecutor/model_engine.py
sed -n '3000,3210p' tensorrt_llm/_torch/pyexecutor/model_engine.py
sed -n '2570,2705p' tensorrt_llm/_torch/pyexecutor/resource_manager.py
sed -n '2705,2865p' tensorrt_llm/_torch/pyexecutor/resource_manager.pyRepository: NVIDIA/TensorRT-LLM Length of output: 26870 🏁 Script executed: #!/bin/bash
sed -n '1225,1345p' tensorrt_llm/_torch/pyexecutor/resource_manager.py
sed -n '1345,1435p' tensorrt_llm/_torch/pyexecutor/resource_manager.py
rg -n -C 6 "def _allocate_blocks|Not enough blocks|add_tokens\\(" tensorrt_llm/_torch/pyexecutor/resource_manager.pyRepository: NVIDIA/TensorRT-LLM Length of output: 12841 🏁 Script executed: #!/bin/bash
rg -n -C 5 "is_vswa|VSWA|max_beam_width.*vswa|vswa.*beam|max_beam_width" tensorrt_llm/_torch/pyexecutor tensorrt_llm/_torch | head -240
sed -n '1380,1475p' tensorrt_llm/_torch/pyexecutor/resource_manager.pyRepository: NVIDIA/TensorRT-LLM Length of output: 29451 🏁 Script executed: #!/bin/bash
rg -n -C 4 "is_vswa" tensorrt_llm/_torch/pyexecutor/resource_manager.py tensorrt_llm/_torch/pyexecutor/model_engine.py tensorrt_llm/_torch/pyexecutor/cuda_graph_runner.py
for n in 1332 2236 2378 3118 3362; do
start=$((n-8)); end=$((n+14))
echo "=== model_engine.py:${n} ==="
sed -n "${start},${end}p" tensorrt_llm/_torch/pyexecutor/model_engine.py
doneRepository: NVIDIA/TensorRT-LLM Length of output: 13411 🏁 Script executed: #!/bin/bash
rg -n -C 5 "add_sequence_batch" tensorrt_llm cpp tests 2>/dev/null | head -220
rg -n -C 5 "uses_vswa_kv_cache_layout|calculate_max_num_blocks_for_vswa|num_free_blocks_per_window_size" tensorrt_llm cpp tests 2>/dev/null | head -180Repository: NVIDIA/TensorRT-LLM Length of output: 40024 Skip warmup when beam capacity is below one token. For a VSWA manager with a small pool, Return before creating the final dummy request when Suggested fix if draft_kv_cache_manager is not None:
draft_available_tokens = draft_kv_cache_manager.get_num_available_tokens(
batch_size=batch_size,
token_num_upper_bound=max_seq_len,
max_num_draft_tokens=_kv_draft,
max_beam_width=self.max_beam_width)
available_tokens = min(available_tokens, draft_available_tokens)
+ if available_tokens < 1:
+ free_warmup_requests()
+ return None
+
token_num = max(
ENC_DEC_CUDA_GRAPH_DUMMY_TOKEN_NUM if is_enc_dec else 1,
min(🤖 Prompt for AI Agents |
||
| capacity = free_blocks * self.tokens_per_block - num_appended_tokens | ||
| result = min(token_num_upper_bound, capacity) | ||
| logger.debug( | ||
| f"[get_num_available_tokens] free_blocks={free_blocks}, " | ||
| f"tokens_per_block={self.tokens_per_block}, " | ||
| f"num_extra_kv_tokens={self.num_extra_kv_tokens}, " | ||
| f"max_beam_width={max_beam_width}, " | ||
| f"token_num_upper_bound={token_num_upper_bound}, result={result}") | ||
| return result | ||
|
|
||
| def _get_num_blocks_for_dummy_request(self, token_num: int, | ||
| num_appended_tokens: int, | ||
| beam_width: int) -> int: | ||
| """Number of blocks ``add_dummy_requests`` allocates for one sequence | ||
| of ``token_num`` prompt tokens followed by ``num_appended_tokens`` | ||
| tokens added one at a time. | ||
|
|
||
| Blocks fully covered by the prompt are shared among beams (for cross | ||
| KV, the partial last prompt block is shared too); every other block is | ||
| allocated once per beam. | ||
| """ | ||
| num_blocks = math.ceil( | ||
| (token_num + num_appended_tokens) / self.tokens_per_block) | ||
| if beam_width == 1: | ||
| return num_blocks | ||
| if self.kv_cache_type == CacheTypeCpp.CROSS: | ||
| num_shared_blocks = math.ceil(token_num / self.tokens_per_block) | ||
| else: | ||
| num_shared_blocks = token_num // self.tokens_per_block | ||
| return num_shared_blocks + beam_width * (num_blocks - num_shared_blocks) | ||
|
|
||
| def get_buffers(self, | ||
| layer_idx: int, | ||
| kv_layout: str = "NHD") -> Optional[torch.Tensor]: | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1050,6 +1050,101 @@ def test_batch_cache_indices_honor_requested_blocks_for_beam0(self): | |||||||||||||
| finally: | ||||||||||||||
| kv_cache_manager.shutdown() | ||||||||||||||
|
|
||||||||||||||
| @staticmethod | ||||||||||||||
| def _create_beam_search_kv_cache_manager(max_beam_width: int, | ||||||||||||||
| max_batch_size: int = 1): | ||||||||||||||
| # 32 blocks of 8 tokens. | ||||||||||||||
| return KVCacheManager( | ||||||||||||||
| kv_cache_config=KvCacheConfig(max_tokens=256, | ||||||||||||||
| enable_block_reuse=False), | ||||||||||||||
| kv_cache_type=tensorrt_llm.bindings.internal.batch_manager. | ||||||||||||||
| CacheType.SELF, | ||||||||||||||
| num_layers=2, | ||||||||||||||
| num_kv_heads=2, | ||||||||||||||
| head_dim=128, | ||||||||||||||
| tokens_per_block=8, | ||||||||||||||
| max_seq_len=256, | ||||||||||||||
| max_batch_size=max_batch_size, | ||||||||||||||
| max_beam_width=max_beam_width, | ||||||||||||||
| mapping=Mapping(), | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| def test_dummy_request_block_count_matches_beam_search_allocation(self): | ||||||||||||||
| """Prompt-covered blocks are shared among beams; the partial last | ||||||||||||||
| block is allocated once per beam.""" | ||||||||||||||
| beam_width = 4 | ||||||||||||||
| kv_cache_manager = self._create_beam_search_kv_cache_manager(beam_width) | ||||||||||||||
| try: | ||||||||||||||
| total_free = kv_cache_manager.get_num_free_blocks() | ||||||||||||||
| for request_id, token_num in enumerate([1, 8, 9, 100, 128]): | ||||||||||||||
| requests = kv_cache_manager.add_dummy_requests( | ||||||||||||||
| [request_id], [token_num], | ||||||||||||||
| is_gen=True, | ||||||||||||||
| max_beam_width=beam_width) | ||||||||||||||
| self.assertIsNotNone(requests) | ||||||||||||||
| used_blocks = (total_free - | ||||||||||||||
| kv_cache_manager.get_num_free_blocks()) | ||||||||||||||
| self.assertEqual( | ||||||||||||||
| used_blocks, | ||||||||||||||
| kv_cache_manager._get_num_blocks_for_dummy_request( | ||||||||||||||
| token_num, 0, beam_width), f"token_num={token_num}") | ||||||||||||||
| kv_cache_manager.free_resources(requests[0]) | ||||||||||||||
| finally: | ||||||||||||||
| kv_cache_manager.shutdown() | ||||||||||||||
|
|
||||||||||||||
| def test_get_num_available_tokens_accounts_for_beam_width(self): | ||||||||||||||
| """Every length up to the reported capacity must fit with beam | ||||||||||||||
| search, including lengths that are not block aligned.""" | ||||||||||||||
| beam_width = 4 | ||||||||||||||
| kv_cache_manager = self._create_beam_search_kv_cache_manager(beam_width) | ||||||||||||||
| try: | ||||||||||||||
| self.assertEqual(kv_cache_manager.get_num_free_blocks(), 32) | ||||||||||||||
| self.assertEqual( | ||||||||||||||
| kv_cache_manager.get_num_available_tokens( | ||||||||||||||
| token_num_upper_bound=1024), 256) | ||||||||||||||
| capacity = kv_cache_manager.get_num_available_tokens( | ||||||||||||||
| token_num_upper_bound=1024, max_beam_width=beam_width) | ||||||||||||||
| # 32 free blocks minus one per-beam tail block for each beam. | ||||||||||||||
| self.assertEqual(capacity, (32 - beam_width + 1) * 8 - 1) | ||||||||||||||
| for token_num in range(1, capacity + 1): | ||||||||||||||
| requests = kv_cache_manager.add_dummy_requests( | ||||||||||||||
| [0], [token_num], is_gen=True, max_beam_width=beam_width) | ||||||||||||||
| self.assertIsNotNone(requests, f"token_num={token_num}") | ||||||||||||||
| kv_cache_manager.free_resources(requests[0]) | ||||||||||||||
| self.assertEqual(kv_cache_manager.get_num_free_blocks(), 32) | ||||||||||||||
| finally: | ||||||||||||||
| kv_cache_manager.shutdown() | ||||||||||||||
|
|
||||||||||||||
| def test_add_dummy_requests_beam_search_returns_none_when_pool_too_small( | ||||||||||||||
| self): | ||||||||||||||
| """Dummy requests that cannot fit with beam search are skipped | ||||||||||||||
| instead of failing inside the block manager, and nothing leaks.""" | ||||||||||||||
| beam_width = 4 | ||||||||||||||
| kv_cache_manager = self._create_beam_search_kv_cache_manager( | ||||||||||||||
| beam_width, max_batch_size=16) | ||||||||||||||
| try: | ||||||||||||||
| total_free = kv_cache_manager.get_num_free_blocks() | ||||||||||||||
| # 31 shared blocks plus one tail block per beam: 35 > 32. | ||||||||||||||
| self.assertIsNone( | ||||||||||||||
| kv_cache_manager.add_dummy_requests([0], [255], | ||||||||||||||
| is_gen=True, | ||||||||||||||
| max_beam_width=beam_width)) | ||||||||||||||
| # One per-beam block per request: 9 * 4 = 36 > 32. | ||||||||||||||
| self.assertIsNone( | ||||||||||||||
| kv_cache_manager.add_dummy_requests(list(range(9)), | ||||||||||||||
| is_gen=True, | ||||||||||||||
| max_beam_width=beam_width)) | ||||||||||||||
| self.assertEqual(kv_cache_manager.get_num_free_blocks(), total_free) | ||||||||||||||
| # 8 * 4 = 32 blocks fits exactly. | ||||||||||||||
| requests = kv_cache_manager.add_dummy_requests( | ||||||||||||||
| list(range(8)), is_gen=True, max_beam_width=beam_width) | ||||||||||||||
| self.assertIsNotNone(requests) | ||||||||||||||
| self.assertEqual(kv_cache_manager.get_num_free_blocks(), 0) | ||||||||||||||
| for request in requests: | ||||||||||||||
| kv_cache_manager.free_resources(request) | ||||||||||||||
|
Comment on lines
+1143
to
+1144
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Verify cleanup after the exact-fit allocation. The test frees the exact-fit batch but does not verify that all 32 blocks return to the pool. It can pass if Proposed test correction for request in requests:
kv_cache_manager.free_resources(request)
+ self.assertEqual(kv_cache_manager.get_num_free_blocks(),
+ total_free)As per path instructions, flag “faulty setup/teardown” and tests that can pass despite resource leakage. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsSource: Path instructions |
||||||||||||||
| finally: | ||||||||||||||
| kv_cache_manager.shutdown() | ||||||||||||||
|
|
||||||||||||||
| def test_add_dummy_requests_failure_frees_partial_allocation(self): | ||||||||||||||
| """A partial add_dummy_requests failure must free every block it | ||||||||||||||
| allocated (TRTLLM-14903): leaked blocks on the minimal pool built for | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: NVIDIA/TensorRT-LLM
Length of output: 39075
🏁 Script executed:
Repository: NVIDIA/TensorRT-LLM
Length of output: 42022
🤖 get_repo_knowledge executed:
get_repo_knowledge NVIDIA/TensorRT-LLM /tmp/coderabbit-repo-knowledge/nvidia-tensorrt-llm-053b2761/architecture /tmp/coderabbit-repo-knowledge/nvidia-tensorrt-llm-053b2761/learnings /tmp/coderabbit-repo-knowledge/nvidia-tensorrt-llm-053b2761/conventionsLength of output: 47702
🏁 Script executed:
Repository: NVIDIA/TensorRT-LLM
Length of output: 41775
🏁 Script executed:
Repository: NVIDIA/TensorRT-LLM
Length of output: 36294
Add caller-level coverage for both cache managers.
The existing test uses the default beam width and registers only a target
KVCacheManagerV2. It checks request construction, not the arguments passed toget_num_available_tokens. It therefore cannot detect an omitted or misroutedmax_beam_width, and it does not exercise the draft-cache call. Add a focused test intests/unittest/_torch/executor/test_pytorch_model_engine_warmup.pywith distinct target and draft spies,max_beam_width > 1, and assertions for both calls. This prevents warmup from using a capacity calculated without the configured beam width.🤖 Prompt for AI Agents