Acquire coherent nightly WASM workloads - #5300
Conversation
305ca46 to
24e79bd
Compare
There was a problem hiding this comment.
🟡 Changes recommended
wasm_sdk_cohort detection in run_performance_job.py treats an empty workload source as enabled, which can skip payload setup and then fail inconsistently.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds support for acquiring and using a coherent, SDK-pinned CoreCLR browser-WASM workload cohort (including wasm-tools closure and exact runtime/ref/Crossgen2/ILLink/WebAssembly SDK packs) and propagates that source through performance CI/Helix, with documentation for local usage.
Changes:
- Add
--wasm-workload-sourceoption and forward it into Helix work item execution for CoreCLR WASM ReadyToRun runs. - Implement cohort resolution and installation logic based on the SDK’s
BundledVersions.props, with validation for coherent versions and required packages. - Plumb the new option through pipeline templates and document nightly/VMR CoreCLR WASM R2R usage.
File summaries
| File | Description |
|---|---|
| scripts/tests/test_wasm_coreclr_r2r.py | Adds coverage for forwarding/command generation and cohort version/package validation behavior. |
| scripts/run_performance_job.py | Threads --wasm-workload-source through Helix setup and payload preparation for CoreCLR WASM R2R runs. |
| scripts/dotnet.py | Adds host-RID detection plus coherent WASM workload cohort derivation and installation from an exclusive source. |
| scripts/benchmarks_ci.py | Adds CLI validation and local installation step for coherent CoreCLR WASM workload sources. |
| eng/pipelines/templates/runtime-perf-job.yml | Introduces a wasmWorkloadSource parameter for runtime perf jobs and forwards it downstream. |
| eng/pipelines/templates/run-performance-job.yml | Passes --wasm-workload-source into run_performance_job.py when configured. |
| docs/benchmarking-workflow-dotnet-runtime.md | Documents how to run CoreCLR WASM ReadyToRun using a nightly/VMR SDK + coherent package source. |
| docs/benchmarkdotnet.md | Updates the table-of-contents entry naming for private runtime builds. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| wasm_sdk_cohort = ( | ||
| wasm_coreclr | ||
| and args.r2r_run_type == "r2r" | ||
| and args.wasm_workload_source is not None) |
24e79bd to
d83298c
Compare
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of correctness/usability issues in the updated docs/CLI error messaging that should be fixed before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Lite
| - [Running In Process](#running-in-process) | ||
| - [CoreRun](#corerun) | ||
| - [dotnet cli](#dotnet-cli) | ||
| - [Private CLR Build](#private-clr-build) | ||
| - [Private Runtime Builds](#private-runtime-builds) | ||
| - [Private CoreRT Build](#private-corert-build) |
| if len(sdk_versions) > 1: | ||
| raise ValueError( | ||
| '--wasm-workload-source requires exactly one SDK version') |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 797f00e5-64e5-4d0b-9b62-bf06a1bf9c9d
d83298c to
6b0c465
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
There are a couple of correctness/documentation issues (empty workload source handling and a duplicate TOC entry) that should be fixed before merging.
Review details
Suppressed comments (3)
docs/benchmarkdotnet.md:31
- The Table of Contents lists “Private Runtime Builds” twice (two bullets with the same #private-runtime-builds anchor), which makes the ToC confusing and adds a duplicate link.
- [Running In Process](#running-in-process)
- [CoreRun](#corerun)
- [dotnet cli](#dotnet-cli)
- [Private Runtime Builds](#private-runtime-builds)
- [Private CoreRT Build](#private-corert-build)
scripts/dotnet.py:278
- This error message is inaccurate: the code allows omitting --dotnet-versions (sdk_versions can be empty) and resolves the SDK from
dotnet --version. The message should only reject multiple SDK versions.
if len(sdk_versions) > 1:
raise ValueError(
'--wasm-workload-source requires exactly one SDK version')
scripts/run_performance_job.py:987
wasm_sdk_cohortbecomes True when--wasm-workload-sourceis an empty string, which then skips the wasm_coreclr payload build and leavesdotnet_pathunset later. Treat empty/whitespace values the same as “not provided”.
wasm_sdk_cohort = (
wasm_coreclr
and args.r2r_run_type == "r2r"
and args.wasm_workload_source is not None)
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
--wasm-workload-sourceto acquire the SDK-pinnedwasm-toolsclosure and exact CoreCLR browser-WASM runtime/ref, WebAssembly SDK, host-RID Crossgen2, and ILLink packagesValidation
python3 -m pytest -q scripts/tests/test_wasm_coreclr_r2r.py(21 passed)11.0.100-preview.6.26351.102and its exact WASM cohort from the exclusivedotnet11feed, includingosx-arm64Crossgen2 and native WASM toolinggit diff --checkStack
Depends on #5297. This draft targets
lewing-wasm-coreclr-r2r-perfand should be reviewed/merged after the lower CoreCLR WASM R2R layer.