[https://nvbugs/6682113][fix] give gb300 v4-pro con4301 ctx_only perf-sanity the 120min budget its workload already carries - #18859
Conversation
…-sanity the 120min budget its workload already carries The aggr_upload-ctx_only con4301 row was waived for exceeding TIMEOUT (90). It is not failing: 3/3 reps on GB300 complete all 43010 requests with 0 failures, taking 5975/5947/5767 s against a 5400 s budget. The same 43010-request workload already runs with TIMEOUT (120) in its two disagg sibling rows, which share the identical config yaml and pass. This aligns the outlier row with that budget and removes its waiver. The wall time is legitimate, not an inefficiency: ~99.8% of the benchmark window is iteration execution, per-iteration cost is flat to 0.07% across 10759 iterations, and the balancer is static (layer_updates_per_iter=0). Startup is 1075 s of which only 366 s is plausibly reducible, so 3/3 reps would still miss 90 min even with all warmup removed. Signed-off-by: chenfeiz0326 <203214996+chenfeiz0326@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughThe GB300 DeepSeek-V4-Pro FP4 performance sanity test timeout increases from 90 to 120 seconds. The related skip waiver is removed. ChangesGB300 performance sanity test
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This enables the GB300 performance sanity test with a longer execution budget and removes its skip waiver. The configuration change is narrowly scoped and has no identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/bot run --stage-list "GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-1,GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-2,GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-3,GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-4,GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-5" |
|
PR_Github #72049 [ run ] triggered by Bot. Commit: |
|
PR_Github #72049 [ run ] completed with state
|
|
/bot skip --comment "Only unwaive perf tests, no need to run the whole CI pipeline" |
|
PR_Github #72133 [ skip ] triggered by Bot. Commit: |
|
PR_Github #72133 [ skip ] completed with state |
…exempt-and-list-refactor Conflict in tests/integration/test_lists/test-db/l0_gb300_multi_gpus_perf_sanity.yml: upstream NVIDIA#18859 (https://nvbugs/6682113) raised the v4-pro con4301 ctx_only TIMEOUT from 90 to 120 in the single post_merge block, while this branch split that block into pre_merge and post_merge. Resolved by keeping the split and taking upstream's 120 min budget on con4301, which stays in post_merge. The v4-pro con666 entry that upstream still lists in post_merge is not re-added there: this branch moved it to pre_merge, so the post-merge pipeline (pre_merge + post_merge) still runs it, at the same 90 min timeout, and re-adding it would duplicate it across the two blocks. Verified against the new upstream tip: 13 pre-merge perf-sanity cases, no new pre/post duplication, and no change to the post-merge run list. Signed-off-by: chenfeiz0326 <203214996+chenfeiz0326@users.noreply.github.com>
…o OpenSearch Re-lands NVIDIA#18445, reverted in NVIDIA#18736 after post-merge CI hit: tests/unittest/tools/test_perf_sanity_matching.py:: test_warmup_is_derived_from_exactly_the_e2e_and_ctx_only_modes ModuleNotFoundError: No module named 'defs.perf.time_breakdown_metrics' The root cause is a semantic merge conflict between two independently-green PRs, not a defect in either. NVIDIA#18432, merged two days earlier, added a _load_module() helper that execs tests/integration/defs/perf/test_perf_sanity.py by file path under a synthetic `defs.perf` package with an empty __path__, plus a closed allowlist of stub siblings in sys.modules so the heavy ones (torch, the OpenSearch client) are never imported. NVIDIA#18445 added a new real sibling, time_breakdown_metrics, and imported it from test_perf_sanity.py. With an empty __path__ and no stub entry, that import cannot resolve. The failing test is not one of NVIDIA#18445's own; dropping those would leave the failure unchanged. The fix belongs in the helper rather than in the perf code: give the synthetic package a real __path__ so a stdlib-only sibling resolves for real. time_breakdown_metrics is deliberately stdlib-only, so it imports cleanly in a CPU-only job. Entries already in sys.modules still take precedence, so the heavy siblings stay stubbed, and the next stdlib-only sibling will not break this file. Everything else is NVIDIA#18445 unchanged, apart from two conflicts with main: * l0_gb300_multi_gpus_perf_sanity.yml keeps main's 120-minute budget for the con4301 ctx_only case (NVIDIA#18859, nvbugs/6682113) and re-adds the time_breakdown con666 lane at 90, the budget its own workload carries. * waives.txt is left as main has it. nvbugs/6661856 has since been unwaived and the con8 base cases now run, so the time_breakdown sibling is not re-waived. Verified on CPU: test_perf_sanity_matching.py 19/19 (previously 1 failed), test_time_breakdown_metrics.py 20/20, test_perf_submit.py 101 passed and 1 skipped (needs torch), test_cache_transceiver_precheck_config.py 60/60, and test_perf_regression_branch.py 19/19 as an unaffected control. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: chenfeiz0326 <203214996+chenfeiz0326@users.noreply.github.com>
Description
perf/test_perf_sanity.py::test_e2e[aggr_upload-ctx_only-gb300_deepseek-v4-pro-fp4_8k1k_con4301_ctx12_dep4_gen1_dep8_eplb384_mtp1_ccb-NIXL]is waived under nvbugs/6682113 because it exceeds its 90-minute CI budget. It is
not failing: it completes all 43,010 requests successfully, every rep. It is
simply given 20% less wall clock than the same workload gets elsewhere in the
same test family.
This PR raises that one row's budget from
TIMEOUT (90)toTIMEOUT (120)andremoves its waiver.
Why 120 is the right number, and not a new precedent
The identical workload already runs with a 120-minute budget in its two sibling
rows. All three con4301 rows resolve to the same config file
(
tests/scripts/perf-sanity/disaggregated/gb300_deepseek-v4-pro-fp4_8k1k_con4301_ctx12_dep4_gen1_dep8_eplb384_mtp1_ccb-NIXL.yaml,concurrency_list: '4301',multi_round: 10→ 43,010 requests):disagg_upload-e2e-...con4301...l0_gb300_multi_nodes_perf_sanity_ctx12_node1_gpu4_gen1_node2_gpu8.yml:19TIMEOUT (120)disagg_upload-gen_only-...con4301...l0_gb300_multi_nodes_perf_sanity_ctx12_node1_gpu4_gen1_node2_gpu8.yml:18TIMEOUT (120)aggr_upload-ctx_only-...con4301...l0_gb300_multi_gpus_perf_sanity.yml:28TIMEOUT (90)So this change aligns the outlier with the budget its own workload already
carries, rather than inventing a looser one.
Evidence: the case passes, and the wall time is legitimate
Re-run on real GB300 hardware (aws_cmh), 3 reps:
Every rep:
1 passed, 43010/43010 successful requests, 0 failed, weight load2327/2327, no qualified OOM signal, all Slurm stepsCOMPLETED. The case isfunctionally healthy; it misses only the clock.
The benchmark window is fully accounted for, so there is no hidden inefficiency
to fix instead:
43010 requests / 8.79 req/s = 4893.1 svs measured4894.03 s.host_step_timeover the run's 10,759 iterations is 4914.6 s against a4894 s window — ~99.8% of the window is iteration execution, with no idle
gap, bubble, or stall.
454.13-454.46 ms;
p99/p1 = 1.011). Only 3 of 10,759 iterations exceed 600 msand 2 of those are warmup. Cost is exactly linear in request count — there is no
scaling defect.
layer_updates_per_iter: 0,num_slots: 384), so there is no per-iteration expert-map work to remove.con8ctx_onlyrow on the same
dep4shape — this case is 46% faster per token, it is just23.9x larger.
Why the budget cannot be met by trimming startup
Startup is 1075 s, measured (rep 1):
Only the autotuner (325 s) and FMHA JIT (41 s) are plausibly reducible, totalling
366 s. Subtracting both entirely — which is neither achievable nor desirable,
since it would corrupt the measurement — still leaves:
3/3 reps miss the 90-minute budget even under that maximally generous assumption.
The 530 s weight load is irreducible I/O on an 805 GiB checkpoint.
test_perf_sanity.pyalready documents this case's exceptional startup in the readiness-timeout comment
("gb300 DeepSeek-V4-Pro ctx_only (con4301) needs ~2000s of model load + autotune
before /health answers", nvbugs/6517846).
At 99.6 minutes worst-case, a 120-minute budget leaves ~20% margin (5975 s vs
7200 s).
Scope
preserved.
full:<PLATFORM>/prefix and the id isdeclared in exactly one test-db, so the deletion re-enables the case on GB300
alone.
Known follow-up, not addressed here
The 90-vs-120 split appears to be assigned per test-db file rather than per
workload. All three
aggr_upload-ctx_onlyrows inl0_gb300_multi_gpus_perf_sanity.ymlcarryTIMEOUT (90)despite spanning a~24x workload range (con180 → 1,800 requests, con666 → 6,660, con4301 → 43,010),
while every
disaggrow in the same family carries 120. Only the con4301 rowcurrently overruns, so this PR fixes just that row. The broader budget-assignment
question belongs with nvbugs/6682328 / 6683835 and is left to their owner.
Note the con180 / con666 figures above are predictions from this case's measured
constants (113.79 ms/request, 1075 s startup), not measurements — no artifacts
were collected for those two rows.
PR Checklist
git commit -s)Dev Engineer Review
aggr_upload-ctx_onlycon4301 perf-sanity entry fromTIMEOUT (90)toTIMEOUT (120).waives.txt.QA Engineer Review
tests/integration/test_lists/test-db/l0_gb300_multi_gpus_perf_sanity.ymlby increasing the test timeout.tests/integration/test_lists/waives.txtby removing the matching waiver.