[https://nvbugs/6248837][fix] Densify trtllm-gen fmha warmup grid to catch missing kernels - #15305
Conversation
|
/bot run --stage-list "DGX_B200-8_GPUs-PyTorch-PerfSanity-Post-Merge-3" |
|
PR_Github #53861 [ run ] triggered by Bot. Commit: |
|
PR_Github #53861 [ run ] completed with state |
|
/bot run --stage-list "PerfSanity" |
|
PR_Github #53898 [ run ] triggered by Bot. Commit: |
|
PR_Github #53898 [ run ] completed with state
|
|
/bot run --stage-list "PerfSanity" --disable-fail-fast |
|
PR_Github #54012 [ run ] triggered by Bot. Commit: |
|
PR_Github #54012 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #54138 [ run ] triggered by Bot. Commit: |
|
PR_Github #54138 [ run ] completed with state
|
7d2a9d3 to
34fa064
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #54319 [ run ] triggered by Bot. Commit: |
|
/bot run --disable-fail-fast |
|
PR_Github #54433 [ run ] triggered by Bot. Commit: |
|
PR_Github #54319 [ run ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #54452 [ run ] triggered by Bot. Commit: |
|
PR_Github #54433 [ run ] completed with state |
|
PR_Github #54452 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #54467 [ run ] triggered by Bot. Commit: |
📝 WalkthroughWalkthroughIn ChangesJIT Warmup Grid Candidate Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.h (1)
304-326: 💤 Low valueWell-designed warmup grid candidates with clear rationale.
The new constant vectors provide good coverage based on autotuner sensitivity analysis. The comments clearly explain the selection strategy (dense coverage for tile counts 1-24, sparse coverage beyond).
Minor readability enhancement
Consider using digit separators for the larger literals to improve readability:
- 10752, 11264, 11776, 12288, 13312, 14336, 16384, 20480, 24576, 32768, 40960, 49152, 65536, 98304, 131072}; + 10'752, 11'264, 11'776, 12'288, 13'312, 14'336, 16'384, 20'480, 24'576, 32'768, 40'960, 49'152, 65'536, 98'304, 131'072};🤖 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 `@cpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.h` around lines 304 - 326, Add digit separators to improve readability of large numeric literals in the warmup candidate vectors. Use C++ digit separator syntax (single quotes) to separate groups of three digits in numbers within kDefaultWarmupBatchSizeCandidates, kDefaultWarmupSeqLenKvCandidates, and kDefaultWarmupSeqLenQCandidates constants. For example, replace numbers like 98304 with 98'304 and 131072 with 131'072 to make the large values easier to read at a glance.
🤖 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 `@cpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.h`:
- Around line 304-326: Add digit separators to improve readability of large
numeric literals in the warmup candidate vectors. Use C++ digit separator syntax
(single quotes) to separate groups of three digits in numbers within
kDefaultWarmupBatchSizeCandidates, kDefaultWarmupSeqLenKvCandidates, and
kDefaultWarmupSeqLenQCandidates constants. For example, replace numbers like
98304 with 98'304 and 131072 with 131'072 to make the large values easier to
read at a glance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6e8a4636-c199-4177-b196-ace462903d9c
📒 Files selected for processing (1)
cpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.h
|
PR_Github #54521 [ run ] triggered by Bot. Commit: |
|
PR_Github #54467 [ run ] completed with state |
Signed-off-by: Pengbo Wang <221450789+pengbowang-nv@users.noreply.github.com>
34fa064 to
a2a9c60
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #54567 [ run ] triggered by Bot. Commit: |
|
PR_Github #54521 [ run ] completed with state |
|
PR_Github #54567 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #54719 [ run ] triggered by Bot. Commit: |
|
PR_Github #54719 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #54747 [ run ] triggered by Bot. Commit: |
|
PR_Github #54747 [ run ] completed with state |
…ngine configs PR NVIDIA#14851 added a TRTLLM-Gen FMHA JIT warmup that enumerates a cartesian grid of (batchSize, seqLenQ, seqLenKv) sized by the engine maxima, eagerly compiling NVRTC kernels for each combo. PR NVIDIA#15305 then densified the candidate lists to catch missing kernels. For long-context disagg configs such as disagg_config_ctxtp2_gentp2_llama31_8b_ucx.yaml (max_num_requests=2048, max_seq_len=131072), the densified grid contains thousands of points and the NVRTC compilation time exceeds the 600s wait_for_disagg_server_ready timeout in test_disaggregated_logprobs_serving[llama-3.1-8b-instruct]. Skip the warmup at the Python entry point when the engine maxima product would produce a problematic grid. The runtime kernel selection path is unchanged -- any kernel that would have been warmed up will JIT-compile lazily on first request instead. This restores the documented pre-PR NVIDIA#14851 behavior for oversized configs without affecting the warmup benefit for regular configs. Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
…catch missing kernels (NVIDIA#15305) Signed-off-by: Pengbo Wang <221450789+pengbowang-nv@users.noreply.github.com> Signed-off-by: GitLab CI Bot <gitlab-ci@nvidia.com>
…catch missing kernels (NVIDIA#15305) Signed-off-by: Pengbo Wang <221450789+pengbowang-nv@users.noreply.github.com> Signed-off-by: GitLab CI Bot <gitlab-ci@nvidia.com>
…ngine configs PR NVIDIA#14851 added a TRTLLM-Gen FMHA JIT warmup that enumerates a cartesian grid of (batchSize, seqLenQ, seqLenKv) sized by the engine maxima, eagerly compiling NVRTC kernels for each combo. PR NVIDIA#15305 then densified the candidate lists to catch missing kernels. For long-context disagg configs such as disagg_config_ctxtp2_gentp2_llama31_8b_ucx.yaml (max_num_requests=2048, max_seq_len=131072), the densified grid contains thousands of points and the NVRTC compilation time exceeds the 600s wait_for_disagg_server_ready timeout in test_disaggregated_logprobs_serving[llama-3.1-8b-instruct]. Skip the warmup at the Python entry point when the engine maxima product would produce a problematic grid. The runtime kernel selection path is unchanged -- any kernel that would have been warmed up will JIT-compile lazily on first request instead. This restores the documented pre-PR NVIDIA#14851 behavior for oversized configs without affecting the warmup benefit for regular configs. Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
…ngine configs PR NVIDIA#14851 added a TRTLLM-Gen FMHA JIT warmup that enumerates a cartesian grid of (batchSize, seqLenQ, seqLenKv) sized by the engine maxima, eagerly compiling NVRTC kernels for each combo. PR NVIDIA#15305 then densified the candidate lists to catch missing kernels. For long-context disagg configs such as disagg_config_ctxtp2_gentp2_llama31_8b_ucx.yaml (max_num_requests=2048, max_seq_len=131072), the densified grid contains thousands of points and the NVRTC compilation time exceeds the 600s wait_for_disagg_server_ready timeout in test_disaggregated_logprobs_serving[llama-3.1-8b-instruct]. Skip the warmup at the Python entry point when the engine maxima product would produce a problematic grid. The runtime kernel selection path is unchanged -- any kernel that would have been warmed up will JIT-compile lazily on first request instead. This restores the documented pre-PR NVIDIA#14851 behavior for oversized configs without affecting the warmup benefit for regular configs. Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
…ngine configs PR NVIDIA#14851 added a TRTLLM-Gen FMHA JIT warmup that enumerates a cartesian grid of (batchSize, seqLenQ, seqLenKv) sized by the engine maxima, eagerly compiling NVRTC kernels for each combo. PR NVIDIA#15305 then densified the candidate lists to catch missing kernels. For long-context disagg configs such as disagg_config_ctxtp2_gentp2_llama31_8b_ucx.yaml (max_num_requests=2048, max_seq_len=131072), the densified grid contains thousands of points and the NVRTC compilation time exceeds the 600s wait_for_disagg_server_ready timeout in test_disaggregated_logprobs_serving[llama-3.1-8b-instruct]. Skip the warmup at the Python entry point when the engine maxima product would produce a problematic grid. The runtime kernel selection path is unchanged -- any kernel that would have been warmed up will JIT-compile lazily on first request instead. This restores the documented pre-PR NVIDIA#14851 behavior for oversized configs without affecting the warmup benefit for regular configs. Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
…ngine configs PR NVIDIA#14851 added a TRTLLM-Gen FMHA JIT warmup that enumerates a cartesian grid of (batchSize, seqLenQ, seqLenKv) sized by the engine maxima, eagerly compiling NVRTC kernels for each combo. PR NVIDIA#15305 then densified the candidate lists to catch missing kernels. For long-context disagg configs such as disagg_config_ctxtp2_gentp2_llama31_8b_ucx.yaml (max_num_requests=2048, max_seq_len=131072), the densified grid contains thousands of points and the NVRTC compilation time exceeds the 600s wait_for_disagg_server_ready timeout in test_disaggregated_logprobs_serving[llama-3.1-8b-instruct]. Skip the warmup at the Python entry point when the engine maxima product would produce a problematic grid. The runtime kernel selection path is unchanged -- any kernel that would have been warmed up will JIT-compile lazily on first request instead. This restores the documented pre-PR NVIDIA#14851 behavior for oversized configs without affecting the warmup benefit for regular configs. Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
…ngine configs PR NVIDIA#14851 added a TRTLLM-Gen FMHA JIT warmup that enumerates a cartesian grid of (batchSize, seqLenQ, seqLenKv) sized by the engine maxima, eagerly compiling NVRTC kernels for each combo. PR NVIDIA#15305 then densified the candidate lists to catch missing kernels. For long-context disagg configs such as disagg_config_ctxtp2_gentp2_llama31_8b_ucx.yaml (max_num_requests=2048, max_seq_len=131072), the densified grid contains thousands of points and the NVRTC compilation time exceeds the 600s wait_for_disagg_server_ready timeout in test_disaggregated_logprobs_serving[llama-3.1-8b-instruct]. Skip the warmup at the Python entry point when the engine maxima product would produce a problematic grid. The runtime kernel selection path is unchanged -- any kernel that would have been warmed up will JIT-compile lazily on first request instead. This restores the documented pre-PR NVIDIA#14851 behavior for oversized configs without affecting the warmup benefit for regular configs. Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
…ngine configs PR NVIDIA#14851 added a TRTLLM-Gen FMHA JIT warmup that enumerates a cartesian grid of (batchSize, seqLenQ, seqLenKv) sized by the engine maxima, eagerly compiling NVRTC kernels for each combo. PR NVIDIA#15305 then densified the candidate lists to catch missing kernels. For long-context disagg configs such as disagg_config_ctxtp2_gentp2_llama31_8b_ucx.yaml (max_num_requests=2048, max_seq_len=131072), the densified grid contains thousands of points and the NVRTC compilation time exceeds the 600s wait_for_disagg_server_ready timeout in test_disaggregated_logprobs_serving[llama-3.1-8b-instruct]. Skip the warmup at the Python entry point when the engine maxima product would produce a problematic grid. The runtime kernel selection path is unchanged -- any kernel that would have been warmed up will JIT-compile lazily on first request instead. This restores the documented pre-PR NVIDIA#14851 behavior for oversized configs without affecting the warmup benefit for regular configs. Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
…ngine configs PR NVIDIA#14851 added a TRTLLM-Gen FMHA JIT warmup that enumerates a cartesian grid of (batchSize, seqLenQ, seqLenKv) sized by the engine maxima, eagerly compiling NVRTC kernels for each combo. PR NVIDIA#15305 then densified the candidate lists to catch missing kernels. For long-context disagg configs such as disagg_config_ctxtp2_gentp2_llama31_8b_ucx.yaml (max_num_requests=2048, max_seq_len=131072), the densified grid contains thousands of points and the NVRTC compilation time exceeds the 600s wait_for_disagg_server_ready timeout in test_disaggregated_logprobs_serving[llama-3.1-8b-instruct]. Skip the warmup at the Python entry point when the engine maxima product would produce a problematic grid. The runtime kernel selection path is unchanged -- any kernel that would have been warmed up will JIT-compile lazily on first request instead. This restores the documented pre-PR NVIDIA#14851 behavior for oversized configs without affecting the warmup benefit for regular configs. Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
Description
Background and Analysis
After previous warmup modification PR at #14851 , most JIT in the middle of execution issue is resolved. However we met a problem on some particular DSR1 case, where a kernel is not covered in the warmup grid and cause performance issue afterwards. In this special case the kernel would be selected only if seqlen between a very narrow area (8193-9316).
This PR densified the grid based on learning from autotuner, and should protect this case and other possible narrow cases without introducing much overhead.
Why the soultion in this PR should work (even for future cases)
Given other parameters, the autotuner would select kernel based on
batchSize,seqLenQ,seqLenKv. For decode case,seqLenQis given. As attention is done in tiles, these parameters only affect autotuner by affecting number of total tile and tile per sequence.When tile number is in the range of 1 - 24, the autotuner logic would be rather delicate so that we require dense cover of this area. Outside this area the autotuner logic will be much less delicate and we can just cover some of them.
Each tile can be of size 128, 256 and 512. The tile number per sequence is capped at
multiprocessorCountand the total tile number also mostly work by comparing withmultiprocessorCountmultiplied by a factor.So for
seqLenKvwe use such range:[*dense_tile_num_range, *sprase_tile_num_range] multiplied by [128, 256, 512].dense_tile_num_range = [1,2, ..., 24]covers full 1-24 tile num range, andsparese_tile_num_range = [26, 28, 32, 40, 48, 64, 96, 128, 192, 256]would loosely cover from 24 - 256 (upper cap of multiprocessor count)For batchsize we want total number of tiles cover 1 - 24 and loosely cover other cases. Remember we have full 1-24 tile for
seqLenKvrange, we would like to cover most cases for eachseqLenKv(effectively tile number per sequence). FortileNumPerSeq = 1, we wantbatchsize = 1, ..., 24; fortileNumPerSeq = 2, we wantbatchsize = 1, ..., 12, etc. So the final coverage would be similar to tile range ofseqLenKv, but I added more batchsizes just to be sure about real cases.Overhead
As the warmup happens on C++ fmhaRunner level, the overhead would be much easier to handle.
Overhead = Tuning Overhead + Kernel Compilation Overhead. For a given model config, as the warmup should catch every posssible kernel in the run, this can be seen as a constant. Before and after this PR this should not change much (it will increase as we did catch a previously missing kernel), likely an increase of 8s. And theTuning Overhead = mesh point * tuning time * model layer number * number of execution, tuning time is typically less than 20us, interpreting to less than 6s for most models. In actual test environment the tuning overhead is measured less than 1s. I believe this is due to the fact we are overestimating the tuning time and mesh point.Follow up tasks
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.Summary by CodeRabbit