[perf-improver] perf: add dotnet test server-mode scenario to performance runner#9486
[perf-improver] perf: add dotnet test server-mode scenario to performance runner#9486Evangelink wants to merge 2 commits into
Conversation
Adds Scenario1_DotnetTest_PlainProcess which runs the 100x100 test asset via 'dotnet test --no-build' instead of direct executable invocation. This exercises the MTP JSON-RPC/named-pipe server-mode path that the existing PlainProcess scenarios do not cover. The new DotnetTestProcess step: - Invokes the repo-local SDK dotnet binary (consistent with DotnetMuxer) - Drains stdout/stderr async to avoid pipe deadlocks - Records wall-clock elapsed time and TotalProcessorTime (parent only) - Writes Result.json in the same format as PlainProcess The scenario name contains 'PlainProcess' so it is automatically captured by the existing nightly workflow filter (*PlainProcess*) without requiring any workflow file changes. Addresses the gap identified in #9480. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new performance-runner scenario to measure the dotnet test (MTP server-mode / JSON-RPC) execution path, complementing the existing direct executable (plain-process) timing scenario so nightly perf runs can detect regressions/improvements in the server-mode pipeline.
Changes:
- Introduces a new
DotnetTestProcessstep that runsdotnet test --no-buildand records timing metrics intoResult.json. - Registers a new pipeline
Scenario1_DotnetTest_PlainProcessin the performance runner so it’s picked up by the existing nightly*PlainProcess*filter.
Show a summary per file
| File | Description |
|---|---|
| test/Performance/MSTest.Performance.Runner/Steps/DotnetTestProcess.cs | New step that launches dotnet test and captures wall-clock/CPU metrics + zips the asset for upload. |
| test/Performance/MSTest.Performance.Runner/Program.cs | Adds a new pipeline wiring Scenario1 + build + DotnetTestProcess into the default runner set. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Low
This comment has been minimized.
This comment has been minimized.
Evangelink
left a comment
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
| # | Dimension | Verdict |
|---|---|---|
| 1 | Algorithmic Correctness | 🟠 1 MAJOR — no exit-code check, silent bad-data on dotnet test failure |
| 1 | Algorithmic Correctness | 🟡 1 MODERATE — missing --configuration flag, fragile implicit Debug coupling |
✅ 21/22 dimensions clean.
- Exit-code check —
dotnet testhas many more failure modes than a pre-located binary (project not found,SDK mismatch, JIT error). Without checkingprocess.ExitCode, a failed run records a short "elapsed time" that silently contaminates the perf baseline. See inline comment on line 77. -
--configurationpropagation —DotnetTestProcessimplicitly relies ondotnet test --no-builddefaulting toDebug. If a pipeline ever wiresDotnetMuxer(BuildConfiguration.Release)before this step, the step will silently measure stale Debug artefacts (or fail). Adding aBuildConfigurationconstructor parameter keeps this class self-contained. See inline comment on line 54.
Non-blocking design note: The scenario name Scenario1_DotnetTest_PlainProcess embeds "PlainProcess" only to satisfy the nightly workflow's *PlainProcess* filter — as the Program.cs comment explains. This works but inverts the normal naming contract: the name describes the measurement class (PlainProcess) rather than the invocation path (DotnetTest). A cleaner long-term solution would be to extend the nightly workflow filter to *PlainProcess*|*DotnetTest* and rename the scenario to Scenario1_DotnetTest so it says what it actually does. The current approach isn't wrong, but the comment dependency (why does a DotnetTest scenario have "PlainProcess" in its name?) will surprise the next contributor who looks at the file in isolation.
…configuration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🧪 Test quality grade — PR #9486No new or modified test methods were identified in the changed regions of this PR. Nothing to grade. Re-run with
|
Goal and Rationale
The nightly perf-timing workflow collects
PlainProcesstiming for MSTest's direct-run mode (MTP native process). This exercises the plain-process path where the test host runs standalone as an executable.The MTP server-mode path (triggered by
dotnet test) follows a completely different code path: the test host is invoked with--server --protocol dotnet-test-protocol, communicates results over a named pipe, and the JSON-RPC serialisation/framing/pipe I/O overhead is included in the user-visible wall-clock time. Multiple recent efficiency-improver PRs targeted exactly this path (#9274, #9300, #9353, #9380, #9408, #9436), but there has been no perf scenario to verify or measure their impact, and no regression detection for it.This PR adds a
Scenario1_DotnetTest_PlainProcesspipeline that runs the same 100×100 test asset viadotnet test --no-build, bridging that measurement gap. Addresses the gap identified in #9480.Approach
New step:
Steps/DotnetTestProcess.csMirrors
PlainProcessin structure but invokesdotnet test "<projectDir>" --no-buildusing the repo-local SDK, rather than running the compiled executable directly. Key design points:{root}/.dotnet/dotnetmuxer asDotnetMuxer(consistent toolchain)DOTNET_ROOT,DOTNET_INSTALL_DIR, etc.)ElapsedTime(wall-clock) andTotalProcessorTimein the same JSON format asPlainProcessMeasurement note (documented in XML doc):
TotalProcessorTimecovers only thedotnet testparent process; the spawned test-host child's CPU time is not included. Wall-clockElapsedTimeis the primary metric and represents what users observe.New pipeline entry in
Program.cs:The name contains
"PlainProcess"so the existing nightly workflow filter--pipelineNameFilter "*PlainProcess*"picks it up automatically — no workflow file changes required.Performance Evidence
This is a measurement infrastructure change, not a code optimization. It adds observability for a previously unmeasured path. Comparable baseline (PlainProcess, 100 classes × 100 methods, Debug, Linux): ~3.5 s wall-clock per run. Server-mode overhead is expected to add protocol setup time (typically 0.5–1.5 s for pipe init + JSON-RPC negotiation) on top of test execution.
Trade-offs
dotnet testinvocations to the nightly perf run (one per_numberOfRun). Each run takes ~5–10 s. Total additional nightly time: ~30–60 s per platform.Test Status
Local SDK not available in CI agent (pinned SDK 11.0.100-preview). Build and scenario execution delegated to CI.
The implementation follows the established
PlainProcesspattern exactly, substituting only the launched process (dotnet testvs. the direct test-host executable). TheDotnetTestProcessstep correctly handles output draining and produces the sameResult.jsonformat.Reproducibility
Add this agentic workflows to your repo
To install this agentic workflow, run