diff --git a/tests/integration/defs/perf/README_test_perf_sanity.md b/tests/integration/defs/perf/README_test_perf_sanity.md index 144d24f6d795..25c07594452c 100644 --- a/tests/integration/defs/perf/README_test_perf_sanity.md +++ b/tests/integration/defs/perf/README_test_perf_sanity.md @@ -63,10 +63,59 @@ If the mean cannot be parsed for a `gen_only` run, `check_test_failure` raises ` ### Match Keys -Match keys differ by deployment mode: - -- **Aggregated**: `["s_gpu_type", "s_runtime"]` + `ServerConfig.to_match_keys()` + `ClientConfig.to_match_keys()` -- **Disaggregated**: `["s_gpu_type", "s_runtime", "s_benchmark_mode", "l_num_ctx_servers", "l_num_gen_servers"]` + prefixed ctx/gen `ServerConfig.to_match_keys()` + `ClientConfig.to_match_keys()` +Match keys are the fields that identify a test case when looking up its history to +build a baseline. They are the same for every deployment mode +(`get_test_case_match_keys()` in `tests/test_common/perf_sanity_matching.py`): + +| Key | Why | +|-----|-----| +| `s_test_case_name` | `-` for aggregated, `--` for disaggregated. Already encodes every fixed parameter of the case: model, parallelism, ISL/OSL, concurrency. | +| `s_gpu_type` | The same case name runs on more than one GPU type. | +| `s_runtime` | The same case name runs on both `aggr_server` and `multi_node_aggr_server`. | +| `s_branch` | Release branches keep their own baseline rather than blending into `main`'s. | + +A case is identified by its **name**, not by the config values it happens to run +with today. Server/client config fields such as `l_max_batch_size`, +`s_kv_cache_dtype`, `s_spec_decoding_type`, `l_force_num_accepted_tokens` or +`b_streaming` are *tunables* — they are adjusted to improve perf on the same test. +Keying on them forked the case and reset its baseline, hiding the very effect the +tuning was meant to have. They are still recorded on each document for filtering +and analysis; they just do not participate in matching. + +**A value that feeds the case name still forks the case.** Dropping a field from +the match key only stops it forking if the name does not encode it, and +`l_iterations` is the one field where that distinction bites: + +- Aggregated configs all set an explicit client `name:` (190 of 190 blocks in + `tests/scripts/perf-sanity/`), so changing `iterations` there keeps the case and + its history. 93 of those names contain a stale `iter` for that reason. +- Disagg configs set no client name, so `ClientConfig` derives + `con_iter_isl_osl`. `benchmark.multi_round` becomes `iterations`, + which lands in that name and therefore in `s_test_case_name` — so changing + `multi_round` renames the case, and the renamed case starts with no history. + +That is intended rather than a gap to close: `iterations` sets how long the +measurement runs, and a longer run amortizes warmup differently, so `iter10` and +`iter12` do not measure the same quantity and should not share a baseline. It is +also not a regression — `l_iterations` was previously a match key in its own +right, so it forked the case before this change too. What matters is that renaming +a case silently costs it its history and its next pre-merge regression check, so +`multi_round` should be treated as a workload parameter, not a knob to sweep. + +`s_benchmark_mode` is deliberately excluded: it is null on every aggregated record +and exactly equals the test case name's prefix on every disaggregated one, so it +adds no information while breaking matching against records written before the +field existed (`benchmark_data_matches` treats `None` and `"e2e"` as different). + +`match_mode: scenario` in the server yamls is now inert — not forking a case on a +config change is the default for every case. + +**Pre-merge branch substitution.** Since `s_branch` is a match key and history +queries are restricted to post-merge documents, a pre-merge run (whose `s_branch` +is `github-pr-`) would match nothing and its regression check would silently +pass. `process_and_upload_test_results` therefore looks history up against a +baseline branch — `PERF_BASELINE_BRANCH`, default `main` — using a lookup-only copy +of the data. The uploaded documents keep their true `s_branch`. ## Overview diff --git a/tests/integration/defs/perf/perf_regression_utils.py b/tests/integration/defs/perf/perf_regression_utils.py index bf69d0fb0a13..3f5a4e98e54a 100644 --- a/tests/integration/defs/perf/perf_regression_utils.py +++ b/tests/integration/defs/perf/perf_regression_utils.py @@ -1,4 +1,4 @@ -# SPDX-FileCopyrightText: Copyright (c) 2022-2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-FileCopyrightText: Copyright (c) 2022-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -494,15 +494,24 @@ def process_and_upload_test_results( data.update(extra_fields) add_id(data) - # Step 3: Find common values to narrow query scope - common_values_dict = get_common_values(new_data_dict, match_keys) + # Step 3: For pre-merge, look history up against the baseline branch + lookup_data_dict = new_data_dict + if not is_post_merge and "s_branch" in match_keys: + baseline_branch = os.environ.get("PERF_BASELINE_BRANCH", "main") + lookup_data_dict = { + cmd_idx: {**data, "s_branch": baseline_branch} + for cmd_idx, data in new_data_dict.items() + } - # Step 4: Query history data + # Step 4: Find common values to narrow query scope + common_values_dict = get_common_values(lookup_data_dict, match_keys) + + # Step 5: Query history data latest_history_data_dict, latest_baseline_threshold_dict, history_data_dict = get_history_data( - new_data_dict, match_keys, common_values_dict + lookup_data_dict, match_keys, common_values_dict ) - # Step 5: Compute regression info + # Step 6: Compute regression info prepare_regressive_test_cases( latest_history_data_dict, latest_baseline_threshold_dict, @@ -513,17 +522,17 @@ def process_and_upload_test_results( regression_metrics, ) - # Step 6: For post-merge, embed baseline fields + # Step 7: For post-merge, embed baseline fields if is_post_merge: add_baseline_fields_to_post_merge_data( latest_baseline_threshold_dict, new_data_dict, maximize_metrics, minimize_metrics ) - # Step 7: Upload to DB + # Step 8: Upload to DB if upload_to_db: post_new_perf_data(new_data_dict) - # Step 8: Check regression (auto-detect fail behavior if not specified) + # Step 9: Check regression (auto-detect fail behavior if not specified) if fail_on_regression is None: fail_on_regression = not is_post_merge check_perf_regression(new_data_dict, fail_on_regression=fail_on_regression) diff --git a/tests/integration/defs/perf/test_perf_sanity.py b/tests/integration/defs/perf/test_perf_sanity.py index 84e76a1aba95..6fe156d597ea 100644 --- a/tests/integration/defs/perf/test_perf_sanity.py +++ b/tests/integration/defs/perf/test_perf_sanity.py @@ -31,7 +31,7 @@ import yaml from test_common.error_utils import report_error from test_common.http_utils import fail_if_proc_died, wait_for_endpoint_ready -from test_common.perf_sanity_matching import get_client_match_keys, get_server_match_keys +from test_common.perf_sanity_matching import get_test_case_match_keys from defs.common import wait_for_reported_addr from defs.trt_test_alternative import print_info @@ -841,9 +841,6 @@ def to_cmd( def to_env(self) -> Dict[str, str]: return to_env_dict(self.env_vars) - def to_match_keys(self) -> List[str]: - return get_server_match_keys(self.match_mode) - def to_db_data(self) -> dict: """Convert ServerConfig to database data.""" db_data = { @@ -1220,9 +1217,6 @@ def _to_default_benchmark_cmd(self) -> List[str]: def to_env(self) -> Dict[str, str]: return to_env_dict(self.env_vars) - def to_match_keys(self) -> List[str]: - return get_client_match_keys() - def to_db_data(self) -> dict: """Convert ClientConfig to database data.""" # b_eos retained for baseline-matching continuity. spec-decoding runs are now @@ -2608,13 +2602,10 @@ def add_prefix(key: str, prefix_name: str) -> str: rest = key[2:] return f"{type_prefix}{prefix_name}_{rest}" - def add_list_prefix(config_list: List, prefix_name: str) -> List: - return [add_prefix(key, prefix_name) for key in config_list] - def add_dict_prefix(config_dict: dict, prefix_name: str) -> dict: return {add_prefix(key, prefix_name): value for key, value in config_dict.items()} - match_keys = [] + match_keys = get_test_case_match_keys() if self.runtime == "aggr_server": new_data_dict = {} @@ -2659,11 +2650,6 @@ def add_dict_prefix(config_dict: dict, prefix_name: str) -> dict: new_data_dict[cmd_idx] = new_data cmd_idx += 1 - if not match_keys: - match_keys.extend(["s_gpu_type", "s_runtime"]) - match_keys.extend(server_config.to_match_keys()) - match_keys.extend(client_config.to_match_keys()) - elif self.runtime == "multi_node_disagg_server": # Only BENCHMARK node uploads if self.server_configs[0][2].disagg_serving_type != "BENCHMARK": @@ -2725,28 +2711,6 @@ def add_dict_prefix(config_dict: dict, prefix_name: str) -> dict: new_data_dict[cmd_idx] = new_data cmd_idx += 1 - if not match_keys: - match_keys.extend( - [ - "s_gpu_type", - "s_runtime", - "s_benchmark_mode", - "l_num_ctx_servers", - "l_num_gen_servers", - ] - ) - if num_ctx_servers > 0: - match_keys.extend(add_list_prefix(ctx_config.to_match_keys(), "ctx")) - if num_gen_servers > 0: - gen_match_keys = add_list_prefix(gen_config.to_match_keys(), "gen") - if disagg_config.benchmark_mode == "gen_only": - gen_match_keys = [ - k - for k in gen_match_keys - if k != "gen_s_cache_transceiver_backend" - ] - match_keys.extend(gen_match_keys) - match_keys.extend(client_config.to_match_keys()) else: return diff --git a/tests/test_common/perf_sanity_matching.py b/tests/test_common/perf_sanity_matching.py index 15de45c6cd76..0d2eb34fd730 100644 --- a/tests/test_common/perf_sanity_matching.py +++ b/tests/test_common/perf_sanity_matching.py @@ -17,53 +17,17 @@ from collections.abc import Iterable, Mapping -_SCENARIO_SERVER_MATCH_KEYS = ( - "s_model_name", - "l_gpus", +_TEST_CASE_MATCH_KEYS = ( + "s_test_case_name", + "s_gpu_type", + "s_runtime", + "s_branch", ) -_CONFIG_SERVER_MATCH_KEYS = ( - "s_model_name", - "l_tp", - "l_ep", - "l_pp", - "l_cp", - "l_gpus_per_node", - "l_max_batch_size", - "b_enable_attention_dp", - "s_serving_backend", - "s_kv_cache_dtype", - "s_cache_transceiver_backend", - "s_spec_decoding_type", - "l_num_nextn_predict_layers", - "l_force_num_accepted_tokens", - "l_load_balancer_num_slots", -) - -_CLIENT_MATCH_KEYS = ( - "l_concurrency", - "l_iterations", - "l_isl", - "l_osl", - "d_random_range_ratio", - "s_backend", - "b_use_chat_template", - "b_streaming", - "b_use_nv_sa_benchmark", - "b_eos", -) - - -def get_server_match_keys(match_mode: str) -> list[str]: - """Return database fields that identify an equivalent server run.""" - if match_mode == "scenario": - return list(_SCENARIO_SERVER_MATCH_KEYS) - return list(_CONFIG_SERVER_MATCH_KEYS) - -def get_client_match_keys() -> list[str]: - """Return database fields that identify an equivalent client workload.""" - return list(_CLIENT_MATCH_KEYS) +def get_test_case_match_keys() -> list[str]: + """Return database fields that identify an equivalent perf test case.""" + return list(_TEST_CASE_MATCH_KEYS) def benchmark_data_matches( diff --git a/tests/unittest/others/test_perf_regression_branch.py b/tests/unittest/others/test_perf_regression_branch.py index ffd7f3dae6e0..e7e89e1adc63 100644 --- a/tests/unittest/others/test_perf_regression_branch.py +++ b/tests/unittest/others/test_perf_regression_branch.py @@ -12,12 +12,22 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -"""Tests that s_branch is read from globalVars, never scraped from the job URL. - -The Jenkins folder segment in a job URL (/job/LLM/job//) names the -folder the job definition lives in, which is "main" even for a release-branch -post-merge build. Recovering a branch from it therefore yields the constant -"main"; the pipeline publishes the real branch in globalVars["build_branch"]. +"""Tests for how perf_regression_utils handles s_branch. + +Two concerns, both of which fail open (a green run with no regression check) if +they break: + +1. s_branch is read from globalVars, never scraped from the job URL. The Jenkins + folder segment in a job URL (/job/LLM/job//) names the folder the job + definition lives in, which is "main" even for a release-branch post-merge + build. Recovering a branch from it therefore yields the constant "main"; the + pipeline publishes the real branch in globalVars["build_branch"]. + +2. A pre-merge run looks its history up against a baseline branch. s_branch is + part of the case identity, and get_history_data only ever returns post-merge + records, so a pre-merge run querying its own "github-pr-" branch matches + nothing and its regression check silently becomes a no-op. The queries must + see the baseline branch while the uploaded document keeps the real one. """ import importlib.util @@ -36,8 +46,10 @@ def _load_perf_regression_utils(): """Load the real module without importing the integration-test packages. ``defs/__init__.py`` pre-imports ``torch._inductor`` and - ``open_search_db_utils`` pulls in the OpenSearch client. ``get_job_info`` - reaches neither, and requiring them would turn this into a GPU-image test. + ``open_search_db_utils`` pulls in the OpenSearch client. Neither + ``get_job_info`` nor the branch routing in + ``process_and_upload_test_results`` reaches them, and requiring them would + turn this into a GPU-image test. """ defs_pkg = types.ModuleType("defs") defs_pkg.__path__ = [] @@ -73,7 +85,8 @@ def _load_perf_regression_utils(): return module -get_job_info = _load_perf_regression_utils().get_job_info +_perf_regression_utils = _load_perf_regression_utils() +get_job_info = _perf_regression_utils.get_job_info # A real post-merge build of release/1.3.0rc22.post1, whose job URL still says # ".../job/LLM/job/main/job/L0_PostMerge/...". @@ -82,7 +95,14 @@ def _load_perf_regression_utils(): _GITHUB_PR_JOB_URL = f"{_JENKINS}/job/LLM/job/main/job/L0_MergeRequest_PR/1234/" _GITLAB_MR_JOB_URL = f"{_JENKINS}/job/LLM/job/main/job/L0_MergeRequest/77/" -_INHERITED_CI_VARS = ("globalVars", "gitlabCommit", "BUILD_ID", "BUILD_URL", "JOB_NAME") +_INHERITED_CI_VARS = ( + "globalVars", + "gitlabCommit", + "BUILD_ID", + "BUILD_URL", + "JOB_NAME", + "PERF_BASELINE_BRANCH", +) @pytest.fixture(autouse=True) @@ -176,3 +196,140 @@ def test_unparsable_global_vars_does_not_raise(monkeypatch): info = get_job_info() assert info["s_branch"] == "" assert info["s_job_url"] == "" + + +# --------------------------------------------------------------------------- # +# Pre-merge history lookup routes through a baseline branch +# --------------------------------------------------------------------------- # + +_MATCH_KEYS = ["s_test_case_name", "s_gpu_type", "s_runtime", "s_branch"] + + +def _new_data_dict() -> dict[int, dict[str, object]]: + return { + 0: { + "s_test_case_name": "example_model_fp8_tp8-con32_iter10_1k1k", + "s_gpu_type": "b200", + "s_runtime": "aggr_server", + "d_output_token_throughput": 1234.5, + } + } + + +def _run_pipeline( + monkeypatch: pytest.MonkeyPatch, + build_branch: str, + job_url: str, + match_keys: list[str] | None = None, +) -> dict[str, dict[int, str]]: + """Run the real pipeline, recording the s_branch each seam observes. + + Only the three OpenSearch seams are replaced. Everything between them -- + get_job_info, the enrichment loop, the branch routing, the regression pass -- + is the production code path, so a regression in the wiring shows up here. + """ + observed: dict[str, dict[int, str]] = {} + + def fake_get_common_values( + data_dict: dict[int, dict[str, object]], keys: list[str] + ) -> dict[str, object]: + observed["common_values"] = {idx: d["s_branch"] for idx, d in data_dict.items()} + return {} + + def fake_get_history_data( + data_dict: dict[int, dict[str, object]], + keys: list[str], + common_values_dict: dict[str, object], + ) -> tuple[dict[int, object], dict[int, object], dict[int, object]]: + observed["history_query"] = {idx: d["s_branch"] for idx, d in data_dict.items()} + observed["history_query_names"] = { + idx: d["s_test_case_name"] for idx, d in data_dict.items() + } + return {}, {}, {} + + def fake_post_new_perf_data(data_dict: dict[int, dict[str, object]]) -> None: + observed["uploaded"] = {idx: d["s_branch"] for idx, d in data_dict.items()} + + monkeypatch.setattr(_perf_regression_utils, "get_common_values", fake_get_common_values) + monkeypatch.setattr(_perf_regression_utils, "get_history_data", fake_get_history_data) + monkeypatch.setattr(_perf_regression_utils, "post_new_perf_data", fake_post_new_perf_data) + + payload = { + "build_branch": build_branch, + "action_info": {"parents": [{"url": job_url, "build_number": "7"}]}, + } + monkeypatch.setenv("globalVars", json.dumps(payload)) + + new_data_dict = _new_data_dict() + _perf_regression_utils.process_and_upload_test_results( + new_data_dict, + match_keys if match_keys is not None else _MATCH_KEYS, + maximize_metrics=["d_output_token_throughput"], + minimize_metrics=[], + regression_metrics=["d_output_token_throughput"], + ) + observed["new_data_dict"] = {idx: d["s_branch"] for idx, d in new_data_dict.items()} + return observed + + +def test_pre_merge_history_is_queried_against_the_baseline_branch( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The queries see "main"; the uploaded document keeps "github-pr-". + + get_common_values must see the substituted branch too: it folds + single-valued match keys into the OpenSearch must-clause, so substituting + only for get_history_data would let the query filter the history away. + """ + observed = _run_pipeline(monkeypatch, "github-pr-18408", _GITHUB_PR_JOB_URL) + + assert observed["common_values"] == {0: "main"} + assert observed["history_query"] == {0: "main"} + assert observed["uploaded"] == {0: "github-pr-18408"} + assert observed["new_data_dict"] == {0: "github-pr-18408"} + + +def test_pre_merge_baseline_branch_is_configurable(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("PERF_BASELINE_BRANCH", "release/1.3.0rc22.post1") + observed = _run_pipeline(monkeypatch, "github-pr-18408", _GITHUB_PR_JOB_URL) + + assert observed["history_query"] == {0: "release/1.3.0rc22.post1"} + assert observed["uploaded"] == {0: "github-pr-18408"} + + +def test_substitution_only_replaces_the_branch(monkeypatch: pytest.MonkeyPatch) -> None: + """The lookup copy is the real data with one field changed, not a stub.""" + observed = _run_pipeline(monkeypatch, "github-pr-18408", _GITHUB_PR_JOB_URL) + + assert observed["history_query_names"] == {0: "example_model_fp8_tp8-con32_iter10_1k1k"} + + +def test_post_merge_history_is_queried_against_its_own_branch( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A release-branch post-merge run must not be rebased onto main.""" + observed = _run_pipeline(monkeypatch, "release/1.3.0rc22.post1", _RELEASE_BUILD_JOB_URL) + + assert observed["common_values"] == {0: "release/1.3.0rc22.post1"} + assert observed["history_query"] == {0: "release/1.3.0rc22.post1"} + assert observed["uploaded"] == {0: "release/1.3.0rc22.post1"} + + +def test_post_merge_ignores_the_baseline_branch_override(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("PERF_BASELINE_BRANCH", "main") + observed = _run_pipeline(monkeypatch, "release/1.3.0rc22.post1", _RELEASE_BUILD_JOB_URL) + + assert observed["history_query"] == {0: "release/1.3.0rc22.post1"} + + +def test_no_substitution_when_branch_is_not_a_match_key(monkeypatch: pytest.MonkeyPatch) -> None: + """Callers keying on other fields keep their own semantics untouched.""" + observed = _run_pipeline( + monkeypatch, + "github-pr-18408", + _GITHUB_PR_JOB_URL, + match_keys=["s_test_case_name", "s_gpu_type"], + ) + + assert observed["history_query"] == {0: "github-pr-18408"} + assert observed["uploaded"] == {0: "github-pr-18408"} diff --git a/tests/unittest/tools/test_perf_sanity_matching.py b/tests/unittest/tools/test_perf_sanity_matching.py index a6e9ab39d42f..84904cd225e2 100644 --- a/tests/unittest/tools/test_perf_sanity_matching.py +++ b/tests/unittest/tools/test_perf_sanity_matching.py @@ -13,17 +13,21 @@ # See the License for the specific language governing permissions and # limitations under the License. -from test_common.perf_sanity_matching import ( - benchmark_data_matches, - get_client_match_keys, - get_server_match_keys, -) +import ast +import importlib.util +import pathlib +import sys +import types + +from test_common.perf_sanity_matching import benchmark_data_matches, get_test_case_match_keys def _benchmark_data(**overrides: object) -> dict[str, object]: data: dict[str, object] = { + "s_test_case_name": "example_model_fp8_tp8-con32_iter10_1k1k", "s_gpu_type": "b200", "s_runtime": "aggr_server", + "s_branch": "main", "s_model_name": "example_model", "l_gpus": 8, "l_tp": 8, @@ -39,63 +43,239 @@ def _benchmark_data(**overrides: object) -> dict[str, object]: return data -def _benchmark_match_keys(match_mode: str) -> list[str]: - return [ +def test_match_keys_are_name_and_environment_only() -> None: + assert get_test_case_match_keys() == [ + "s_test_case_name", "s_gpu_type", "s_runtime", - *get_server_match_keys(match_mode), - *get_client_match_keys(), + "s_branch", ] -def test_scenario_matching_ignores_recipe_tuning_changes() -> None: +def test_matching_ignores_tuning_changes() -> None: + """Tunables do not fork a case *while the name is held constant*. + + That proviso is the whole story for l_iterations, which reaches the case name + on the disagg path -- see test_iterations_fork_a_case_through_the_derived_name. + """ previous_data = _benchmark_data() updated_data = _benchmark_data( - l_tp=4, - l_ep=2, l_max_batch_size=64, s_kv_cache_dtype="auto", + l_iterations=20, + l_force_num_accepted_tokens=3, ) - match_keys = _benchmark_match_keys("scenario") - assert get_server_match_keys("scenario") == ["s_model_name", "l_gpus"] - assert {"l_concurrency", "l_isl", "l_osl"}.issubset(match_keys) - assert benchmark_data_matches(previous_data, updated_data, match_keys) + assert benchmark_data_matches(previous_data, updated_data, get_test_case_match_keys()) -def test_config_matching_keeps_recipe_tuning_keys() -> None: - previous_data = _benchmark_data() - updated_data = _benchmark_data( - l_tp=4, - l_ep=2, - l_max_batch_size=64, - s_kv_cache_dtype="auto", +def _load_client_config() -> type: + """Return the real ClientConfig, without the integration-test packages. + + The derived-name rule is only worth testing against the class that owns it; + re-implementing the f-string here would assert nothing. test_perf_sanity.py + reaches torch and the OpenSearch client through its imports, so those are + stubbed -- ClientConfig.__init__ touches none of them. + """ + repo_root = pathlib.Path(__file__).resolve().parents[3] + module_path = repo_root / "tests" / "integration" / "defs" / "perf" / "test_perf_sanity.py" + + def stub(name: str, **attrs: object) -> types.ModuleType: + module = types.ModuleType(name) + for key, value in attrs.items(): + setattr(module, key, value) + return module + + def noop(*args: object, **kwargs: object) -> None: + return None + + defs_pkg = stub("defs") + defs_pkg.__path__ = [] + perf_pkg = stub("defs.perf") + perf_pkg.__path__ = [] + stubs = { + "defs": defs_pkg, + "defs.perf": perf_pkg, + "defs.common": stub("defs.common", wait_for_reported_addr=noop), + "defs.trt_test_alternative": stub( + "defs.trt_test_alternative", print_info=noop, print_warning=noop + ), + "defs.conftest": stub( + "defs.conftest", + get_llm_root=lambda *a, **k: "/repo", + llm_models_root=lambda *a, **k: "/models", + ), + "defs.perf._model_paths": stub("defs.perf._model_paths", MODEL_PATH_DICT={}), + "defs.perf.open_search_db_utils": stub( + "defs.perf.open_search_db_utils", + add_id=noop, + get_history_data=noop, + post_new_perf_data=noop, + ), + "defs.perf.perf_regression_utils": stub( + "defs.perf.perf_regression_utils", + process_and_upload_test_results=noop, + get_job_info=noop, + _percentile=noop, + ), + } + + saved = {name: sys.modules.get(name) for name in stubs} + sys.modules.update(stubs) + try: + spec = importlib.util.spec_from_file_location("defs.perf.test_perf_sanity", module_path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + finally: + for name, previous in saved.items(): + if previous is None: + sys.modules.pop(name, None) + else: + sys.modules[name] = previous + return module.ClientConfig + + +def _disagg_client_data(multi_round: int) -> dict[str, object]: + """The client dict a disagg config builds (test_perf_sanity.py:2266-2277). + + Notably it carries no "name", so ClientConfig derives one. + """ + return { + "concurrency": 12, + "iterations": multi_round, + "isl": 50000, + "osl": 2048, + } + + +def test_disagg_iterations_come_from_multi_round() -> None: + """Pin the yaml key that feeds "iterations" on the disagg path. + + _parse_disagg_config_file cannot be called here -- PerfSanityTestConfig's + constructor shells out to nvidia-smi and raises without a GPU -- so the + mapping is asserted against its source instead of re-stated in a docstring. + Without this, renaming the yaml key would leave the tests below green while + the documented behaviour silently changed. + """ + repo_root = pathlib.Path(__file__).resolve().parents[3] + module_path = repo_root / "tests" / "integration" / "defs" / "perf" / "test_perf_sanity.py" + tree = ast.parse(module_path.read_text()) + + iterations_values = [ + value + for node in ast.walk(tree) + if isinstance(node, ast.Dict) + for key, value in zip(node.keys, node.values) + if isinstance(key, ast.Constant) and key.value == "iterations" + ] + + assert iterations_values, 'no dict literal builds an "iterations" entry' + assert any("multi_round" in ast.dump(value) for value in iterations_values), ( + '"iterations" is no longer derived from benchmark.multi_round; ' + "update README_test_perf_sanity.md and the tests below" ) + +def test_iterations_fork_a_case_through_the_derived_name() -> None: + """benchmark.multi_round renames a disagg case, so it still forks history. + + multi_round becomes the client's "iterations" (test_perf_sanity.py:2270), which + lands in the derived name (:1119) and hence in s_test_case_name. Dropping + l_iterations from the match key therefore does not make it fork-free on this + path. Intended -- iterations sets the measurement length, so iter10 and iter12 + do not measure the same quantity -- but it costs the renamed case its history, + so it is pinned here rather than left as an assumption. + """ + client_config = _load_client_config() + + ten = client_config(_disagg_client_data(10), "example_model") + twelve = client_config(_disagg_client_data(12), "example_model") + + assert ten.name == "con12_iter10_isl50000_osl2048" + assert twelve.name == "con12_iter12_isl50000_osl2048" + + stem = "e2e-gb300_example_model_50k2k_con12_ctx1_dep4_gen6_tep4-" assert not benchmark_data_matches( - previous_data, - updated_data, - _benchmark_match_keys("config"), + _benchmark_data(s_test_case_name=stem + ten.name, l_iterations=10), + _benchmark_data(s_test_case_name=stem + twelve.name, l_iterations=12), + get_test_case_match_keys(), ) -def test_scenario_matching_distinguishes_gpu_count() -> None: - previous_data = _benchmark_data() - updated_data = _benchmark_data(l_gpus=4) +def test_an_explicit_client_name_keeps_the_case_across_an_iterations_change() -> None: + """Aggregated configs all name their clients, so tuning keeps the history. - assert not benchmark_data_matches( - previous_data, - updated_data, - _benchmark_match_keys("scenario"), + This is the path where removing l_iterations from the key pays off: the name is + pinned by the yaml, so a changed iterations count stays on one curve. + """ + client_config = _load_client_config() + + ten = client_config({**_disagg_client_data(10), "name": "con1024_iter10_1k1k"}, "example_model") + twenty = client_config( + {**_disagg_client_data(20), "name": "con1024_iter10_1k1k"}, "example_model" ) + assert ten.name == twenty.name == "con1024_iter10_1k1k" + assert benchmark_data_matches( + _benchmark_data(s_test_case_name="example_model_fp8_tp8-" + ten.name, l_iterations=10), + _benchmark_data(s_test_case_name="example_model_fp8_tp8-" + twenty.name, l_iterations=20), + get_test_case_match_keys(), + ) -def test_scenario_matching_distinguishes_client_workload() -> None: + +def test_matching_distinguishes_test_case_name() -> None: previous_data = _benchmark_data() - updated_data = _benchmark_data(l_concurrency=64) + updated_data = _benchmark_data(s_test_case_name="example_model_fp8_tp4-con32_iter10_1k1k") - assert not benchmark_data_matches( - previous_data, - updated_data, - _benchmark_match_keys("scenario"), + assert not benchmark_data_matches(previous_data, updated_data, get_test_case_match_keys()) + + +def test_matching_distinguishes_gpu_type() -> None: + previous_data = _benchmark_data() + updated_data = _benchmark_data(s_gpu_type="gb200") + + assert not benchmark_data_matches(previous_data, updated_data, get_test_case_match_keys()) + + +def test_matching_distinguishes_runtime() -> None: + previous_data = _benchmark_data() + updated_data = _benchmark_data(s_runtime="multi_node_aggr_server") + + assert not benchmark_data_matches(previous_data, updated_data, get_test_case_match_keys()) + + +def test_matching_distinguishes_branch() -> None: + previous_data = _benchmark_data() + updated_data = _benchmark_data(s_branch="release/1.3.0") + + assert not benchmark_data_matches(previous_data, updated_data, get_test_case_match_keys()) + + +def test_benchmark_mode_is_not_a_match_key() -> None: + assert "s_benchmark_mode" not in get_test_case_match_keys() + history = _benchmark_data( + s_test_case_name="e2e-example_disagg-con32_iter10_1k1k", + s_runtime="multi_node_disagg_server", + s_benchmark_mode=None, ) + new = _benchmark_data( + s_test_case_name="e2e-example_disagg-con32_iter10_1k1k", + s_runtime="multi_node_disagg_server", + s_benchmark_mode="e2e", + ) + + assert benchmark_data_matches(history, new, get_test_case_match_keys()) + + +def test_a_pre_merge_branch_does_not_match_post_merge_history() -> None: + """Branch is identity, so a PR run cannot match main's history unaided. + + This is the precondition that makes the baseline-branch substitution in + process_and_upload_test_results necessary; the substitution itself is tested + against that function in + tests/unittest/others/test_perf_regression_branch.py. + """ + history = _benchmark_data(s_branch="main") + pre_merge_data = _benchmark_data(s_branch="github-pr-12345") + + assert not benchmark_data_matches(history, pre_merge_data, get_test_case_match_keys())