Repository navigation
[TRTLLM-11324][perf] Add host performance regression test suite for PyExecutor - #12148
Conversation
8fe6459 to
98f0cc6
Compare
|
/bot run --disable-fail-fast |
1 similar comment
|
/bot run --disable-fail-fast |
|
PR_Github #39155 [ run ] triggered by Bot. Commit: |
|
PR_Github #39155 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #39191 [ run ] triggered by Bot. Commit: |
|
PR_Github #39191 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
1 similar comment
|
/bot run --disable-fail-fast |
9534f66 to
b89052b
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #39336 [ run ] triggered by Bot. Commit: |
|
PR_Github #39337 [ run ] triggered by Bot. Commit: |
|
PR_Github #39336 [ run ] completed with state |
📝 WalkthroughWalkthroughIntroduces a comprehensive host performance regression testing framework for the PyExecutor pipeline, comprising configuration-driven E2E tests using trtllm-serve, LLM API-level benchmarks, and module-level performance tests for scheduler, sampler, and resource manager components. Includes test discovery, server/client orchestration, metrics extraction, multi-run statistical analysis, and configuration files for multiple model variants. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner as Test Runner
participant Discovery as Test Discovery
participant Config as Config Loader
participant Server as trtllm-serve
participant Client as benchmark_serving
participant Metrics as Metrics Parser
participant Output as Output/Logs
TestRunner->>Discovery: discover_test_cases()
Discovery->>Discovery: Scan YAML files
Discovery-->>TestRunner: [HostPerfTestCase, ...]
loop For each test case
TestRunner->>Config: load_configs(test_case)
Config-->>TestRunner: (HostPerfServerConfig, HostPerfClientConfig)
TestRunner->>Server: Generate extra-llm-api-config.yml
TestRunner->>Server: spawn trtllm-serve process
TestRunner->>Server: wait_for_server_ready(health check)
Server-->>TestRunner: Server is ready
TestRunner->>Client: invoke benchmark_serving
Client->>Server: Execute benchmark requests
Server->>Client: Return responses
Client-->>Output: Write metrics output
TestRunner->>Metrics: parse_metrics(output)
Metrics-->>TestRunner: Dict[metric_name, value]
TestRunner->>Output: Store results & logs
TestRunner->>Server: Terminate server process
end
opt Multi-run mode
TestRunner->>TestRunner: run_multi_run_benchmark(num_runs)
TestRunner->>TestRunner: report_multi_run_stats()
TestRunner-->>Output: JSON summary + CV analysis
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (9)
tests/integration/defs/perf/host_perf/test_module_sampler.py (3)
89-99: Use explicit| Nonefor optional list parameters.♻️ Suggested fix
def make_scheduled_requests( - generation_requests: List[LlmRequest] = None, - context_requests: List[LlmRequest] = None, + generation_requests: list[LlmRequest] | None = None, + context_requests: list[LlmRequest] | None = None, ) -> ScheduledRequests:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/defs/perf/host_perf/test_module_sampler.py` around lines 89 - 99, Update make_scheduled_requests signature to use explicit union type for optional lists (e.g., generation_requests: List[LlmRequest] | None = None and context_requests: List[LlmRequest] | None = None) so the parameters clearly indicate they can be None; keep the body unchanged and retain references to ScheduledRequests and LlmRequest used inside the function.
57-76: Use explicit| Nonefor optional parameters.The
stop_wordsparameter defaults toNonewithout explicit union type annotation.♻️ Suggested fix
def make_request( request_id: int, seq_slot: int, prompt_len: int = 128, max_new_tokens: int = 100000, - stop_words: list = None, + stop_words: list | None = None, ) -> LlmRequest:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/defs/perf/host_perf/test_module_sampler.py` around lines 57 - 76, Update the make_request function signature to use an explicit optional union type for stop_words (e.g., stop_words: list | None) so the parameter is clearly annotated as optional; update any necessary imports or typing usage if your codebase targets Python<3.10 (use Optional[list] or from typing import Optional) and ensure the function body still checks "if stop_words:" and assigns it to req.py_stop_words_list as before (refer to make_request, stop_words, and req.py_stop_words_list).
79-86: Same fix needed formake_generation_requests.♻️ Suggested fix
-def make_generation_requests(count: int, stop_words: list = None) -> List[LlmRequest]: +def make_generation_requests(count: int, stop_words: list | None = None) -> list[LlmRequest]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/defs/perf/host_perf/test_module_sampler.py` around lines 79 - 86, make_generation_requests currently passes the same stop_words list reference into every request, causing shared-mutable state across LlmRequest objects; update the call inside make_generation_requests (where make_request(...) is invoked) to pass a copy of stop_words (e.g., list(stop_words) or stop_words.copy() when stop_words is not None) so each generated request gets its own stop_words list; keep the existing seq_slot assignment and setting req.state = LlmRequestState.GENERATION_IN_PROGRESS.tests/integration/defs/perf/host_perf/test_module_resource_manager.py (1)
91-101: Use explicit| Nonefor optional parameters.Per PEP 484, implicit
Optional(defaultNonewithout union type) is prohibited. With Python 3.10+, use the union syntax directly.♻️ Suggested fix for type annotations
def make_scheduled_requests( - generation_requests: List[LlmRequest] = None, - context_requests: List[LlmRequest] = None, + generation_requests: list[LlmRequest] | None = None, + context_requests: list[LlmRequest] | None = None, ) -> ScheduledRequests:Based on learnings: "In TensorRT-LLM (Python requires >=3.10 and <4 as per setup.py), you can use Python 3.10+ features."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/defs/perf/host_perf/test_module_resource_manager.py` around lines 91 - 101, The function make_scheduled_requests currently uses implicit Optional by defaulting generation_requests and context_requests to None; update the type annotations to use explicit union syntax (generation_requests: List[LlmRequest] | None and context_requests: List[LlmRequest] | None) so it follows PEP 484 for Python 3.10+ and keep the same default values and behavior inside the function; locate the signature for make_scheduled_requests and change only the parameter annotations accordingly.tests/integration/defs/perf/host_perf/test_module_scheduler.py (1)
40-40: Prefer PEP 585 built-in generics overtyping.List.Since TensorRT-LLM requires Python 3.10+, you can use built-in generic types directly without importing from
typing.♻️ Suggested refactor to use modern type hints
-from typing import ListThen update the type hints throughout the file:
List[float]→list[float]List[LlmRequest]→list[LlmRequest]Based on learnings: "In TensorRT-LLM (Python requires >=3.10 and <4 as per setup.py), you can use Python 3.10+ features (e.g., PEP 585 generics like dict[str, int], list[str], etc.) throughout the codebase."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/defs/perf/host_perf/test_module_scheduler.py` at line 40, Replace the legacy typing.List usage with PEP 585 built-in generics: remove the "from typing import List" import and change all occurrences of List[...] to list[...] (e.g., change List[float] → list[float] and List[LlmRequest] → list[LlmRequest]) in this test module; search for the symbol "List" and update the type annotations in functions and variable declarations such as those referencing LlmRequest and numeric lists to use the native list[...] syntax.tests/integration/defs/perf/host_perf/test_host_perf.py (3)
329-333: Consider handlingCalledProcessErrorfor better diagnostics.If the benchmark client fails,
subprocess.check_outputraisesCalledProcessError. While this will fail the test (appropriate behavior), catching it and logging the partial output could improve debugging.💡 Optional: Enhanced error handling
- output = subprocess.check_output( - client_cmd, - stderr=subprocess.STDOUT, - env=copy.deepcopy(os.environ), - ).decode() + try: + output = subprocess.check_output( + client_cmd, + stderr=subprocess.STDOUT, + env=copy.deepcopy(os.environ), + ).decode() + except subprocess.CalledProcessError as e: + print_info(f"[host_perf] Benchmark failed with exit code {e.returncode}") + print_info(f"[host_perf] Output: {e.output.decode() if e.output else 'None'}") + raise🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/defs/perf/host_perf/test_host_perf.py` around lines 329 - 333, The test currently calls subprocess.check_output(client_cmd, stderr=subprocess.STDOUT, env=copy.deepcopy(os.environ)).decode() which will raise subprocess.CalledProcessError on failure and loses partial output; wrap this call in a try/except that catches subprocess.CalledProcessError, decode and log or attach the partial output (e.output or e.output.decode(errors="replace")) along with client_cmd for diagnostics, then re-raise the exception (or fail the test) so behavior remains failing but with improved logs for debugging.
403-403: Minor: EN DASH in docstring.Line 403 contains an EN DASH (–) instead of a HYPHEN-MINUS (-) in "Welch–Satterthwaite". This is flagged by Ruff (RUF002). While not a functional issue, consider using ASCII hyphen for consistency.
🔧 Proposed fix
- Uses the Welch–Satterthwaite approximation for degrees of freedom. + Uses the Welch-Satterthwaite approximation for degrees of freedom.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/defs/perf/host_perf/test_host_perf.py` at line 403, Replace the EN DASH in the docstring phrase "Welch–Satterthwaite" with a standard ASCII hyphen-minus ("Welch-Satterthwaite") to satisfy linting (RUF002); locate the docstring containing that phrase in tests/integration/defs/perf/host_perf/test_host_perf.py (search for "Welch–Satterthwaite") and edit the text accordingly so only the punctuation changes.
49-49: Import style deviation from guidelines.The import
from tensorrt_llm._utils import get_free_portimports a function directly. Per coding guidelines, prefer importing the module and maintaining the namespace:from tensorrt_llm import _utilsthen_utils.get_free_port().That said, this is a minor style point for test code and the current form is common in this codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/defs/perf/host_perf/test_host_perf.py` at line 49, Replace the direct function import with a module import to preserve namespace: change the import of get_free_port to import the module (use _utils) by importing tensorrt_llm._utils as _utils or using from tensorrt_llm import _utils, and then update all call sites in this test (where get_free_port is invoked) to call _utils.get_free_port() instead of get_free_port().tests/integration/defs/perf/host_perf/test_llmapi_perf.py (1)
53-64: Consider extracting shared constants to avoid duplication.
MODEL_PATH_DICTandget_model_dirare duplicated between this file andtest_host_perf.py(lines 55-58 and 84-88 respectively). Consider extracting these to a shared module (e.g., ahost_perf_common.pyor the existingconftest.py) to maintain a single source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/defs/perf/host_perf/test_llmapi_perf.py` around lines 53 - 64, Extract the duplicated constants and helper into a shared module: move MODEL_PATH_DICT, HOST_PERF_CONFIG_DIR and get_model_dir into a new module (e.g., host_perf_common.py or place in existing conftest.py), export get_model_dir and MODEL_PATH_DICT from there, then update both test_llmapi_perf.py and test_host_perf.py to import MODEL_PATH_DICT and get_model_dir instead of redefining them and remove the local duplicates; ensure get_model_dir still calls llm_models_root() and paths resolve the same way after the refactor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/integration/defs/perf/host_perf/test_host_perf.py`:
- Around line 329-333: The test currently calls
subprocess.check_output(client_cmd, stderr=subprocess.STDOUT,
env=copy.deepcopy(os.environ)).decode() which will raise
subprocess.CalledProcessError on failure and loses partial output; wrap this
call in a try/except that catches subprocess.CalledProcessError, decode and log
or attach the partial output (e.output or e.output.decode(errors="replace"))
along with client_cmd for diagnostics, then re-raise the exception (or fail the
test) so behavior remains failing but with improved logs for debugging.
- Line 403: Replace the EN DASH in the docstring phrase "Welch–Satterthwaite"
with a standard ASCII hyphen-minus ("Welch-Satterthwaite") to satisfy linting
(RUF002); locate the docstring containing that phrase in
tests/integration/defs/perf/host_perf/test_host_perf.py (search for
"Welch–Satterthwaite") and edit the text accordingly so only the punctuation
changes.
- Line 49: Replace the direct function import with a module import to preserve
namespace: change the import of get_free_port to import the module (use _utils)
by importing tensorrt_llm._utils as _utils or using from tensorrt_llm import
_utils, and then update all call sites in this test (where get_free_port is
invoked) to call _utils.get_free_port() instead of get_free_port().
In `@tests/integration/defs/perf/host_perf/test_llmapi_perf.py`:
- Around line 53-64: Extract the duplicated constants and helper into a shared
module: move MODEL_PATH_DICT, HOST_PERF_CONFIG_DIR and get_model_dir into a new
module (e.g., host_perf_common.py or place in existing conftest.py), export
get_model_dir and MODEL_PATH_DICT from there, then update both
test_llmapi_perf.py and test_host_perf.py to import MODEL_PATH_DICT and
get_model_dir instead of redefining them and remove the local duplicates; ensure
get_model_dir still calls llm_models_root() and paths resolve the same way after
the refactor.
In `@tests/integration/defs/perf/host_perf/test_module_resource_manager.py`:
- Around line 91-101: The function make_scheduled_requests currently uses
implicit Optional by defaulting generation_requests and context_requests to
None; update the type annotations to use explicit union syntax
(generation_requests: List[LlmRequest] | None and context_requests:
List[LlmRequest] | None) so it follows PEP 484 for Python 3.10+ and keep the
same default values and behavior inside the function; locate the signature for
make_scheduled_requests and change only the parameter annotations accordingly.
In `@tests/integration/defs/perf/host_perf/test_module_sampler.py`:
- Around line 89-99: Update make_scheduled_requests signature to use explicit
union type for optional lists (e.g., generation_requests: List[LlmRequest] |
None = None and context_requests: List[LlmRequest] | None = None) so the
parameters clearly indicate they can be None; keep the body unchanged and retain
references to ScheduledRequests and LlmRequest used inside the function.
- Around line 57-76: Update the make_request function signature to use an
explicit optional union type for stop_words (e.g., stop_words: list | None) so
the parameter is clearly annotated as optional; update any necessary imports or
typing usage if your codebase targets Python<3.10 (use Optional[list] or from
typing import Optional) and ensure the function body still checks "if
stop_words:" and assigns it to req.py_stop_words_list as before (refer to
make_request, stop_words, and req.py_stop_words_list).
- Around line 79-86: make_generation_requests currently passes the same
stop_words list reference into every request, causing shared-mutable state
across LlmRequest objects; update the call inside make_generation_requests
(where make_request(...) is invoked) to pass a copy of stop_words (e.g.,
list(stop_words) or stop_words.copy() when stop_words is not None) so each
generated request gets its own stop_words list; keep the existing seq_slot
assignment and setting req.state = LlmRequestState.GENERATION_IN_PROGRESS.
In `@tests/integration/defs/perf/host_perf/test_module_scheduler.py`:
- Line 40: Replace the legacy typing.List usage with PEP 585 built-in generics:
remove the "from typing import List" import and change all occurrences of
List[...] to list[...] (e.g., change List[float] → list[float] and
List[LlmRequest] → list[LlmRequest]) in this test module; search for the symbol
"List" and update the type annotations in functions and variable declarations
such as those referencing LlmRequest and numeric lists to use the native
list[...] syntax.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0b354729-0c7a-4532-abcc-c823252439fe
📒 Files selected for processing (11)
tests/integration/defs/perf/host_perf/README.mdtests/integration/defs/perf/host_perf/__init__.pytests/integration/defs/perf/host_perf/host_perf_deepseek_v3_lite.yamltests/integration/defs/perf/host_perf/host_perf_llama8b.yamltests/integration/defs/perf/host_perf/host_perf_llama8b_spec_decode.yamltests/integration/defs/perf/host_perf/test_host_perf.pytests/integration/defs/perf/host_perf/test_llmapi_perf.pytests/integration/defs/perf/host_perf/test_module_resource_manager.pytests/integration/defs/perf/host_perf/test_module_sampler.pytests/integration/defs/perf/host_perf/test_module_scheduler.pytests/integration/test_lists/test-db/l0_b200.yml
b89052b to
21ebadd
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #39337 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #39385 [ run ] triggered by Bot. Commit: |
|
/bot run --stage-list "DGX_B200-1_GPU-PyTorch" --disable-fail-fast |
|
PR_Github #41375 [ run ] triggered by Bot. Commit: |
|
/bot run --stage-list "DGX_B200-1_GPU-PyTorch" --disable-fail-fast |
|
PR_Github #41387 [ run ] triggered by Bot. Commit: |
|
PR_Github #41375 [ run ] completed with state |
f420ff1 to
f5fc721
Compare
|
PR_Github #41387 [ run ] completed with state
|
f5fc721 to
a417bd3
Compare
|
/bot run --stage-list "DGX_B200-PyTorch-1, DGX_B200-PyTorch-2, DGX_B200-PyTorch-3" --disable-fail-fast |
|
PR_Github #42030 [ run ] triggered by Bot. Commit: |
|
PR_Github #42030 [ run ] completed with state
|
d2c553f to
fef371c
Compare
|
/bot run --stage-list "DGX_B200-PyTorch-1, DGX_B200-PyTorch-2, DGX_B200-PyTorch-3" --disable-fail-fast |
|
PR_Github #42278 [ run ] triggered by Bot. Commit: |
|
PR_Github #42278 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #42340 [ run ] triggered by Bot. Commit: |
|
PR_Github #42340 [ run ] completed with state
|
…ntegration Add a comprehensive host performance regression test suite that detects CPU-side overhead regressions in the PyExecutor inference pipeline. Three test levels: - Module tests: micro-benchmark scheduler, sampler, kv_cache (6 tests) - E2E serve tests: trtllm-serve + benchmark_serving (3 tests) Llama8B dense, DeepSeek-V3-Lite MoE/MLA, Llama8B spec decode Features: - YAML-driven config discovery for E2E tests - Baseline regression detection from baselines_b200.json - OpenSearch DB integration via process_and_upload_test_results() - Auto-detect upload from OPEN_SEARCH_DB_BASE_URL env var - Reuses shared constants (PERF_METRIC_LOG_QUERIES, SUPPORTED_GPU_MAPPING) from test_perf_sanity.py to avoid duplication Signed-off-by: Yukun He <23156053+hyukn@users.noreply.github.com>
…y.py Remove test_host_perf.py and reuse the existing test_perf_sanity.py infrastructure for host-overhead-dominant E2E benchmarks. The YAML configs are moved to tests/scripts/perf-sanity/aggregated/ and the small model entries are added to test_perf_sanity.py's MODEL_PATH_DICT. This eliminates 485 lines of duplicated serve+benchmark logic while preserving identical test coverage through perf_sanity's test_e2e. Signed-off-by: Yukun He <23156053+hyukn@users.noreply.github.com>
01cc32c to
754cfd9
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #42506 [ run ] triggered by Bot. Commit: |
|
PR_Github #42506 [ run ] completed with state |
Summary by CodeRabbit
Release Notes
Tests
Documentation
Description
Layered test suite for detecting host (CPU) overhead regressions in TensorRT-LLM's PyExecutor pipeline.
Module tests — pre-merge (CPU-bounded, ~15s total, no model weights)
REGRESSION DETECTEDmarkers (test stays green; CI can grep)E2E tests — post-merge (GPU + model weights required)
LLM.generate()direct — cleanest host signal, no HTTP overheadtrtllm-serve+benchmark_serving— full serving stack validationCI integration
l0_b200.ymlpre-merge pytorch stagel0_b200.ymlpost-merge pytorch stageTest Coverage
SampleStateAPI (requests: list[LlmRequest])PR Checklist
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.