Skip to content

test(trtllm): speed up TRTLLM gpu test time by reducing imports - #12413

Merged
rmccorm4 merged 11 commits into
mainfrom
rmccormick/trtllm-test-fixes
Aug 4, 2026
Merged

test(trtllm): speed up TRTLLM gpu test time by reducing imports#12413
rmccorm4 merged 11 commits into
mainfrom
rmccormick/trtllm-test-fixes

Conversation

@rmccorm4

@rmccorm4 rmccorm4 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Reduce TRTLLM test job time from ~25min to ~19min.

  • assign independent system-status ports to the TensorRT-LLM multimodal prefill and decode workers
  • gate pd_multimodal startup on active one-token canary health checks instead of a fixed 120-second delay
  • move 79 import-heavy, zero-VRAM TensorRT-LLM tests out of the per-node GPU-parallel orchestrator and into the existing single-process GPU stage to amortize a single TRTLLM import across all of the fast tests instead of paying the slow import cost across all parallel processes.
  • retain the dedicated model-predownload sentinel and the real 2 GiB CUDA logits test in the parallel pool

Root cause

pd_multimodal used a fixed startup sleep because the prefill and decode workers shared a system-status port. A readiness check against that shared port could report the wrong worker as ready. Independent ports make per-worker active readiness probes reliable.

Separately, the GPU-parallel orchestrator launches one pytest subprocess per selected test node. In run 30510438808, 79 import-heavy tests were marked profiled_vram_gib(0). They used no GPU memory but each paid pytest and TensorRT-LLM import/setup overhead, consuming process slots and delaying queued positive-VRAM workloads while the GPU was nearly idle.

The rc22 request-ID compatibility behavior is unchanged by this patch.

Impact

The multimodal test now advances as soon as both workers successfully serve a canary request, while retaining bounded timeouts and useful failure logs.

The moved tests remain marked pre_merge, trtllm, and gpu_1. Omitting only profiled_vram_gib(0) routes them through the existing sequential GPU expression, preserving their CUDA-visible runtime while sharing one TensorRT-LLM initialization. Baseline logs show the 79 nodes accounted for about 4,087 child-seconds and delayed the next 20 GiB workload by about 372 seconds. The expected wall-clock reduction is roughly six minutes, pending a new full-CI run.

Validation

  • pre-commit passed for all eight marker-routing files
  • bash -n tests/backends/trtllm/common.sh tests/backends/trtllm/agg_multimodal.sh tests/backends/trtllm/pd_multimodal.sh
  • focused multimodal helper tests: 7 passed, 5 skipped
  • marker audit confirms the real CUDA logits test retains profiled_vram_gib(2.0) and the predownload sentinel retains profiled_vram_gib(0)
  • full TensorRT-LLM GPU E2E validation is pending CI; the local environment does not include tensorrt_llm

Summary by CodeRabbit

  • Bug Fixes

    • Prevented system-status port collisions between disaggregated multimodal prefill and decode workers.
    • Improved worker readiness checks by requiring a successful health canary before reporting readiness.
  • Tests

    • Updated GPU test scheduling and documentation to support shared TensorRT-LLM initialization and avoid unnecessary VRAM profiling.

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
@github-actions github-actions Bot added test backend::trtllm Relates to the trtllm backend labels Jul 30, 2026
@datadog-official

This comment has been minimized.

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
@pull-request-size pull-request-size Bot added size/M and removed size/S labels Jul 30, 2026
@rmccorm4 rmccorm4 changed the title test(trtllm): replace multimodal startup delay with readiness checks test(trtllm): reduce pre-merge GPU startup overhead Jul 30, 2026
@rmccorm4 rmccorm4 changed the title test(trtllm): reduce pre-merge GPU startup overhead test(trtllm): speed up TRTLLM gpu test time by reducing imports Jul 30, 2026
@rmccorm4
rmccorm4 marked this pull request as ready for review July 30, 2026 05:56
@rmccorm4
rmccorm4 requested review from a team as code owners July 30, 2026 05:56
@rmccorm4
rmccorm4 enabled auto-merge (squash) July 30, 2026 05:56
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

The change removes zero-VRAM profiling from TRT-LLM-related tests, documents shared sequential GPU initialization, assigns separate system-status ports to disaggregated workers, and enables dynamic health checks for the multimodal serving test.

TRT-LLM Runtime Updates

Layer / File(s) Summary
Unprofiled TRT-LLM test scheduling
components/src/dynamo/trtllm/tests/*, tests/kvbm_integration/test_consolidator_config_unit.py, tests/serve/test_trtllm_mm_hashes_protocol.py
TRT-LLM-related tests remove the zero-VRAM profiling marker and document sequential GPU-stage initialization; related CUDA-context comments are clarified.
Disaggregated worker system ports
examples/backends/trtllm/launch/disagg_multimodal.sh
Prefill and decode workers use separate system-status ports, defaulting to 8081 and 8082.
Multimodal worker health checks
tests/serve/test_trtllm.py
The multimodal serving configuration enables worker health checks and sets DYN_HEALTH_CHECK_ENABLED instead of using delayed startup.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and matches the main goal of speeding up TRTLLM GPU tests by reducing import overhead.
Description check ✅ Passed The description is mostly complete and covers summary, root cause, impact, validation, and related issue context.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@tests/serve/test_trtllm.py`:
- Line 359: Add an explicit pytest.mark.timeout(900) entry to the marks list for
the pd_multimodal test, alongside the existing health_check_workers
configuration. Do not rely on TRTLLMConfig.timeout, since it does not enforce a
pytest test timeout.
🪄 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: 799b0939-3021-4761-a5a2-20ceb29ebebe

📥 Commits

Reviewing files that changed from the base of the PR and between 0caba6a and c4c230c.

📒 Files selected for processing (10)
  • components/src/dynamo/trtllm/tests/test_trtllm_autodeploy.py
  • components/src/dynamo/trtllm/tests/test_trtllm_image_token_resolver.py
  • components/src/dynamo/trtllm/tests/test_trtllm_logits_runtime.py
  • components/src/dynamo/trtllm/tests/test_trtllm_main_init.py
  • components/src/dynamo/trtllm/tests/test_trtllm_multimodal_processor.py
  • components/src/dynamo/trtllm/tests/test_trtllm_unit.py
  • examples/backends/trtllm/launch/disagg_multimodal.sh
  • tests/kvbm_integration/test_consolidator_config_unit.py
  • tests/serve/test_trtllm.py
  • tests/serve/test_trtllm_mm_hashes_protocol.py

Comment thread tests/serve/test_trtllm.py

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment thread tests/serve/test_trtllm.py
devin-ai-integration[bot]

This comment was marked as resolved.

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
devin-ai-integration[bot]

This comment was marked as resolved.

rmccorm4 added 2 commits July 31, 2026 14:10
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment thread tests/serve/test_trtllm.py
devin-ai-integration[bot]

This comment was marked as resolved.

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment thread examples/backends/trtllm/launch/disagg_multimodal.sh Outdated
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

Open in Devin Review

Comment thread tests/serve/test_trtllm.py
Comment thread tests/serve/test_trtllm.py
@rmccorm4
rmccorm4 merged commit 3f2230f into main Aug 4, 2026
108 checks passed
@rmccorm4
rmccorm4 deleted the rmccormick/trtllm-test-fixes branch August 4, 2026 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend::trtllm Relates to the trtllm backend size/M test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants