Skip to content

[https://nvbugs/6248987][fix] Made the slow-tokenizer swap lazy and idempotent. __init__ now just sets `_slo - #14846

Merged
longlee0622 merged 1 commit into
NVIDIA:mainfrom
tensorrt-cicd:repair-bot-bug6248987
Jun 3, 2026
Merged

[https://nvbugs/6248987][fix] Made the slow-tokenizer swap lazy and idempotent. __init__ now just sets `_slo#14846
longlee0622 merged 1 commit into
NVIDIA:mainfrom
tensorrt-cicd:repair-bot-bug6248987

Conversation

@tensorrt-cicd

@tensorrt-cicd tensorrt-cicd commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Root cause: PR [https://nvbugs/6182617][fix] Restore K2.5 multimodal dep8 accuracy test on transformers 5.5.x #14392 (commit 546a5b0) added an unconditional call to _ensure_k25_slow_tokenizer() in KimiK25InputProcessor.__init__, swapping in the pure-Python TikTokenTokenizer. On the text-only k25_thinking_fp4 perf path (ISL=8K, concurrency=2) every request is tokenized on the orchestrator's GIL, inflating _fetch_new_requests / broadcast_requests ~34% and TPOT 9.8× (output_token_throughput drops ~92%). GPU kernel inventory and runtimes were unchanged — pure host overhead.
  • Fix: Made the slow-tokenizer swap lazy and idempotent. __init__ now just sets _slow_tokenizer_active=False. _ensure_k25_slow_tokenizer() early-returns once active and is invoked only when an input actually needs correct mapping of K2.5 special tokens — i.e. multimodal data is present, the disagg get_prompt_token_ids path runs, or the prompt itself contains markers like <|media_pad|> / <|im_user|> (checked via _input_needs_slow_tokenizer). Pure-text prompts keep the fast Rust tokenizer; the multimodal accuracy path NVBug 6182617 still gets the slow tokens_trie.
  • Automated fix generated by repair-bot

Test plan

  • Verify fix on the same GPU type as the original failure
  • Check for regressions in related tests

Links

Summary by CodeRabbit

Bug Fixes

  • Improved K2.5 tokenization with optimized special token detection and lazy loading
  • Enhanced multimodal input processing to ensure correct token mapping for media placeholders
  • Refined tokenizer initialization behavior for better performance across different input scenarios

…_` now just sets `_slo

Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>
@tensorrt-cicd
tensorrt-cicd requested a review from a team as a code owner June 2, 2026 02:46
@tensorrt-cicd
tensorrt-cicd requested a review from syuoni June 2, 2026 02:46
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR introduces lazy slow-tokenizer loading for K2.5 special-token handling in the Kimi K25 model. Rather than eagerly swapping tokenizers, the processor now detects K2.5-specific markers in prompt text and conditionally activates the slow tokenizer only when needed, improving performance on marker-free prompts.

Changes

K2.5 Lazy Slow-Tokenizer Loading

Layer / File(s) Summary
Special token markers and state initialization
tensorrt_llm/_torch/models/modeling_kimi_k25.py
Module-level constant _K25_SPECIAL_TOKEN_MARKERS lists K2.5 media and control markers. KimiK25InputProcessor.__init__ initializes _slow_tokenizer_active = False to support lazy swap tracking.
Idempotent slow-tokenizer swapping
tensorrt_llm/_torch/models/modeling_kimi_k25.py
New _input_needs_slow_tokenizer(text) helper detects marker presence. _ensure_k25_slow_tokenizer() becomes idempotent via early return when already active, sets _slow_tokenizer_active = True after swap, and updates log messaging.
Lazy swapping in text and multimodal paths
tensorrt_llm/_torch/models/modeling_kimi_k25.py
call_with_text_prompt() conditionally swaps slow tokenizer for text-only prompts only when markers are detected, and always ensures it for multimodal. get_prompt_token_ids() ensures slow tokenizer before placeholder expansion in disaggregated serving.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title is truncated and incomplete, cutting off mid-sentence at 110 characters. It describes the main fix (lazy and idempotent slow-tokenizer swap) but is unfinished. Complete the truncated title. It should be a full sentence like: '[https://nvbugs/6248987][fix] Made the slow-tokenizer swap lazy and idempotent.'
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description covers the root cause, fix details, and test plan well. However, it lacks the PR Checklist section required by the template, though the core content sections are substantially complete.
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

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

@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.

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/models/modeling_kimi_k25.py (1)

1220-1241: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve the fast tokenizer after the first slow-tokenizer request.

This still mutates the shared tokenizer permanently. Once a multimodal request or get_prompt_token_ids() hits _ensure_k25_slow_tokenizer(), later pure-text requests at Lines 1274-1279 also run through the slow Python tokenizer because self._tokenizer now points at it. That brings the perf regression back for the lifetime of this KimiK25InputProcessor, which contradicts the PR goal of keeping pure-text prompts on the fast path. Keep separate fast/slow tokenizer references and select per call instead of overwriting self._tokenizer.

Also applies to: 1274-1279, 1524-1526

🤖 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/models/modeling_kimi_k25.py` around lines 1220 - 1241,
The current _ensure_k25_slow_tokenizer implementation permanently overwrites
self._tokenizer with the slow Python tokenizer, causing later pure-text calls
(e.g., get_prompt_token_ids and the pure-text path) to use the slow path;
instead, keep the original fast tokenizer and store the slow tokenizer
separately (e.g., self._slow_tokenizer) and a flag self._slow_tokenizer_active,
do not assign slow_tok to self._tokenizer; update only self._processor.tokenizer
if the image-only code truly requires the slow instance, and change call sites
(where you currently reference self._tokenizer for tokenization in multimodal vs
pure-text paths) to select slow vs fast explicitly (use self._slow_tokenizer
when self._slow_tokenizer_active and multimodal/image path, otherwise use the
preserved fast tokenizer).
🤖 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.

Outside diff comments:
In `@tensorrt_llm/_torch/models/modeling_kimi_k25.py`:
- Around line 1220-1241: The current _ensure_k25_slow_tokenizer implementation
permanently overwrites self._tokenizer with the slow Python tokenizer, causing
later pure-text calls (e.g., get_prompt_token_ids and the pure-text path) to use
the slow path; instead, keep the original fast tokenizer and store the slow
tokenizer separately (e.g., self._slow_tokenizer) and a flag
self._slow_tokenizer_active, do not assign slow_tok to self._tokenizer; update
only self._processor.tokenizer if the image-only code truly requires the slow
instance, and change call sites (where you currently reference self._tokenizer
for tokenization in multimodal vs pure-text paths) to select slow vs fast
explicitly (use self._slow_tokenizer when self._slow_tokenizer_active and
multimodal/image path, otherwise use the preserved fast tokenizer).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 00aa6cee-0714-48cc-8011-ba3ec6f5645a

📥 Commits

Reviewing files that changed from the base of the PR and between 6222112 and 9431982.

📒 Files selected for processing (1)
  • tensorrt_llm/_torch/models/modeling_kimi_k25.py

@chenfeiz0326
chenfeiz0326 requested a review from tianyuxbear June 2, 2026 04:26
@chenfeiz0326

Copy link
Copy Markdown
Collaborator

/bot run --disable-fail-fast --stage-list "DGX_B200-8_GPUs-PyTorch-PerfSanity-Post-Merge-1,GB200-4_GPUs-PyTorch-PerfSanity-Post-Merge-7"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator Author

PR_Github #51500 [ run ] triggered by Bot. Commit: 9431982 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator Author

PR_Github #51500 [ run ] completed with state SUCCESS. Commit: 9431982
/LLM/main/L0_MergeRequest_PR pipeline #40905 (Partly Tested) 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

@chenfeiz0326

Copy link
Copy Markdown
Collaborator

/bot run --disable-fail-fast --stage-list "GB200-4_GPUs-PyTorch-PerfSanity-Post-Merge-1,GB200-4_GPUs-PyTorch-PerfSanity-Post-Merge-2,GB200-4_GPUs-PyTorch-PerfSanity-Post-Merge-3,GB200-4_GPUs-PyTorch-PerfSanity-Post-Merge-4,GB200-4_GPUs-PyTorch-PerfSanity-Post-Merge-5,GB200-4_GPUs-PyTorch-PerfSanity-Post-Merge-6"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator Author

PR_Github #51618 [ run ] triggered by Bot. Commit: 9431982 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator Author

PR_Github #51618 [ run ] completed with state SUCCESS. Commit: 9431982
/LLM/main/L0_MergeRequest_PR pipeline #41003 (Partly Tested) 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

@chenfeiz0326

chenfeiz0326 commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

@tianyuxbear You PR: #14392 broke K25 perf. Now Perf recovers.

k25_thinking_fp4_tep8_8k1k-con2_iter10_8k1k [main] [b200]
Branch: main
Stage Name: DGX_B200-8_GPUs-PyTorch-PerfSanity-Post-Merge-1
Pytest Command: aggr_upload-k25_thinking_fp4_blackwell-k25_thinking_fp4_tep8_8k1k

Dashboar link: https://tensorrt-llm.tensorrt-llm-perf-ci-report.sc2-paas.nvidia.com/pre-merge?selectedBranches=main&selectedCurve=__all__&pinnedSection=k25_thinking_fp4_tep8_8k1k-con2_iter10_8k1k%7Cmain%7Cb200&in_build-id-input=40905&in_job-name-select=LLM%2Fmain%2FL0_MergeRequest_PR

image

k25_thinking_fp4_tep4_8k1k-con2_iter10_8k1k [main] [gb200]
Branch: main
Stage Name: GB200-4_GPUs-PyTorch-PerfSanity-Post-Merge-7
Pytest Command: aggr_upload-k25_thinking_fp4_grace_blackwell-k25_thinking_fp4_tep4_8k1k

Dashboard link: https://tensorrt-llm.tensorrt-llm-perf-ci-report.sc2-paas.nvidia.com/pre-merge?selectedBranches=main&selectedCurve=__all__&pinnedSection=k25_thinking_fp4_tep4_8k1k-con2_iter10_8k1k%7Cmain%7Cgb200&in_build-id-input=41003&in_job-name-select=LLM%2Fmain%2FL0_MergeRequest_PR

image

@chenfeiz0326

Copy link
Copy Markdown
Collaborator

/bot run --disable-fail-fast

@chenfeiz0326
chenfeiz0326 requested a review from litaotju June 3, 2026 05:57
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator Author

PR_Github #51769 [ run ] triggered by Bot. Commit: 9431982 Link to invocation

@chenfeiz0326
chenfeiz0326 requested a review from lancelly June 3, 2026 06:43
@longlee0622
longlee0622 enabled auto-merge (squash) June 3, 2026 06:59

@lancelly lancelly left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator Author

PR_Github #51769 [ run ] completed with state SUCCESS. Commit: 9431982
/LLM/main/L0_MergeRequest_PR pipeline #41137 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

@longlee0622

Copy link
Copy Markdown
Collaborator

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator Author

PR_Github #51817 [ run ] triggered by Bot. Commit: 9431982 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator Author

PR_Github #51817 [ run ] completed with state SUCCESS. Commit: 9431982
/LLM/main/L0_MergeRequest_PR pipeline #41180 completed with status: 'SUCCESS'

CI Report

Link to invocation

@longlee0622
longlee0622 merged commit e5b8094 into NVIDIA:main Jun 3, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants