Add CoreCLR WASM R2R performance lane - #5297
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
It changes CI/Helix orchestration and MSBuild/runtime-pack resolution behavior where correctness is environment-dependent and should be validated by a human with end-to-end runs.
Pull request overview
Adds a dedicated CoreCLR browser-WASM ReadyToRun (R2R) microbenchmark lane (V8) and wires the R2R selection through the performance repo’s Helix/job plumbing and MSBuild configuration, including validation to fail fast when required settings aren’t applied.
Changes:
- Adds CoreCLR-vs-Mono runtime-pack override logic for WASM builds and validates the expected CoreCLR runtime pack for R2R runs.
- Introduces
--wasm-ready-to-runend-to-end (CLI arg → env var → MSBuild properties → Helix forwarding) and emits a distinctR2RType=r2rrun dimension. - Adds a non-release-branch pipeline lane for CoreCLR WASM R2R microbenchmarks on V8 plus focused pytest coverage.
File summaries
| File | Description |
|---|---|
| src/scenarios/build-common/WasmOverridePacks.targets | Chooses CoreCLR vs Mono WASM runtime-pack IDs based on UseMonoRuntime. |
| src/benchmarks/micro/MicroBenchmarks.Wasm.targets | Enables/validates CoreCLR WASM R2R publish properties and validates runtime-pack selection. |
| scripts/tests/test_wasm_coreclr_r2r.py | Adds pytest coverage for argument validation, env propagation, Helix forwarding, and result dimensions. |
| scripts/run_performance_job.py | Forwards --wasm-ready-to-run to Helix work items and adds R2RType=r2r dimension. |
| scripts/micro_benchmarks.py | Adds --wasm-ready-to-run and propagates it via PERFLAB_WASM_READY_TO_RUN. |
| eng/pipelines/runtime-wasm-perf-jobs.yml | Adds a CoreCLR WASM R2R microbenchmark lane (non-release branches), V8 on linux_x64. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
dotnet/runtime#133040 is the runtime side |
|
Pushed |
There was a problem hiding this comment.
🔵 Needs a closer look
The environment-dependent MSBuild and Helix integration still requires the stated end-to-end validation after the latest package-version fix.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
is there a better (non-workload) way to use the crossgen from the build? |
|
Build 1579662 was green but did not actually compile the benchmark app as R2R: the inner-TFM restore evaluation reset |
There was a problem hiding this comment.
🔵 Needs a closer look
The configuration validation runs after publishing rather than failing before build work begins.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/benchmarks/micro/MicroBenchmarks.Wasm.targets:21
PrepareForWasmBuildis a BenchmarkDotNet-generated target that runsAfterTargets="Publish", so this validation occurs only after the full publish/R2R work has already completed. Invalid Mono/R2R or package settings therefore do not fail fast as described; hook this validation before framework-pack resolution (or another pre-build target) instead.
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
the crossgen we're building for the pipeline had a newer glibc because it wasn't crossbuilt dotnet/runtime#133143 it the fix for that and I will rereun the test once it is in. |
There was a problem hiding this comment.
🟡 Changes recommended
Payload validation rejects the actual runtime artifact and regresses non-R2R CoreCLR WASM runs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
scripts/build_runtime_payload.py:412
- The linked
BrowserWasmCoreCLRproducer stages only the WebAssembly SDK/ref packages and the newly added Crossgen2 package instaging/built-nugets; it does not stage aMicrosoft.NET.ILLink.Tasksnupkg. This check therefore raises “found 0” for every CoreCLR WASM job before Helix submission, including the existing non-R2R lane. Either stage the matching ILLink package in the runtime artifact or validate/use the ILLink pack already installed indotnet-noneinstead.
illink_packages = [
package for package in Path(built_nugets_dir).glob("Microsoft.NET.ILLink.Tasks.*.nupkg")
if not package.name.endswith(".symbols.nupkg")
]
if len(illink_packages) != 1:
raise ValueError(
f"Expected one ILLink package in {built_nugets_dir}, found {len(illink_packages)}")
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Approval depends on coordinated runtime artifacts and final end-to-end validation of the cross-repository toolchain path.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f286edf-0a3c-43dc-92fe-26a0a57d5360
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f286edf-0a3c-43dc-92fe-26a0a57d5360
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f286edf-0a3c-43dc-92fe-26a0a57d5360
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f286edf-0a3c-43dc-92fe-26a0a57d5360
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f286edf-0a3c-43dc-92fe-26a0a57d5360
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f286edf-0a3c-43dc-92fe-26a0a57d5360
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f286edf-0a3c-43dc-92fe-26a0a57d5360
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f286edf-0a3c-43dc-92fe-26a0a57d5360
c51c6c4 to
0790cb8
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The new package-version requirement breaks an existing wasm_coreclr unit-test path.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f286edf-0a3c-43dc-92fe-26a0a57d5360
maraf
left a comment
There was a problem hiding this comment.
Beside the amount of places that hardcode 11.* it looks good to me
|
The PR description should also mention dependency on runtime's #133203 |
@radekdoulik fixed |
This is all just test related to verify the different version formats all work so you can run locally, against unmerged prs, or against versioned builds. |
…ort (#133203) ## Summary Stage the complete same-build CoreCLR browser-WASM package cohort consumed by the performance pipeline. This fixes two failures: - The Mono artifact job now downloads both the CoreCLR browser runtime pack and Crossgen2 pack before installing the wasm-tools workload. In runtime-wasm-perf build [1584559](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1584559), workload installation failed because `Microsoft.NETCore.App.Crossgen2.linux-x64` version `11.0.0-ci` was absent from the local feed. - The CoreCLR R2R artifact now includes the locally built `Microsoft.NET.ILLink.Tasks` package. In build [1581666](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1581666), restore requested ILLink product version `.109`, the artifact did not contain it, NuGet floated to `.118`, and `_RunILLink` then failed because the payload only contained the `.109` runtime. - Use the canonical SDK R2R output directory spelling when staging trimmed CoreCLR browser-WASM images for WebCIL conversion. Add a focused target evaluation test that compares the path ordinally so case-insensitive hosts catch regressions too. The existing CoreCLR-only Crossgen2 staging switch becomes one toolchain/cohort staging switch that: - packs `ILLink.Tasks.csproj` without rebuilding or restoring, because the normal `clr+libs+host+packs` build compiles the project but does not pack it; - requires exactly one non-symbol package for each local package ID; - rejects packages whose versions do not match the local WebAssembly SDK cohort; - publishes the CoreCLR runtime and Crossgen2 packs for the dependent Mono artifact job; - stages those packages into the Mono job's local feed before workload installation. This completes the runtime artifact consumed by dotnet/performance#5297 and repairs the prerequisite artifact build used by the existing Mono WASM performance lane. ## Complete staged cohort The CoreCLR performance artifact now carries the complete same-build publish inputs: - SDK: `staging/dotnet-none` - CoreCLR browser runtime pack: `staging/microsoft.netcore.app.runtime.browser-wasm` - `Microsoft.NETCore.App.Ref` - `Microsoft.NET.Sdk.WebAssembly.Pack` - `Microsoft.NETCore.App.Crossgen2.linux-x64` - `Microsoft.NET.ILLink.Tasks` The dependent Mono artifact job also receives the same-build CoreCLR browser runtime and Crossgen2 packages before installing its workload. WebAssembly SDK and ILLink are direct SDK tool restore dependencies, while Crossgen2 and the runtime/ref packs are selected through the SDK framework/tool-pack graph. ILLink's private dependencies are included in its tooling package, so no additional locally built SDK tool package is required. The performance-side override aligns `KnownWebAssemblySdkPack`, `KnownCrossgen2Pack`, and `KnownILLinkPack` to the staged local package version. ## Validation - Parsed both changed Azure Pipelines YAML files. - Simulated the extracted staging script with matching non-symbol, symbol, and unrelated nupkgs; only the required non-symbol cohort packages were selected. - Simulated an absent package and a mismatched package version; both failed with explicit diagnostics. - Audited build 1584559 and confirmed the Mono artifact job failed specifically because `Microsoft.NETCore.App.Crossgen2.linux-x64` version `11.0.0-ci` was missing from the local package feed; this change publishes and downloads that exact package from the CoreCLR build job. - Audited build 1581666's restore/publish binlogs and CoreCLR build artifact. This confirmed the NU1603 fallback, the `.118` ILLink/runtime mismatch, the package graph above, and that the regular runtime build compiles but does not pack `ILLink.Tasks.csproj`. - Ran `git diff --check` and an independent focused review of the final pipeline diff. ## Remaining end-to-end validation Queue `runtime-wasm-perf` pipeline 156 on Ubuntu 22.04 with repository resource overrides: ```json { "resources": { "repositories": { "self": { "refName": "refs/heads/lewing-stage-wasm-r2r-tools" }, "performance": { "refName": "refs/heads/lewing-wasm-coreclr-r2r-perf" } } } } ``` The Mono artifact job must install the workload from the local feed without a missing Crossgen2 package. The CoreCLR R2R lane must restore without NU1603/fallback, complete ILLink, emit non-empty per-app R2R images through Crossgen2, stage WebCIL, and start the benchmark under V8. Existing failed builds cannot provide that proof because their published artifacts predate the complete cohort staged by this change. > [!NOTE] > This pull request description was generated with the assistance of GitHub Copilot. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 26a97530-a6d9-44e6-8754-2783961f1313
radekdoulik
left a comment
There was a problem hiding this comment.
Besides the imports nit it LGTM
Summary
R2RType=r2rresult dimensionDependency
Depends on dotnet/runtime#133040 and finally dotnet/runtime#133203, which stages the matching host Crossgen2 pack in
BrowserWasmCoreCLR.Validation
Remaining validation
Run
runtime-wasm-perfwith this branch and the runtime artifact together, confirmCreateReadyToRunImagesexecutes, and verify uploaded results are labeledR2RType=r2r.