Skip to content

[None][test] add metric for trtllm-bench#14178

Merged
ruodil merged 2 commits into
NVIDIA:mainfrom
ruodil:user/ruodil/add_cases
May 18, 2026
Merged

[None][test] add metric for trtllm-bench#14178
ruodil merged 2 commits into
NVIDIA:mainfrom
ruodil:user/ruodil/add_cases

Conversation

@ruodil
Copy link
Copy Markdown
Collaborator

@ruodil ruodil commented May 15, 2026

Summary by CodeRabbit

  • Chores
    • Updated performance benchmarking metrics to track total output throughput and total token throughput separately, enabling more detailed performance measurement across multiple dimensions including per-GPU and per-user throughput analysis.

Review Change Stack

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-compatible or api-breaking. For api-breaking, include BREAKING in 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.

@ruodil ruodil requested a review from a team as a code owner May 15, 2026 08:27
@ruodil
Copy link
Copy Markdown
Collaborator Author

ruodil commented May 15, 2026

/bot skip --comment "skip test as just adding metrics"

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

📝 Walkthrough

Walkthrough

This PR migrates the perf testing infrastructure from using a single TOKEN_THROUGHPUT metric to a more granular TOTAL_OUTPUT_THROUGHPUT metric, adds TOTAL_TOKEN_THROUGHPUT support to benchmark parsing, and updates related thresholds, string mappings, and metric collection lists accordingly.

Changes

Perf Metric Type Migration

Layer / File(s) Summary
Enum type definition
tests/integration/defs/perf/utils.py
PerfMetricType enum adds TOTAL_OUTPUT_THROUGHPUT member.
Log parsing queries
tests/integration/defs/perf/test_perf.py
PERF_METRIC_LOG_QUERIES reassigns the throughput regex to TOTAL_OUTPUT_THROUGHPUT; BENCH_PERF_METRIC_LOG_QUERIES adds parsing for both TOTAL_OUTPUT_THROUGHPUT and TOTAL_TOKEN_THROUGHPUT; AGGR_SERVER_PERF_METRIC_LOG_QUERIES replaces TOKEN_THROUGHPUT placeholder with TOTAL_OUTPUT_THROUGHPUT.
Metric configuration and mappings
tests/integration/defs/perf/test_perf.py
PERF_METRIC_THRESHOLD adds regression thresholds for TOTAL_OUTPUT_THROUGHPUT; PERF_METRIC_STRING adds string mapping "total_output_throughput" for the new metric type.
Metric collection lists
tests/integration/defs/perf/test_perf.py
AGGR_SERVER_METRICS includes TOTAL_OUTPUT_THROUGHPUT instead of TOKEN_THROUGHPUT; BENCH_INFERENCE_METRICS expands to include TOTAL_OUTPUT_THROUGHPUT, TOTAL_TOKEN_THROUGHPUT, PER_GPU_OUTPUT_THROUGHPUT, and PER_USER_OUTPUT_THROUGHPUT.
Release test documentation
tests/integration/defs/perf/README_release_test.md
Performance metrics list updated to show TOTAL_OUTPUT_THROUGHPUT instead of TOKEN_THROUGHPUT.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is entirely empty, containing only the template structure with no actual content filled in any required sections. Fill in the 'Description' section explaining what metric is being added and why. Complete the 'Test Coverage' section listing relevant tests. Ensure the PR title follows the required format [JIRA/NVBugs/GitHub/None][type] Summary.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title '[None][test] add metric for trtllm-bench' is specific and directly related to the changes, which add TOTAL_OUTPUT_THROUGHPUT metric to the performance testing framework.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/integration/defs/perf/test_perf.py (1)

1-1: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update the SPDX copyright year.

This file is modified in this PR, but the header still ends at 2025. Please bump it to 2026 to match the latest modification year. As per coding guidelines, "Include NVIDIA copyright header on all new files; update year on modified files".

🤖 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 `@tests/integration/defs/perf/test_perf.py` at line 1, Update the SPDX header
year in the file's top comment by changing the copyright range end from 2025 to
2026; specifically modify the SPDX line (the header comment starting with "#
SPDX-FileCopyrightText") in tests/integration/defs/perf/test_perf.py so it reads
2022-2026.
🤖 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/integration/defs/perf/test_perf.py`:
- Around line 483-486: The metrics map is missing threshold entries for the new
bench throughput metrics, causing _get_metrics() (which calls
_get_metric_threshold/_get_metric_abs_threshold) to raise ValueError for
BENCH_INFERENCE_METRICS; add threshold tuples for
PerfMetricType.PER_USER_OUTPUT_THROUGHPUT and
PerfMetricType.PER_GPU_OUTPUT_THROUGHPUT (similar to
TOTAL_OUTPUT_THROUGHPUT/TOTAL_TOKEN_THROUGHPUT entries) so the tests can look up
thresholds without failing. Ensure you update the same mapping that currently
contains PerfMetricType.TOTAL_OUTPUT_THROUGHPUT and
PerfMetricType.TOTAL_TOKEN_THROUGHPUT.

---

Outside diff comments:
In `@tests/integration/defs/perf/test_perf.py`:
- Line 1: Update the SPDX header year in the file's top comment by changing the
copyright range end from 2025 to 2026; specifically modify the SPDX line (the
header comment starting with "# SPDX-FileCopyrightText") in
tests/integration/defs/perf/test_perf.py so it reads 2022-2026.
🪄 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: a713544e-9da1-409f-91d3-9e055e4c53fc

📥 Commits

Reviewing files that changed from the base of the PR and between ae8956f and 02da9d6.

⛔ Files ignored due to path filters (2)
  • tests/integration/defs/perf/base_perf.csv is excluded by !**/*.csv
  • tests/integration/defs/perf/base_perf_pytorch.csv is excluded by !**/*.csv
📒 Files selected for processing (3)
  • tests/integration/defs/perf/README_release_test.md
  • tests/integration/defs/perf/test_perf.py
  • tests/integration/defs/perf/utils.py

Comment thread tests/integration/defs/perf/test_perf.py
@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #48565 [ skip ] triggered by Bot. Commit: 02da9d6 Link to invocation

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #48565 [ skip ] completed with state SUCCESS. Commit: 02da9d6
Skipping testing for commit 02da9d6

Link to invocation

@ruodil ruodil enabled auto-merge (squash) May 15, 2026 09:26
ruodil added 2 commits May 18, 2026 03:11
Signed-off-by: Ruodi Lu <ruodil@users.noreply.github.com>
…rics

BENCH_INFERENCE_METRICS collects PER_USER_OUTPUT_THROUGHPUT and
PER_GPU_OUTPUT_THROUGHPUT, but PERF_METRIC_THRESHOLD had no entries for
them, causing _get_metric_threshold/_get_metric_abs_threshold to raise
ValueError before any bench perf command runs. Add (-0.1, 10) entries
matching the other throughput metrics.

Signed-off-by: Ruodi Lu <ruodil@users.noreply.github.com>
@ruodil ruodil force-pushed the user/ruodil/add_cases branch from 97c1ed2 to 33a3bfb Compare May 18, 2026 03:11
@ruodil
Copy link
Copy Markdown
Collaborator Author

ruodil commented May 18, 2026

/bot skip --comment "skip test as just adding metrics"

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #48821 [ skip ] triggered by Bot. Commit: 33a3bfb Link to invocation

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #48821 [ skip ] completed with state SUCCESS. Commit: 33a3bfb
Skipping testing for commit 33a3bfb

Link to invocation

@ruodil ruodil merged commit ccd98c3 into NVIDIA:main May 18, 2026
7 checks passed
KleinBlueC pushed a commit to KleinBlueC/TensorRT-LLM that referenced this pull request May 19, 2026
Signed-off-by: Ruodi Lu <ruodil@users.noreply.github.com>
Co-authored-by: Ruodi Lu <ruodil@users.noreply.github.com>
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.

3 participants