Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 53 additions & 4 deletions tests/integration/defs/perf/README_test_perf_sanity.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` | `<server_config.name>-<client_config.name>` for aggregated, `<e2e\|ctx_only\|gen_only>-<disagg stem>-<client_config.name>` 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<N>` for that reason.
- Disagg configs set no client name, so `ClientConfig` derives
`con<C>_iter<N>_isl<I>_osl<O>`. `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-<N>`) 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

Expand Down
27 changes: 18 additions & 9 deletions tests/integration/defs/perf/perf_regression_utils.py
Original file line number Diff line number Diff line change
@@ -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");
Expand Down Expand Up @@ -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:
Comment thread
chenfeiz0326 marked this conversation as resolved.
Resolved
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,
Expand All @@ -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)
40 changes: 2 additions & 38 deletions tests/integration/defs/perf/test_perf_sanity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Comment thread
chenfeiz0326 marked this conversation as resolved.
Resolved

if self.runtime == "aggr_server":
new_data_dict = {}
Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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

Expand Down
52 changes: 8 additions & 44 deletions tests/test_common/perf_sanity_matching.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
"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(
Expand Down
Loading
Loading