Repository navigation
Reduce Mono/WASM PR pipeline jobs on library and non-mono changes - #126137
davidwrighton wants to merge 5 commits into
Conversation
- Add shouldRunSmokeOnly: onLibrariesAndIllinkChanges to browser_wasm_win Mono WASM library tests, so they run smoke-only when only libraries or illink changed. browser_wasm (Linux) keeps full test suite, ensuring at least 1 full Mono WASM run per library PR. - Remove libraries.containsChange and installer.containsChange from the Mono Android (android_arm, android_arm64) build+test condition. Mono Android now only runs when mono code changes or on rolling builds. CoreCLR Android jobs remain unchanged for continued Android PR coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Split the Mono_MiniJIT_LibrariesTests job into two blocks: - linux_arm64: runs on library changes, mono changes, and rolling builds - linux_x64 + osx_x64: runs only on mono changes and rolling builds This saves 2 helix jobs on library-only PRs while maintaining full platform coverage when mono code is explicitly changed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove WasmTestOnFirefox scenario from the browser_wasm Mono library tests. Firefox coverage is retained on rolling builds via the extra-platforms pipeline. - Remove browser_wasm_win from AOT Smoke tests. browser_wasm (Linux) still runs AOT Smoke. Windows AOT Smoke coverage continues on rolling builds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @akoeplinger, @matouskozak, @simonrozsival |
There was a problem hiding this comment.
Pull request overview
This PR reduces Mono/WASM PR pipeline workload for changes that don’t touch Mono/WASM-specific paths, while preserving full coverage when Mono/WASM code changes and on rolling builds.
Changes:
- Removed the Firefox scenario from the Mono WASM library test job to reduce Helix work items.
- Switched
browser_wasm_winMono WASM library tests to smoke-only on libraries/illink-only PRs viashouldRunSmokeOnly: onLibrariesAndIllinkChanges. - Narrowed PR triggers for Mono Android jobs and split Mono MiniJIT libraries tests so only
linux_arm64runs on libraries-only PRs.
|
It looks sensible to me. I think we should add mono smoke firefox on browser_wasm_win to have some minimal coverage. Besides that, I can look at reducing the current coreclr libraries tests set. It is already clear, that we cannot run all of the coreclr test on CI for performance reasons. We have already disabled some because they would take too long. Last time I triaged them, I chose these which will be under 1 hour on my machine. I can retriage and choose lower threshold. Would it make sense to have broader coverage of wasm coreclr tests in outerloop or would it cause troubles? |
|
I looked a bit deeper into libraries tests (wasm/coreclr in our case) and it looks like many suites run very quickly, but still have nontrivial start overhead on helix. So more than half of the total time is just the overhead. It would be interesting to group quick suites together, to reduce that overhead. From copilot's analysis:
|
|
@radekdoulik Broader coverage in outerloop is great. We can always do that, and the cost turns out to be minimal. Instead of adding a firefox mono smoke run for browser wasm, why don't we make the mono aot wasm run execute on firefox. That should give some coverage on firefox, and work fairly well. |
Add WasmTestOnFirefox to the browser_wasm AOT Smoke test scenarios to retain some Firefox coverage in the default PR pipeline, since Firefox was removed from the Mono WASM full library test run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
The grouping of wasm library tests works pretty good in #126157, it saves more than half of total time. copilot's analysis: I think it makes sense to do it later for mono too. |
| scenarios: | ||
| - WasmTestOnChrome | ||
| - WasmTestOnFirefox |
There was a problem hiding this comment.
The AOT smoke job now explicitly adds a Firefox scenario (in addition to Chrome). Previously, the AOT smoke tests defaulted to Chrome-only via wasm-library-aot-tests.yml, so this is a behavior/work-item count change that isn’t called out in the PR description and may add extra runtime/flakiness. If the intent is only to drop browser_wasm_win while keeping the same smoke coverage, consider keeping scenarios unset (or Chrome-only) and/or updating the PR description to reflect the added Firefox run.
|
Workflow state for the Holistic Review Orchestrator. {
"version": 5,
"last_dispatched_commit": "4c0f68329f6d29b74da870527ab8be9a401dffd5",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "8b4d9c22144fcbf00f3af4b0a013bd909dc30501",
"last_reviewed_commit": "4c0f68329f6d29b74da870527ab8be9a401dffd5",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "8b4d9c22144fcbf00f3af4b0a013bd909dc30501",
"last_recorded_worker_run_id": "29674629683",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "4c0f68329f6d29b74da870527ab8be9a401dffd5",
"review_id": 4730178513
}
]
} |
There was a problem hiding this comment.
Holistic Review
Motivation: PR builds on the runtime pipeline were submitting Mono and WebAssembly helix jobs/work items for library- and infrastructure-only changes that don't touch Mono or WASM code. Based on analysis of the last 100 builds (~33% of PRs are library/infra changes), this wastes helix capacity. Reducing that redundant work shortens PR queue times without materially reducing pre-merge signal.
Approach: The change is confined to eng/pipelines/runtime.yml and reuses existing template plumbing rather than introducing new mechanisms:
browser_wasm_winMono WASM library tests gainshouldRunSmokeOnly: onLibrariesAndIllinkChanges, which thewasm-library-tests.ymltemplate already understands (it degrades to smoke tests only when libraries/illink change and no WASM-specific/darc changes are present). The Linuxbrowser_wasmChrome run stays a full run, preserving one complete Mono WASM validation per library PR.- Firefox is dropped from the
browser_wasmMono full-run scenarios (rolling still covers it via extra-platforms). - Mono Android (
android_arm/android_arm64) dropslibraries.containsChange/installer.containsChangefrom both its job condition and its test-run condition, keeping thelibrariesContainsChangevariable removal consistent; it still runs on explicit Mono changes and rolling. - The AOT Smoke leg drops
browser_wasm_win, leaving Linux-only. Mono_MiniJIT_LibrariesTestsis split into alinux_arm64block (library + Mono + rolling) and alinux_x64/osx_x64block (Mono + rolling only). Both blocks share the samenameSuffixbut target disjoint platform sets, so there is no job-name collision, and full 3-platform coverage is retained when Mono changes.
All conditions retain the isRollingBuild disjunct, so rolling/CI coverage is unchanged. I could not run the pipeline; assessment is from reading the YAML and the referenced templates.
Summary: This is a well-scoped, low-risk CI configuration change that trades redundant pre-merge Mono/WASM validation on non-Mono PRs for reduced helix load, while preserving full coverage on Mono/WASM PRs and on rolling builds. The template parameter value used is valid and already implemented, the split job blocks are correct, and the condition edits are internally consistent. Two minor observations below are worth a glance but are not blocking.
Detailed Findings
-
AOT Smoke
browser_wasm_winremoval has no rolling fallback in this template (minor). Unlike the smoke-degradation approach used for thebrowser_wasm_winlibrary tests (which keeps the platform but reduces scope, still running on rolling viaalwaysRun), the AOT Smoke leg fully removesbrowser_wasm_winfrom itsplatformslist. BecausealwaysRun: ${{ variables.isRollingBuild }}only applies to platforms that remain in the matrix, this appears to drop Windows AOT smoke coverage from rolling builds entirely, not merely defer it. If the intent (per the PR description) is for Windows AOT smoke to continue on rolling, consider keepingbrowser_wasm_winin that leg guarded byalwaysRun, or confirm the coverage is genuinely provided by another pipeline (e.g. extra-platforms). Worth verifying before merge. -
Scenario change on the AOT Smoke leg (informational). The AOT Smoke leg now explicitly lists
WasmTestOnChromeandWasmTestOnFirefox, whereas previously it relied on the template default (Chrome only). This adds a Firefox scenario to the (Linux-only) AOT smoke run, which slightly increases its work items even as the overall PR reduces WASM load elsewhere. This looks intentional but is worth a sanity check that the net-savings figures in the PR description account for it.
Note
This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.
Generated by Holistic Review · 63.7 AIC · ⌖ 10.8 AIC · ⊞ 10K
Note
This PR was generated with the assistance of GitHub Copilot.
Summary
Reduce the number of helix jobs and work items submitted for PR builds that don't touch mono or WASM code. Based on analysis of the last 100 builds on the runtime pipeline, ~33% of PRs are library or infrastructure changes that were triggering unnecessary mono/WASM work.
Changes
1. browser_wasm_win Mono library tests → smoke-only on library changes
Added
shouldRunSmokeOnly: onLibrariesAndIllinkChangesto the browser_wasm_win Mono WASM library test. When only libraries or illink paths change (no WASM-specific changes), this runs ~4 smoke test projects instead of the full ~60+ test assemblies.The browser_wasm (Linux, Chrome) Mono test remains a full run, ensuring at least 1 complete Mono WASM validation per library PR.
2. Mono Android removed from library/installer PR triggering
Removed
libraries.containsChangeandinstaller.containsChangefrom the Mono Android (android_arm, android_arm64) job condition. Android is moving to CoreCLR-only for customers, so Mono Android PR validation on library changes is no longer needed.mono_excluding_wasm.containsChange)3. Mono MiniJIT Library Tests reduced to 1 platform on library PRs
Split the
Mono_MiniJIT_LibrariesTestsjob into two blocks:When mono code explicitly changes, all 3 platforms still run.
4. Removed Firefox from Mono WASM full run
The browser_wasm Mono library test previously ran both
WasmTestOnChromeandWasmTestOnFirefoxscenarios. Each test assembly runs once per scenario, so this roughly halved the helix work items for that job. Firefox coverage continues on rolling builds via the extra-platforms pipeline.5. Removed browser_wasm_win from AOT Smoke tests
AOT Smoke tests now only run on browser_wasm (Linux). The Windows AOT Smoke variant is deferred to rolling builds.
Impact by PR type
Library-only PRs (~20% of all builds)
EAT tests, CoreCLR WASM tests, and AOT Smoke (Linux) are unchanged — library PRs retain full coverage on those.
CoreCLR-only PRs (~34% of all builds)
No change — CoreCLR PRs were already well-isolated from mono. They trigger 7 WASM jobs (4 build-only + 3 helix), all CoreCLR-flavored.
Mono/WASM PRs (~9% of all builds)
No change — full test coverage preserved on all platforms when mono or WASM code changes.
Rolling/CI builds
No change — all jobs run at full capacity on rolling builds.