Skip to content

[None][infra] Move the pre-merge perf-sanity cases back to post-merge - #21320

Merged
chenfeiz0326 merged 1 commit into
NVIDIA:mainfrom
chenfeiz0326:chenfeiz/perf-sanity-premerge-to-postmerge
Oct 30, 2026
Merged

chenfeiz0326 merged 1 commit into
NVIDIA:mainfrom
chenfeiz0326:chenfeiz/perf-sanity-premerge-to-postmerge

Conversation

@chenfeiz0326

@chenfeiz0326 chenfeiz0326 commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Dev Engineer Review

The GB300 perf-sanity list now runs at post_merge, and its Jenkins stage is split into seven shards. The pre-merge perf-sanity stage is removed. The change affects stage scheduling, not test implementation. Test IDs and timeouts remain unchanged.

The documentation now states that no perf-sanity stage runs in pre-merge. No runtime code or API changes are reported. The author reports that seven shards keep the longest shard at 94.1 minutes, compared with 98.8 minutes for six shards.

QA Engineer Review

The test-list change moves the DeepSeek-V4-Pro ctx_only cases from pre-merge to post-merge. Their IDs and timeouts remain unchanged. The list remains in the test-db/ CI test-list directory, and Jenkins references it for the seven post-merge shards. No test functions or test-code files changed.

The author reports that list validation returned OK: 2096 unique test entries validated, pre-commit passed on changed files, jenkins/L0_Test.groovy parsed, and 13 mapping tests passed. The author also reports no additional CBTS failures beyond those present on main in that environment. Coverage verdict: sufficient for this scheduling change.

Per-File QA Perspective

  • jenkins/L0_Test.groovy: The GB300 perf-sanity post-merge stage now has seven shards, and the pre-merge stage is removed. Verify that post-merge scheduling invokes all seven shards and that the removed stage is not scheduled.
  • tests/integration/defs/perf/README_test_perf_sanity.md: The gating note now says no perf-sanity stage runs in pre-merge. Verify that this description stays aligned with the CI stage configuration.
  • tests/integration/test_lists/test-db/l0_gb300_multi_gpus_perf_sanity.yml: The list condition selects post_merge; DeepSeek-V4-Pro ctx_only cases remain in the list with their existing IDs and timeouts. This is a CI test-list file, and Jenkins references it for all seven shards.

Description

#18766 moved two DeepSeek-V4-Pro ctx_only cases from the GB300 post-merge perf-sanity list into a new pre-merge stage, GB300-4_GPUs-PyTorch-PerfSanity-1. It is the only perf-sanity stage in the PR pipeline. This PR moves both cases back to post-merge and removes that stage:

  • perf/test_perf_sanity.py::test_e2e[aggr_upload-ctx_only-gb300_deepseek-v4-pro-fp4_8k1k_con8_ctx1_dep4_gen4_tep8_eplb0_mtp3_ccb-NIXL]
  • perf/test_perf_sanity.py::test_e2e[aggr_upload-ctx_only-gb300_deepseek-v4-pro-fp4_8k1k_con666_ctx6_dep4_gen1_dep16_eplb384_mtp3_ccb-NIXL]

Changes:

  • l0_gb300_multi_gpus_perf_sanity.yml: removes the pre_merge block and puts both cases back in the post_merge block, next to the other DeepSeek-V4-Pro cases. Ids and TIMEOUTs are unchanged.
  • L0_Test.groovy: removes GB300-4_GPUs-PyTorch-PerfSanity-1; GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge goes from 6 to 7 shards.
  • README_test_perf_sanity.md: updates the one sentence that described the pre-merge perf-sanity stage.

What changes in CI (real launchTestJobs(), main vs. this PR):

  • PR pipeline: GB300-4_GPUs-PyTorch-PerfSanity-1 is no longer scheduled. Before, it ran whenever multi-GPU stages were added or CBTS force-kept perf-sanity stages, and a perf regression in either case failed the PR. Nothing else changes; x86_64 is unchanged.
  • Post-merge pipeline: still 66 SBSA stages, with -PerfSanity-1 replaced by -PerfSanity-Post-Merge-7. Post-merge already ran both cases on every build through -PerfSanity-1, because it adds all multi-GPU stages (L0_PostMerge perf: [AutoDeploy] Enable AutoDeploy as a backend in trtllm-bench #3041 ran and passed both there). They now run in -Post-Merge-4 (con8) and -Post-Merge-6 (con666). s_stage_name is not a perf-history match key, so their baselines carry over.

Shard sizing from .test_durations (32 tests, 621 min):

GB300-4 post-merge perf-sanity longest shard shortest shard
main: 6 shards, 30 tests 98.8 min 94.1 min
6 shards, 32 tests (not chosen) 106.8 min 101.3 min
this PR: 7 shards, 32 tests 94.1 min 85.4 min

With 7 shards, no shard is longer than today's longest. The 94.1 min shard is the con4301 case alone, as it is today.

Left in pre-merge on purpose: l0_b200.yml's test_e2e[aggr_upload-host_perf_deepseek_v3_lite-v3lite_fp8_bs8_128_256]. It belongs to the host-perf regression suite (#12148) and was never a post-merge case.

Test Coverage

Offline:

  • scripts/check_test_list.py --validate: OK: 2096 unique test entries validated (same count as main).
  • pre-commit run on the changed files: passed.
  • jenkins/L0_Test.groovy parses. The real launchTestJobs(), run with Jenkins steps stubbed for SBSA and x86_64 × {pre-merge, pre-merge with a multi-GPU file change, pre-merge with CBTS perfsanity_required, post-merge}, gives exactly the stage diff above.
  • Shard membership comes from this commit's jenkins/scripts/perf/submit.py::_select_least_duration_group, the mirror of pytest-split least_duration. As a control, the same simulation at L0_PostMerge perf: [AutoDeploy] Enable AutoDeploy as a backend in trtllm-bench #3041's commit put 30 of 30 tests in the shard they actually ran in.
  • tests/unittest/tools/test_test_to_stage_mapping.py: 13 passed. tests/unittest/scripts/cbts: no failures beyond those main also has in this environment.

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.

🤖 Generated with Claude Code

Move the two DeepSeek-V4-Pro ctx_only cases from the pre-merge block of
l0_gb300_multi_gpus_perf_sanity.yml into its post-merge block and remove
the pre-merge stage GB300-4_GPUs-PyTorch-PerfSanity-1. No perf-sanity
stage runs in the PR pipeline any more.

Split the GB300-4 post-merge perf-sanity stage 7 ways instead of 6 so
that the longest shard stays below its current length.

Signed-off-by: chenfeiz0326 <203214996+chenfeiz0326@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Walkthrough

The GB300 4-GPU PerfSanity configuration removes its pre-merge stage and increases the post-merge shard count from six to seven. The test list switches to post-merge, adds the concurrency-180 ctx_only case, and retains the concurrency-8 and concurrency-666 cases. The documentation now states that no pre-merge perf-sanity stage runs.

Changes

GB300 PerfSanity scheduling

Layer / File(s) Summary
Move GB300 PerfSanity to post-merge
tests/integration/test_lists/test-db/l0_gb300_multi_gpus_perf_sanity.yml, jenkins/L0_Test.groovy, tests/integration/defs/perf/README_test_perf_sanity.md
The test list selects post-merge and adds the concurrency-180 case while retaining the concurrency-8 and concurrency-666 cases. Jenkins removes the pre-merge stage and configures seven post-merge shards. The documentation states that no pre-merge perf-sanity stage currently runs.

Priority: Low

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

Change: Other

Possibly related PRs

Suggested reviewers: chzblych, tburt-nv, brnguyen2

Merge Risk: Minimal · up to 64cdf

The scheduling, shard-count, test-list, and documentation changes are consistent and ready to merge.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly summarizes the main change: moving the pre-merge perf-sanity cases back to post-merge. It uses the required ticket/type format and is concise.
Description check Passed The description is complete and directly related to the pull request. It explains the issue, the solution, CI impact, shard sizing, test coverage, and checklist status.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
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.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (3 skipped: 3 unsupported.)

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@chenfeiz0326
chenfeiz0326 enabled auto-merge (squash) October 30, 2026 05:04

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

APPROVE

Review summary - Approve

Reviewed the full diff; no blocking or major issues found.

Automated review by NVCortex Lite, run by @fredricz-20070104.

@github-actions

Copy link
Copy Markdown

Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements.

@chenfeiz0326

Copy link
Copy Markdown
Collaborator Author

/bot skip --comment "Only move perf tests from pre-merge to post-merge, no need to run the whole CI pipeline"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #81624 [ skip ] triggered by Bot. Commit: 64cdfe9 Link to invocation

@ZhanruiSunCh ZhanruiSunCh 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 for infra part

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #81624 [ skip ] completed with state SUCCESS. Commit: 64cdfe9
Skipping testing for commit 64cdfe9

Link to invocation

@chenfeiz0326
chenfeiz0326 merged commit 6086b1c into NVIDIA:main Oct 30, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants