[None][test] Enable overlap scheduler in the remaining perf sanity configs - #16991
[None][test] Enable overlap scheduler in the remaining perf sanity configs#16991hyukn wants to merge 1 commit into
Conversation
WalkthroughThe changes preserve configured overlap-scheduler settings during context-only parsing and enable overlap scheduling in three PyTorch or Dynamo performance configurations. ChangesOverlap scheduler configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/scripts/perf-sanity/disaggregated/gb200_qwen3-235b-fp4_8k1k_con1024_ctx1_tp1_gen1_dep8_eplb0_mtp0_ccb-NIXL.yaml (1)
1-8: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd the required NVIDIA copyright header.
This modified file starts directly with
metadata:and has no NVIDIA copyright header. Add the header and update its copyright year before merging. As per coding guidelines, “New files must include the NVIDIA copyright header; modified files must have the copyright year updated.”🤖 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/scripts/perf-sanity/disaggregated/gb200_qwen3-235b-fp4_8k1k_con1024_ctx1_tp1_gen1_dep8_eplb0_mtp0_ccb-NIXL.yaml` around lines 1 - 8, Add the standard NVIDIA copyright header at the beginning of the YAML file before metadata, using the current copyright year as required for modified files. Preserve the existing metadata unchanged.Source: Coding guidelines
🤖 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
`@tests/scripts/perf-sanity/disaggregated/gb200_qwen3-235b-fp4_8k1k_con1024_ctx1_tp1_gen1_dep8_eplb0_mtp0_ccb-NIXL.yaml`:
- Around line 1-8: Add the standard NVIDIA copyright header at the beginning of
the YAML file before metadata, using the current copyright year as required for
modified files. Preserve the existing metadata unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c0571265-93f8-4085-a9d0-b1a31086b19f
📒 Files selected for processing (48)
tests/scripts/perf-sanity/disaggregated/b200_deepseek-r1-fp4_8k1k_con1536_ctx1_dep4_gen1_dep8_eplb0_mtp1_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/b200_deepseek-r1-fp4_8k1k_con1_ctx1_dep4_gen1_tep8_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/b200_deepseek-r1-fp4_8k1k_con256_ctx1_dep4_gen1_dep8_eplb0_mtp1_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_deepseek-r1-fp4_128k8k_con128_ctx1_pp8_gen1_dep16_eplb0_mtp1_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_deepseek-r1-fp4_128k8k_con1_ctx1_pp8_gen1_tep8_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_deepseek-r1-fp4_8k1k_con1024_ctx1_dep4_gen1_dep32_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_deepseek-r1-fp4_8k1k_con1_ctx1_dep4_gen1_tep8_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_deepseek-r1-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp1_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_deepseek-v32-fp4_32k4k_con1_ctx1_dep4_gen1_tep8_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_deepseek-v32-fp4_32k4k_con2048_ctx1_dep4_gen1_dep32_eplb288_mtp1_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_deepseek-v32-fp4_32k4k_con256_ctx1_dep4_gen1_dep32_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_deepseek-v32-fp4_32k4k_con256_ctx1_dep8_gen1_dep8_eplb0_mtp0_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_deepseek-v32-fp4_8k1k_con1024_ctx1_dep4_gen1_dep32_eplb256_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_deepseek-v32-fp4_8k1k_con1_ctx1_dep4_gen1_tep8_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_deepseek-v32-fp4_8k1k_con4096_ctx1_dep4_gen1_dep32_eplb256_mtp0_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_gpt-oss-120b-fp4_8k1k_con1024_ctx1_tp1_gen1_tp4_eplb0_mtp0_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_gpt-oss-120b-fp4_8k1k_con128_ctx1_tp1_gen1_tp4_eplb0_mtp0_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_gpt-oss-120b-fp4_8k1k_con4_ctx1_tp1_gen1_tp4_eplb0_mtp0_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_gpt-oss-120b-fp4_8k1k_con512_ctx1_tp1_gen1_dep2_eplb0_mtp0_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_kimi-k25-thinking-fp4_8k1k_con1024_ctx1_dep4_gen1_dep32_eplb416_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_kimi-k25-thinking-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp0_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_kimi-k25-thinking-fp4_8k1k_con4_ctx1_dep4_gen1_tep8_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_qwen3-235b-fp4_8k1k_con1024_ctx1_tp1_gen1_dep8_eplb0_mtp0_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_qwen3-235b-fp4_8k1k_con1_ctx1_tp1_gen1_tep4_eplb0_mtp0_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb200_qwen3-235b-fp4_8k1k_con64_ctx1_tp1_gen1_tep4_eplb0_mtp0_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_128k8k_con1_ctx1_pp4_gen1_tep8_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_128k8k_con256_ctx1_pp4_gen1_dep8_eplb0_mtp1_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_128k8k_con64_ctx1_pp4_gen1_dep16_eplb0_mtp1_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_8k1k_con1024_ctx1_dep4_gen1_dep32_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_8k1k_con1_ctx1_dep4_gen1_tep8_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp1_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-v32-fp4_32k4k_con1_ctx1_dep4_gen1_tep8_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-v32-fp4_32k4k_con2048_ctx1_dep4_gen1_dep32_eplb288_mtp1_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-v32-fp4_32k4k_con256_ctx1_dep4_gen1_dep32_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-v32-fp4_32k4k_con256_ctx1_dep8_gen1_dep8_eplb0_mtp0_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-v32-fp4_8k1k_con1024_ctx1_dep4_gen1_dep32_eplb256_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-v32-fp4_8k1k_con1_ctx1_dep4_gen1_tep8_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-v32-fp4_8k1k_con4096_ctx1_dep4_gen1_dep32_eplb256_mtp0_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-v4-pro-fp4_8k1k_con180_ctx3_dep4_gen1_dep32_eplb384_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-v4-pro-fp4_8k1k_con4301_ctx12_dep4_gen1_dep8_eplb384_mtp1_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-v4-pro-fp4_8k1k_con666_ctx6_dep4_gen1_dep16_eplb384_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_deepseek-v4-pro-fp4_8k1k_con8_ctx1_dep4_gen4_tep8_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_glm-5-fp4_8k1k_con1024_ctx1_dep2_gen1_dep8_eplb256_mtp1_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_glm-5-fp4_8k1k_con1_ctx1_dep2_gen1_tep8_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_glm-5-fp4_8k1k_con512_ctx1_dep2_gen1_dep32_eplb0_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_kimi-k25-thinking-fp4_8k1k_con1024_ctx1_dep4_gen1_dep32_eplb416_mtp3_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_kimi-k25-thinking-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp0_ccb-NIXL.yamltests/scripts/perf-sanity/disaggregated/gb300_kimi-k25-thinking-fp4_8k1k_con4_ctx1_dep4_gen1_tep8_eplb0_mtp3_ccb-NIXL.yaml
| transceiver_runtime: PYTHON | ||
| kv_transfer_timeout_ms: 600000 | ||
| disable_overlap_scheduler: true | ||
| disable_overlap_scheduler: false |
There was a problem hiding this comment.
Do you think we can just remove this for it's false by default?
…nfigs NVIDIA#17390 re-enabled the overlap scheduler across the disaggregated perf sanity YAMLs in both tests/scripts/perf-sanity/ and tests/scripts/perf/, which supersedes the config-only change this branch originally carried. Four places outside those YAMLs were missed and still force the scheduler off, so the cases they drive keep measuring a runtime path nothing else exercises any more. Aggregated bench configs (pytorch_model_config.py): - deepseek_v4_pro_dspark - gpt_oss_120b_eagle3_throughput-bench-pytorch, whose latency sibling gpt_oss_120b_eagle3-bench-pytorch already runs with the scheduler enabled, so the two variants disagreed. Prefill side (test_perf_sanity.py): the ctx_only benchmark mode overwrote the flag with True after reading the ctx worker config, which silently defeated the ctx entries the YAML flip had just updated. Drop the override so the ctx worker config decides. ServerConfig already defaults the flag to False when it is absent, so configs that omit it are unchanged and a config that deliberately sets it keeps its setting instead of being overridden. Aggregated perf sanity YAML: the Dynamo-replica DeepSeek-V3.2 config was the last file under tests/scripts/perf-sanity/ still pinned to true. It mirrors an upstream Dynamo recipe, but the flag is a TRT-LLM runtime knob rather than part of the model or accuracy setup it replicates. Perf impact: these cases will re-baseline in OpenSearch, as the disaggregated cases did after NVIDIA#17390. The affected stages do not gate on perf regression, so no thresholds change. Signed-off-by: Yukun He <23156053+hyukn@users.noreply.github.com> Co-Authored-By: Claude <noreply@anthropic.com>
acedee3 to
856d233
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_sanity.py`:
- Around line 1934-1936: Add focused parser tests for ctx_only that assert
server_configs[0].disable_overlap_scheduler remains True when explicitly
enabled, remains False when explicitly disabled, and defaults to False when
omitted; then register all three tests in
tests/integration/test_lists/test-db/l0_b200_multi_gpus_perf_sanity.yml.
🪄 Autofix
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: fe62eda3-3801-47a4-b99e-c606b6567a30
📒 Files selected for processing (3)
tests/integration/defs/perf/pytorch_model_config.pytests/integration/defs/perf/test_perf_sanity.pytests/scripts/perf-sanity/aggregated/dynamo_deepseek_v32_fp4_2_nodes_grace_blackwell.yaml
| # The overlap scheduler setting is taken from the ctx worker config | ||
| # so that ctx_only measures the same scheduler path the ctx worker | ||
| # uses in e2e. ServerConfig defaults it to False when unset. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' "Relevant parser and test references:"
rg -n -C 4 \
'ctx_only|disable_overlap_scheduler|PerfSanityTestConfig' \
tests/integration --glob '*.py' --glob '*.yaml' || true
printf '%s\n' "Relevant test-list references:"
rg -n -C 2 \
'perf_sanity|ctx_only|disable_overlap_scheduler' \
tests/integration/test_lists || trueRepository: NVIDIA/TensorRT-LLM
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' 'Changed files:'
git diff --name-only
printf '%s\n' 'Parser implementation:'
sed -n '1860,2010p' tests/integration/defs/perf/test_perf_sanity.py
printf '%s\n' 'Perf sanity test functions and parser assertions:'
rg -n -C 3 \
'^(def|async def) test_|parse_config_file|server_configs\[0\]|disable_overlap_scheduler' \
tests/integration/defs/perf tests/integration/test_lists \
--glob 'test_perf_sanity.py' --glob '*.py' --glob '*.yaml' \
| head -n 400
printf '%s\n' 'Perf sanity test-list files:'
git ls-files tests/integration/test_lists | rg -i 'perf|sanity|test-db|qa' | head -n 200Repository: NVIDIA/TensorRT-LLM
Length of output: 29319
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' 'Perf sanity test-list entries:'
rg -n -i -C 3 \
'test_perf_sanity|perf_sanity|ctx_only' \
tests/integration/test_lists/qa/llm_perf_core.yml \
tests/integration/test_lists/qa/llm_perf_disagg.yml \
tests/integration/test_lists/test-db/l0_b200_perf_sanity.yml \
tests/integration/test_lists/test-db/l0_b200_multi_gpus_perf_sanity.yml \
tests/integration/test_lists/test-db/l0_gb200_multi_gpus_perf_sanity.yml \
tests/integration/test_lists/test-db/l0_gb300_multi_gpus_perf_sanity.yml \
tests/integration/test_lists/test-db/l0_perf.yml \
2>/dev/null || true
printf '%s\n' 'All standalone tests targeting PerfSanityTestConfig:'
rg -n -i -C 4 \
'PerfSanityTestConfig|_parse_disagg_config_file|ctx_only' \
tests --glob '*.py' --glob '!tests/integration/defs/perf/test_perf_sanity.py' \
| head -n 300
printf '%s\n' 'ServerConfig initialization and scheduler field:'
sed -n '450,510p' tests/integration/defs/perf/test_perf_sanity.pyRepository: NVIDIA/TensorRT-LLM
Length of output: 37701
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
from pathlib import Path
import ast
path = Path("tests/integration/defs/perf/test_perf_sanity.py")
source = path.read_text(encoding="utf-8")
tree = ast.parse(source)
server_default = None
ctx_only_branch = None
for node in ast.walk(tree):
if isinstance(node, ast.Assign):
target_names = [
target.id for target in node.targets if isinstance(target, ast.Name)
]
if "self.disable_overlap_scheduler" in ast.unparse(node):
server_default = ast.unparse(node)
if isinstance(node, ast.If) and "benchmark_mode == \"ctx_only\"" in ast.unparse(node):
text = ast.unparse(node)
if "ctx_config = dict(worker_config.get(\"ctx\", {}))" in text:
ctx_only_branch = text
assert server_default == (
'self.disable_overlap_scheduler = '
'server_config_data.get("disable_overlap_scheduler", False)'
)
assert ctx_only_branch is not None
assert '**ctx_config' in ctx_only_branch
def parsed_value(ctx_config):
merged = {
"concurrency": -1,
"model_name": "model",
"disagg_run_type": "aggr",
**dict(ctx_config),
}
return merged.get("disable_overlap_scheduler", False)
cases = [
({"disable_overlap_scheduler": True}, True),
({"disable_overlap_scheduler": False}, False),
({}, False),
]
for config, expected in cases:
actual = parsed_value(config)
assert actual is expected, (config, actual, expected)
print("ctx_only merge preserves explicit True and False and defaults omitted key to False")
print("No focused parser test function is present in the repository search results")
PYRepository: NVIDIA/TensorRT-LLM
Length of output: 246
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
from pathlib import Path
import ast
path = Path("tests/integration/defs/perf/test_perf_sanity.py")
tree = ast.parse(path.read_text(encoding="utf-8"))
print("Matching scheduler assignments:")
for node in ast.walk(tree):
if isinstance(node, ast.Assign) and "disable_overlap_scheduler" in ast.unparse(node):
print(ast.unparse(node))
print("ctx_only conditionals:")
for node in ast.walk(tree):
if isinstance(node, ast.If) and "ctx_only" in ast.unparse(node):
text = ast.unparse(node)
print(text[:1200].replace("\n", " "))
print("Relevant source lines:")
for number, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1):
if 1928 <= number <= 1953 or number == 494:
print(f"{number}: {line}")
PYRepository: NVIDIA/TensorRT-LLM
Length of output: 7540
Add focused ctx_only parser regression coverage.
The parser preserves explicit True and False values and defaults an omitted key to False. No focused test asserts server_configs[0].disable_overlap_scheduler. Add all three cases and list the test in tests/integration/test_lists/test-db/l0_b200_multi_gpus_perf_sanity.yml.
Test coverage verdict: insufficient.
🤖 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_sanity.py` around lines 1934 - 1936,
Add focused parser tests for ctx_only that assert
server_configs[0].disable_overlap_scheduler remains True when explicitly
enabled, remains False when explicitly disabled, and defaults to False when
omitted; then register all three tests in
tests/integration/test_lists/test-db/l0_b200_multi_gpus_perf_sanity.yml.
Source: Path instructions
BowenFu
left a comment
There was a problem hiding this comment.
The open CodeRabbit focused-parser-test request is non-blocking: this test-harness change preserves explicit values and follows the existing false default.
Verification1. Config-generation A/B (deterministic, runs the real
|
| check | before (main) | after |
|---|---|---|
ctx_only, non-QA DSv4-Pro config |
ctx True |
ctx False |
e2e (control) |
ctx False / gen False | unchanged |
gen_only (control) |
ctx False / gen False | unchanged |
ctx_only, a config that sets ctx: true |
True | stays True |
deepseek_v4_pro_dspark |
True | False |
gpt_oss_120b_eagle3_throughput-bench-pytorch |
True | False |
gpt_oss_120b_eagle3-bench-pytorch (control) |
False | False |
The fourth row is the important one for the ctx_only change: because the override is removed rather than set to False, a config that deliberately disables overlap keeps its setting instead of being clobbered in either direction.
2. On-cluster functional run — GB300, DeepSeek-V4-Pro
perf/test_perf_sanity.py::test_e2e[aggr-ctx_only-gb300_deepseek-v4-pro-fp4_8k1k_con8_ctx1_dep4_gen4_tep8_eplb0_mtp3_ccb-NIXL]
(1 node / 4 GPUs — in ctx_only only the ctx worker runs, aggregated, at dep4)
PASSED, tests=1 failures=0 errors=0 skipped=0, 1657.9 s.
The generated server config confirms the ctx worker actually ran with the overlap scheduler enabled — on main this same run emits true:
tensor_parallel_size: 4
moe_expert_parallel_size: 4
enable_attention_dp: true
disable_overlap_scheduler: false # <-- was hardcoded to true before this change
speculative_config:
decoding_type: MTP
max_draft_len: 3Benchmark result — real prefill work, no functional regression:
Total requests: 80
Successful requests: 80
Failed requests: 0
Total input tokens: 596480
Total Token throughput (tok/s): 57433.15
Zero tracebacks, CUDA errors, or assertions in the server log.
brnguyen2
left a comment
There was a problem hiding this comment.
Approving — the comments below are optional touch-ups, not blockers.
Verified against the tree: after this change nothing under tests/scripts/perf-sanity/ still sets disable_overlap_scheduler: true, and the four remaining true sites are all in tests/scripts/perf/ as the description says. Since these cases will re-baseline in OpenSearch, worth pinging whoever watches the perf dashboards before merge so the step change isn't read as a regression. The QA-tree stragglers listed in the description would be easier to chase with a tracking ticket than a PR comment.
| ctx_config.pop("cache_transceiver_config", None) | ||
| # Disable overlap scheduler for ctx_only | ||
| ctx_config["disable_overlap_scheduler"] = True | ||
| # The overlap scheduler setting is taken from the ctx worker config |
There was a problem hiding this comment.
The comment now explains code that isn't here, which reads oddly for someone landing on this block later. Consider folding it into the ctx_config line above, e.g. # ctx_config carries disable_overlap_scheduler through as-is (ServerConfig defaults it to False) so ctx_only exercises the same scheduler path as the e2e ctx worker.
Summary
PR #17390 ("Enable overlap schedule on qa side for disagg perf") re-enabled the overlap scheduler across the disaggregated perf sanity YAMLs. Despite its title it touched both trees — 64 files in
tests/scripts/perf/and 90 files intests/scripts/perf-sanity/— flipping 278 linestrue→falseon ctx and gen alike.That supersedes this PR's original diff entirely (all 48 files it changed are now already
falseon main), so this branch has been reset onto latest main and repurposed to close the four places #17390 missed. Those four sit outside the disagg YAMLs and still force the scheduler off, so the cases they drive keep measuring a runtime path nothing else exercises any more.Changes
tests/integration/defs/perf/pytorch_model_config.pydeepseek_v4_pro_dsparktests/integration/defs/perf/pytorch_model_config.pygpt_oss_120b_eagle3_throughput-bench-pytorchgpt_oss_120b_eagle3-bench-pytorchalready ran with the scheduler enabled — the two variants disagreed.tests/integration/defs/perf/test_perf_sanity.pyctx_onlymode overrideTrueafter reading the ctx worker config, silently defeating the ctx entries #17390 had just flipped.tests/scripts/perf-sanity/aggregated/dynamo_deepseek_v32_fp4_2_nodes_grace_blackwell.yamldisable_overlap_schedulertests/scripts/perf-sanity/still ontrue.On the
ctx_onlyoverrideThe override is removed rather than set to
False. Forcing it either way defeats the config file;ServerConfigalready defaults the flag toFalsewhen absent (test_perf_sanity.py:494), so:false(all non-QA disagg configs after [None][test] Enable overlap schedule on qa side for disagg perf #17390) — now actually run with overlap enabled inctx_only,true— keeps its setting instead of being clobbered.Reviewer notes
disable_overlap_scheduleris a TRT-LLM runtime knob rather than part of the model/accuracy setup being replicated — but if the intent is to track that recipe verbatim, say so and I'll drop this one line.tests/scripts/perf/still havectx: disable_overlap_scheduler: true. Four of them were touched by [None][test] Enable overlap schedule on qa side for disagg perf #17390 (its diff flipped theirgenbut missed theirctx); one (gb200_gpt-oss-120b-fp4_8k1k_con1024_...) it never touched. Listed for whoever owns the QA tree:gb200_deepseek-r1-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp1_ccb-NIXL.yamlgb200_gpt-oss-120b-fp4_8k1k_con1024_ctx1_tp1_gen1_tp4_eplb0_mtp0_ccb-NIXL.yamlgb300_deepseek-v32-fp4_8k1k_con4096_ctx1_dep4_gen1_dep32_eplb256_mtp0_ccb-NIXL.yamlgb300_glm-5-fp4_8k1k_con512_ctx1_dep2_gen1_dep32_eplb0_mtp3_ccb-NIXL.yamlgb300_kimi-k25-thinking-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp0_ccb-NIXL.yamlPerf impact
These cases will re-baseline in OpenSearch, exactly as the disaggregated cases did after #17390. The affected stages do not gate on perf regression, so no thresholds change.
Test Coverage
Local functional check on GB300 DeepSeek-V4-Pro disagg perf sanity configs (see comments).
PR Checklist
[JIRA/NVBugs/GitHub issue/None][type] Summarypre-commit runpasses on all changed filesDev Engineer Review
disable_overlap_scheduler: falsefor context and generation workers.ctx_onlyparsing. The setting now uses the context worker configuration and defaults toFalsethroughServerConfig.LlmArgsdefaults.QA Engineer Review
tests/integration/defs/perf/test_perf_sanity.py.ctx_onlyconfiguration parsing logic was modified. No test function was added, removed, or explicitly renamed.test-db/orqa/test-list entry was reported.