[https://nvbugs/6185446][fix] Add warmup for trtllm-gen fmha JIT kernels - #14851
Conversation
📝 WalkthroughWalkthroughThis PR introduces an optional JIT warmup mechanism for TensorRT-LLM Generation FMHA kernels to reduce first-dispatch latency. It adds warmup configuration fields across parameter structs, removes ChangesTRTLLM-Gen FMHA JIT Warmup Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tensorrt_llm/_torch/pyexecutor/model_engine.py (1)
84-91:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix
forwardsignature mismatch (positional arg binding) betweenModelEngineandPyTorchModelEngine.
ModelEngine.forwardplacestrtllm_gen_fmha_jit_warmupimmediately afternum_accepted_tokens_device, butPyTorchModelEngine.forwardinsertsreq_id_to_old_requestbefore it—so positional callers following the abstract interface will bind the warmup flag toreq_id_to_old_requestand leavetrtllm_gen_fmha_jit_warmupat its default.🤖 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 84 - 91, The abstract/parent signature and subclass signature are misaligned: ModelEngine.forward currently places trtllm_gen_fmha_jit_warmup immediately after num_accepted_tokens_device while PyTorchModelEngine.forward inserts req_id_to_old_request before the warmup flag, causing incorrect positional binding; fix by adding the req_id_to_old_request parameter (matching PyTorchModelEngine) into ModelEngine.forward between num_accepted_tokens_device and trtllm_gen_fmha_jit_warmup (with the same Optional type/default), and update the type hints/docstring for ModelEngine.forward to match PyTorchModelEngine.forward so both signatures are identical.
🤖 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.
Inline comments:
In `@cpp/tensorrt_llm/common/attentionOp.cpp`:
- Around line 297-300: When trtllmGenFmhaJITWarmup is enabled, reject negative
warmup limits before forwarding: change the assignments around
xqaParams.trtllmGenFmhaJITWarmupMaxNumRequests,
xqaParams.trtllmGenFmhaJITWarmupMaxSeqLenQ and
xqaParams.trtllmGenFmhaJITWarmupMaxSeqLenKv so they are only copied from
generationsParams if generationsParams.trtllmGenFmhaJITWarmup is true AND the
corresponding generationsParams.trtllmGenFmhaJITWarmupMax* value > 0; otherwise
set the xqaParams field to 0 (or another safe non-negative sentinel). Apply the
same guard pattern wherever these fields are set (including the other
occurrences around the blocks referenced: the assignments at the other
occurrences for the same symbols).
In `@cpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.h`:
- Around line 370-423: runJITWarmupGridIfRequested currently re-runs the full
warmup grid every call; add memoization to avoid redundant warmups by recording
completed "envelopes" and skipping any grid point already covered by an
equal-or-larger prior warm. Implement a compact key type (e.g., tuple or encoded
int) representing (useGenKernelForPrefill, batchSize, seqLenQ, seqLenKv) as a
member (or shared static) cache in the TllmGenFmhaKernel/ TllmFmhaKernelFactory
scope, check this cache before calling warmupOneKernel, and when a warmup
finishes insert the envelope; ensure comparison treats a prior warmed envelope
with batch>=batchSize and seqQ>=seqLenQ and seqKv>=seqLenKv as covering the
current point and skip warmup accordingly. Use the existing symbols
runJITWarmupGridIfRequested, warmupOneKernel, RunnerParams,
mJITWarmupMaxNumRequests, mJITWarmupMaxSeqLenQ, mJITWarmupMaxSeqLenKv and
mUseGenKernelForPrefill to locate where to add the checks and cache updates.
- Around line 390-391: The runtime check currently uses != 0 which allows
negative warmup limits; update the validation in the TLLM_CHECK_WITH_INFO call
to require strictly positive bounds (e.g., maxBatchSize > 0 && maxSeqLenKv > 0
&& (!useGenKernelForPrefill || maxSeqLenQ > 0)) so negative values are rejected
before they reach makeWarmupCandidateSizes() and warmupOneKernel(); ensure the
error message still references the same context (TRTLLM-Gen Fmha Warmup Param is
invalid) so failures fail fast with correct validation.
In `@cpp/tensorrt_llm/thop/attentionOp.h`:
- Around line 86-87: The API in attentionOp.h allows callers to set
trtllmGenFmhaJITWarmup=true while leaving trtllmGenFmhaJITWarmupMaxSeqLenKv
defaulting to 0, which is rejected by the warmup path; change the function
signature/behavior so enabling warmup cannot pick an invalid default—either give
trtllmGenFmhaJITWarmupMaxSeqLenKv a sensible non-zero default (e.g. derive from
a constant or existing max KV size), remove the default so callers must supply a
valid max when trtllmGenFmhaJITWarmup is true, or add an overload/validation
that auto-populates a valid max when trtllmGenFmhaJITWarmup is true; update
callers and any validation logic that checks trtllmGenFmhaJITWarmupMaxSeqLenKv
accordingly.
In `@tensorrt_llm/_torch/pyexecutor/model_engine.py`:
- Around line 903-930: The TRT FMHA JIT warmup in _run_attention_warmup may
raise torch.OutOfMemoryError and currently can abort initialization; wrap the
forward+torch.cuda.synchronize() call inside a try/except that catches
torch.OutOfMemoryError, logs a warning (including the exception), calls
torch.cuda.empty_cache() to free memory, and then continues to the next warmup
shape rather than re-raising; keep the existing no_cuda_graph() and
_release_batch_context(...) usage and only catch torch.OutOfMemoryError so other
exceptions still surface.
---
Outside diff comments:
In `@tensorrt_llm/_torch/pyexecutor/model_engine.py`:
- Around line 84-91: The abstract/parent signature and subclass signature are
misaligned: ModelEngine.forward currently places trtllm_gen_fmha_jit_warmup
immediately after num_accepted_tokens_device while PyTorchModelEngine.forward
inserts req_id_to_old_request before the warmup flag, causing incorrect
positional binding; fix by adding the req_id_to_old_request parameter (matching
PyTorchModelEngine) into ModelEngine.forward between num_accepted_tokens_device
and trtllm_gen_fmha_jit_warmup (with the same Optional type/default), and update
the type hints/docstring for ModelEngine.forward to match
PyTorchModelEngine.forward so both signatures are identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: da358839-ed5b-47d2-9984-c0f7fea9a4a7
📒 Files selected for processing (13)
cpp/tensorrt_llm/common/attentionOp.cppcpp/tensorrt_llm/common/attentionOp.hcpp/tensorrt_llm/kernels/contextFusedMultiHeadAttention/fused_multihead_attention_common.hcpp/tensorrt_llm/kernels/decoderMaskedMultiheadAttention/xqaParams.hcpp/tensorrt_llm/kernels/fmhaDispatcher.cppcpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.hcpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaRunnerParams.hcpp/tensorrt_llm/kernels/xqaDispatcher.cppcpp/tensorrt_llm/nanobind/thop/bindings.cppcpp/tensorrt_llm/thop/attentionOp.cppcpp/tensorrt_llm/thop/attentionOp.htensorrt_llm/_torch/attention_backend/trtllm.pytensorrt_llm/_torch/pyexecutor/model_engine.py
a8297f6 to
023ec1c
Compare
|
/bot run |
|
PR_Github #51541 [ run ] triggered by Bot. Commit: |
|
/bot run |
|
PR_Github #51541 [ run ] completed with state
|
|
PR_Github #51549 [ run ] triggered by Bot. Commit: |
|
PR_Github #51549 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #51612 [ run ] triggered by Bot. Commit: |
|
PR_Github #51612 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #51750 [ run ] triggered by Bot. Commit: |
|
PR_Github #51750 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #51940 [ run ] triggered by Bot. Commit: |
|
PR_Github #51940 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #51999 [ run ] triggered by Bot. Commit: |
|
PR_Github #51999 [ run ] completed with state |
Signed-off-by: Pengbo Wang <221450789+pengbowang-nv@users.noreply.github.com>
Signed-off-by: Pengbo Wang <221450789+pengbowang-nv@users.noreply.github.com>
Signed-off-by: Pengbo Wang <221450789+pengbowang-nv@users.noreply.github.com>
Signed-off-by: Pengbo Wang <221450789+pengbowang-nv@users.noreply.github.com>
Signed-off-by: Pengbo Wang <221450789+pengbowang-nv@users.noreply.github.com>
Signed-off-by: Pengbo Wang <221450789+pengbowang-nv@users.noreply.github.com>
4ec118b to
1a0b6e0
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #52650 [ run ] triggered by Bot. Commit: |
|
PR_Github #52647 [ run ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #52733 [ run ] triggered by Bot. Commit: |
|
PR_Github #52650 [ run ] completed with state
|
|
PR_Github #52733 [ run ] completed with state |
…c working set PR NVIDIA#14851 added a JIT-warmup cartesian grid sized by engine-config maxima (mMaxNumRequests, mMaxSeqLen). For long-context disagg configs such as disagg_config_ctxtp2_gentp2_llama31_8b_ucx.yaml (max_num_requests=2048, max_seq_len=131072), this enumerates many NVRTC compilations dominated by the unrealizable batchSize=2048 x seqLenKv=131072 corner: the runtime KV pool can hold at most ~135k tokens, so that combination can never appear at runtime. NVRTC compile time blows past the 600s server-start timeout for test_disaggregated_logprobs_serving[llama-3.1-8b-instruct]. Cap the warmup grid to a realistic working set (batch<=256, seqLenKv<=16384) in runJITWarmupGridIfRequested. The runtime kernel launch still receives the full engine maxima, so larger shapes JIT-compile lazily on first request (the original pre-PR behavior); only the eager-warmup grid is bounded. 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>
…oval: (1) pin `mMaxSeqLenKv Signed-off-by: Chenfei Zhang <chenfeiz@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>
Revert c37992c to remove padding.
Added warmup with a grid that should be dense enough to catch almost every possible JIT kernels. Note that this PR only enables warmup and have no shape binning at runtime. I'd like to see how it performs and if it can warmup most kernels then we may not need further works.
Added a message when a possible JIT happened, to give a clear clue why performance dropped.
Some warmup data collected on our unit tests.
The kernel compilation times are similar, generally falling within the 7-9s range. The increase on warmup time can be estimated as$t_{kernel Compile} \cdot (N_{WarmupKernels} - N_{OriginalWarmupKernel})$
Description
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
Release Notes