[None][fix] only configure gc thresholds once by ixlmar · Pull Request #13910 · NVIDIA/TensorRT-LLM

ixlmar · 2026-05-08T15:03:46Z

Description

Avoids latency spikes caused by repeated calls to gc.set_threshold.

Test Coverage

n/a

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)
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

Refactor
- Enhanced garbage collection efficiency by optimizing how memory management thresholds are applied during executor result processing.
- Improved thread context management to ensure garbage collection optimizations apply at the thread level rather than at operation level.
- Consolidated internal utilities and simplified context handling in core threading components.
Chores
- Removed internal helper class.

ixlmar · 2026-05-08T15:04:00Z

/bot run

ixlmar · 2026-05-08T15:13:03Z

/bot run

tensorrt-cicd · 2026-05-08T15:15:52Z

PR_Github #47416 [ run ] triggered by Bot. Commit: 2cfda96 Link to invocation

tensorrt-cicd · 2026-05-08T15:18:31Z

PR_Github #47417 [ run ] triggered by Bot. Commit: 2cfda96 Link to invocation

coderabbitai · 2026-05-08T16:10:20Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c059d174-2103-4817-9307-eaaef51bc2d3

📥 Commits

Reviewing files that changed from the base of the PR and between 1651d1b and 2cfda96.

📒 Files selected for processing (2)

tensorrt_llm/executor/proxy.py
tensorrt_llm/llmapi/utils.py

📝 Walkthrough

Walkthrough

The PR refactors garbage collection threshold customization in the executor's result dispatcher. Previously, the GC threshold context wrapped only the blocking queue-check operation; now it is passed to the ManagedThread as a context manager, applying the customization at the entire thread-execution level. The ManagedThread class gains an optional context parameter and wraps its main loop accordingly, replacing the now-removed ContextManager helper class.

Changes

GC Threshold Thread Context

Layer / File(s)	Summary
Imports & Helper Class Removal `tensorrt_llm/llmapi/utils.py`	`nullcontext` and `ContextManager` type imports are added; the `ContextManager` helper class definition is removed.
ManagedThread Context Support `tensorrt_llm/llmapi/utils.py`	`ManagedThread.__init__()` accepts an optional `context` parameter and stores it; `run()` main loop is wrapped with `with self.context:` so thread execution occurs within the supplied context manager (or `nullcontext()` by default).
GC Threshold Thread Context `tensorrt_llm/executor/proxy.py`	Result dispatcher `ManagedThread` receives `context=customized_gc_thresholds(...)` instead of wrapping only the sentinel check, shifting GC-threshold scope to the entire thread.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

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.
Description check	❓ Inconclusive	The description explains the issue (latency spikes from repeated gc.set_threshold calls) but lacks details on the solution, affected components, and test coverage is marked as n/a without explanation.	Expand the description to explain how the solution works (context manager approach, thread-level configuration) and clarify why test coverage is not applicable for these changes.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: moving GC threshold configuration to occur once at thread level rather than repeatedly.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

Generate code and open pull requests
Plan features and break down work
Investigate incidents and troubleshoot customer tickets together
Automate recurring tasks and respond to alerts with triggers
Summarize progress and report instantly

Built for teams:

Shared memory across your entire org—no repeating context
Per-thread sandboxes to safely plan and execute work
Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

tensorrt-cicd · 2026-05-08T19:19:06Z

PR_Github #47417 [ run ] completed with state SUCCESS. Commit: 2cfda96
/LLM/main/L0_MergeRequest_PR pipeline #37344 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

Please check the failed tests and fix your PR
If you cannot view the failures, ask the CI triggerer to share details
Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

Signed-off-by: ixlmar <206748156+ixlmar@users.noreply.github.com>

ixlmar · 2026-05-11T08:39:19Z

/bot run

tensorrt-cicd · 2026-05-11T08:44:59Z

PR_Github #47704 [ run ] triggered by Bot. Commit: 5c6b37a Link to invocation

tensorrt-cicd · 2026-05-11T13:43:27Z

PR_Github #47704 [ run ] completed with state SUCCESS. Commit: 5c6b37a
/LLM/main/L0_MergeRequest_PR pipeline #37600 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

Please check the failed tests and fix your PR
If you cannot view the failures, ask the CI triggerer to share details
Once fixed, request an NVIDIA team member to trigger CI again

Link to invocation

ixlmar · 2026-05-11T14:55:16Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-05-11T15:02:01Z

PR_Github #47761 [ run ] triggered by Bot. Commit: 5c6b37a Link to invocation

tensorrt-cicd · 2026-05-12T07:13:58Z

PR_Github #47761 [ run ] completed with state SUCCESS. Commit: 5c6b37a
/LLM/main/L0_MergeRequest_PR pipeline #37652 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

Please check the failed tests and fix your PR
If you cannot view the failures, ask the CI triggerer to share details
Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

ixlmar · 2026-05-12T07:16:42Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-05-12T07:22:51Z

PR_Github #47920 [ run ] triggered by Bot. Commit: 5c6b37a Link to invocation

tensorrt-cicd · 2026-05-12T12:16:38Z

PR_Github #47920 [ run ] completed with state SUCCESS. Commit: 5c6b37a
/LLM/main/L0_MergeRequest_PR pipeline #37767 completed with status: 'SUCCESS'

CI Report

Link to invocation

Superjomn

LGTM on the llmapi changes

…_cache-dp4-trtllm-fp8] The test was waived after a single round-4 pytest-timeout in pre-merge build #37652 (PR NVIDIA#13910). Investigation shows that's a false-positive auto-file: - Same build's rounds 1-3 all PASSED this test (14:43 / 16:21 / 19:01) but their results.xml were dropped at SLURM TimeLimit with the Jenkins log line 'Stage is interrupted, skip to upload test result.' Only round 4's results.xml reached the bot, and on round 4 the test happened to hit pytest's per-test --timeout=3600 instead of finishing in the usual ~10-20 min. - Reproduced on dlcluster 4x GB200: 11/11 PASS (job 1003130: 1 PASS at 13:29; job 1003278: 10-iter loop, all PASS, 8:42-16:53, mean 10:44). - PR NVIDIA#13910 is a 2-file gc-threshold refactor with no GPT-OSS / MoE / KV-cache / test-infra code touched, so it cannot be the cause; it was approved and merged to main ~10h after this NVBug was filed. Across all known executions of this exact parameterization on real GB200, 14/15 are PASS; the one cliff event was on nvl72129-T18, a node not reachable from the lease used for the repro. Signed-off-by: Xiao Wang <24860335+xwang233@users.noreply.github.com>

Signed-off-by: ixlmar <206748156+ixlmar@users.noreply.github.com>

ixlmar requested review from Funatiq and dcampora May 8, 2026 15:03

github-actions Bot assigned ixlmar May 8, 2026

ixlmar mentioned this pull request May 8, 2026

[None][feat] support calling gc.freeze #13907

Closed

1 task

ixlmar marked this pull request as ready for review May 8, 2026 16:05

ixlmar requested a review from a team as a code owner May 8, 2026 16:05

ixlmar requested a review from nv-guomingz May 8, 2026 16:05

fix: only configure gc thresholds once

5c6b37a

Signed-off-by: ixlmar <206748156+ixlmar@users.noreply.github.com>

ixlmar force-pushed the fix/gc-config branch from 2cfda96 to 5c6b37a Compare May 11, 2026 08:39

Funatiq approved these changes May 11, 2026

View reviewed changes

ixlmar enabled auto-merge (squash) May 12, 2026 08:04

Superjomn approved these changes May 12, 2026

View reviewed changes

ixlmar merged commit 0119a23 into NVIDIA:main May 12, 2026
7 checks passed

ixlmar deleted the fix/gc-config branch May 12, 2026 13:24

yufeiwu-nv pushed a commit to yufeiwu-nv/TensorRT-LLM that referenced this pull request May 19, 2026

[None][fix] only configure gc thresholds once (NVIDIA#13910)

b8c85b7

Signed-off-by: ixlmar <206748156+ixlmar@users.noreply.github.com>

Uh oh!

Conversation

ixlmar commented May 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test Coverage

PR Checklist

GitHub Bot Help

Summary by CodeRabbit

Uh oh!

ixlmar commented May 8, 2026

Uh oh!

ixlmar commented May 8, 2026

Uh oh!

tensorrt-cicd commented May 8, 2026

Uh oh!

tensorrt-cicd commented May 8, 2026

Uh oh!

coderabbitai Bot commented May 8, 2026

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

tensorrt-cicd commented May 8, 2026

Uh oh!

ixlmar commented May 11, 2026

Uh oh!

tensorrt-cicd commented May 11, 2026

Uh oh!

tensorrt-cicd commented May 11, 2026

Uh oh!

ixlmar commented May 11, 2026

Uh oh!

tensorrt-cicd commented May 11, 2026

Uh oh!

tensorrt-cicd commented May 12, 2026

Uh oh!

ixlmar commented May 12, 2026

Uh oh!

tensorrt-cicd commented May 12, 2026

Uh oh!

tensorrt-cicd commented May 12, 2026

Uh oh!

Superjomn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ixlmar commented May 8, 2026 •

edited by coderabbitai Bot

Loading