Conversation
Enable composite R2R output for browser CoreCLR, load the owner image before runtime initialization, and preserve component stubs and linked inputs across incremental publishes. Fix WebCIL function relocations and add browser publish coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Composite mode is silently accepted during ordinary builds, and stale per-assembly R2R outputs may remain after switching to composite publishing.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This PR enables composite ReadyToRun publishing for CoreCLR browser-WASM with WebCIL, including runtime loading, asset routing, relocation handling, and browser coverage.
Changes:
- Routes composite assemblies through CoreCLR startup while retaining component stubs in the TPA.
- Adds composite R2R staging, fingerprinting, invalidation, and WebCIL relocation support.
- Adds browser tests and documentation.
Two moderate issues remain in CoreCLR target validation and stale R2R output cleanup.
| File | Summary |
|---|---|
src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/GenerateWasmBootJson.cs |
Classifies composite images as core assemblies. |
src/native/libs/Common/JavaScript/loader/run.ts |
Loads composite core assemblies before CoreCLR initialization. |
src/native/libs/Common/JavaScript/loader/assets.ts |
Preserves composite .r2r.wasm paths. |
src/native/libs/Common/JavaScript/host/host.ts |
Excludes composite images from the TPA. |
src/mono/wasm/Wasm.Build.Tests/ReadyToRunTests.cs |
Adds composite publishing and browser coverage. |
src/mono/nuget/Microsoft.NET.Sdk.WebAssembly.Pack/build/Microsoft.NET.Sdk.WebAssembly.Browser.targets |
Publishes composite assets and manages invalidation stamps. |
src/mono/nuget/Microsoft.NET.Sdk.WebAssembly.Pack/build/Microsoft.NET.Sdk.WebAssembly.Browser.CoreCLR.targets |
Adds composite routing, validation, and output handling. |
src/coreclr/tools/Common/Compiler/ObjectWriter/WebCilObjectWriter.cs |
Handles WebCIL function-index relocations. |
docs/workflow/testing/libraries/testing-wasm.md |
Documents composite ReadyToRun testing. |
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
The loader now decides once, while fetching resources.coreAssembly, whether an asset is the composite ReadyToRun owner and records it on the asset; the host builds the TPA from that flag instead of re-deriving it from the .r2r.wasm suffix. The composite test app now calls into the referenced R2rSuffixLibrary.r2r at startup and asserts it loaded, so an ordinary library whose name ends in .r2r is verified to load as a managed component rather than only checked in the boot manifest. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@jkotas @davidwrighton Could you please explain what does "composite" mean technically for WASM ? My mental map:
We can also ship
A) is opt-in until we fix all/most library tests. In my testing of C) so far, loading the second half (of the same assembly) doesn't work for various reasons. I have not tested single-file composite yet. The challenge for E) F) is with I think C) is most likely, but I would not like to give up on possibility of F) just yet. |
Stock SDK ReadyToRun tasks name the browser composite owner '<entry>.r2r.dll' and plan component outputs as '<name>.dll', while crossgen2 emits '<entry>.r2r.wasm' plus '<name>.wasm' stubs. Fail fast with an actionable error instead of a downstream ConvertDllsToWebcil file-not-found. Ship the Crossgen2Tasks shim to BuildWasmApps Helix payloads and use it only for composite Wasm.Build.Tests so per-assembly R2R keeps stock SDK coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Boot config: flag the composite owner with isCompositeImage (new WasmResource/readyToRunComposite trait) instead of the loader inferring it from the .r2r.wasm file name. - Define the composite owner static web asset with DefineStaticWebAssets rather than a hand-built candidate. - Stale-prune keeps what crossgen2 actually emitted (compile outputs plus composite component stubs), not the task's publish plan. - Move R2rSuffixLibrary into testassets; GetBootConfigPath picks the newest fingerprinted dotnet.js so composite assertions use the shared helper. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@pavelsavara, really what composite means is that compilation is performed as a single R2R step which logically compiles it all at once. Yes, this comes with a non-trivial build cost, but it also allows us to actually get the set of R2R code to compile to be about as close to "right" as we can. Currently, it also means that all of the actual code gets glommed into 1 binary (the metadata/original IL is split across a series of other files). As you note, the code all being in 1 binary is probably not ideal, but we can probably address that with changes to the compiler itself (the sharding idea we've been talking about) |
davidwrighton
left a comment
There was a problem hiding this comment.
I'd like to understand the new reloc handling.
The composite mode allows us to take advantage of full program view. It is why it produces output that is as close to "right".
The way to think about it: There is one logical monolith that is physically split into two or more shards using some set of policies (e.g. the simplest policy is one shard with hot startup code, second shard with everything else). |
The relocation only arises with #134555, where a same-image managed helper is returned as IAT_VALUE but genEmitHelperCall still loads the PEP through an indirection cell. Encoding it as a function index would load from imageBase + funcIndex, so the fix belongs in the JIT. On main no code path emits it for composite images. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
I think we should hold off on switching the default until @maraf has landed the sdk changes to the r2r targets we need for the r2r.wasm extension. I can add libraries CI covereage in this pr or a follow up, it probably makes sense to do it in a follow up to make sure we understand the build times. we can keep the non composite in the rolling builds too to make sure we don't regress the codegen. After that I'll make sure the perf pipeline is working for composite and enable it. We have enough perf hardware now to run any interesting wasm configurations while we characterize the different profiles. |
#132721 duplicated the composite and container-format errors into _WasmCoreClrSelectR2RDirectories. That brought back the composite rejection this PR removes and failed PublishRunAllPagesComposite on CI. Keep the checks in _WasmCoreClrValidateReadyToRun, which runs first, and move the new WasmPerformanceInstrumentation check there. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ish (#134689) ## Summary Pass `--strip-debug-info` to crossgen2 for CoreCLR browser-wasm ReadyToRun publish. This drops the R2R `DebugInfo` section (native-to-IL offset maps and variable locations) from shipped images. Opt out with `PublishReadyToRunStripDebugInfo=false` to keep the data for debugging R2R code (e.g. the cDAC work in #133086 / #133890). Also makes crossgen2 argument changes invalidate per-app R2R images. `_CreateR2RImages` only tracks file inputs, so toggling `PublishReadyToRunStripDebugInfo` (or any `PublishReadyToRunCrossgen2ExtraArgs` change) previously left stale images in `obj/R2R`. The arguments are now written to `obj/wasm-r2r-args.stamp` (only when different) and added to `_ReadyToRunCompilerInputs`, mirroring the existing P/Invoke manifest input. > [!IMPORTANT] > Stacked on #134618, which rewrites the same targets file. Retarget to `main` after it merges. ## Size impact Per-assembly R2R images compiled directly with crossgen2 using the SDK's browser-wasm arguments (`--obj-format:wasm --opt-cross-module:* --codegenopt:JitWasm*NyiToR2RUnsupported=1`), with and without `--strip-debug-info`. Total for System.Private.CoreLib, System.Text.Json, System.Linq, and System.Collections: | Compiler | Raw saved | gzip -9 saved | brotli -q 11 saved | | --- | --- | --- | --- | | Current (#134618 base) | 859,712 B (2.4%) | 624,690 B (7.1%) | 565,366 B (9.4%) | | With #133086 variable info | 2,105,520 B (5.6%) | 1,332,733 B (13.9%) | 1,121,574 B (17.0%) | <details> <summary>Per-assembly brotli sizes (bytes, keep → strip)</summary> | Assembly | Current | With #133086 | | --- | --- | --- | | System.Private.CoreLib | 4,822,231 → 4,347,541 | 5,331,799 → 4,403,849 | | System.Text.Json | 823,314 → 761,533 | 887,982 → 754,444 | | System.Linq | 219,177 → 200,471 | 238,853 → 200,441 | | System.Collections | 119,635 → 109,446 | 130,742 → 109,068 | The #133086 compiler is based on an older commit, so compare keep vs. strip within a column rather than across columns. </details> ## Validation - MSBuild evaluation: `--strip-debug-info` is present by default, absent with `PublishReadyToRunStripDebugInfo=false`, and absent when `PublishReadyToRun` is off. - Incremental harness: `_CreateR2RImages` runs on first build, skips when unchanged, reruns on opt-out, skips when repeated, and reruns when switching back. - Not yet run: an end-to-end browser-wasm publish loading stripped images in a browser (Wasm.Build.Tests in CI will cover this). Runtime-pack framework R2R images (used only by the dev-loop build) are unchanged; publish recompiles the whole closure through these targets. `--strip-inlining-info` is intentionally not included: it removes `CrossModuleInlineInfo`, and cross-module inlining is load-bearing on wasm, so it needs separate validation. > [!NOTE] > This pull request was created with assistance from GitHub Copilot. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| <PublishReadyToRunCrossgen2ExtraArgs>$(PublishReadyToRunCrossgen2ExtraArgs);--codegenopt:JitWasmNyiToR2RUnsupported=1;--codegenopt:JitWasmSimdNyiToR2RUnsupported=1</PublishReadyToRunCrossgen2ExtraArgs> | ||
| <!-- Strip R2R debug info (IL offset maps and variable locations) to minimize download size. | ||
| Set PublishReadyToRunStripDebugInfo=false to keep it for debugging R2R code. --> | ||
| <PublishReadyToRunCrossgen2ExtraArgs Condition="'$(PublishReadyToRunStripDebugInfo)' != 'false'">$(PublishReadyToRunCrossgen2ExtraArgs);--strip-debug-info</PublishReadyToRunCrossgen2ExtraArgs> |
There was a problem hiding this comment.
#134690 adds both --strip-inlining-info and --strip-debug-info in different places.
There was a problem hiding this comment.
I've removed the change here and opened dotnet/sdk#56411 since the targets in #134690 are not used in customer builds
#134690 adds --strip-debug-info and --strip-inlining-info for browser and wasi ReadyToRun where the other strip defaults live, so the wasm pack shouldn't add its own. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Composite component stubs are written as <name>.wasm, the same names as per-assembly images. After a composite publish, a per-assembly republish (-p:PublishReadyToRunComposite=false) found them up to date, pruned the composite owner, and shipped stubs that fail startup. Add the mode to the crossgen2 configuration stamp, and switch modes in the trimmed composite test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary
PublishReadyToRunCompositefor CoreCLR browser-WASM when WebCIL is enabledresources.coreAssemblybefore CoreCLR initialization, while keeping component stubs in the TPAcoreAssemblyentry named<entry>.r2r.wasm, the exact name Crossgen2 records in each component stub) and the host builds the TPA from that decision, so an ordinary library named*.r2rstill loads as a managed assemblyComposite is opt-in (
PublishReadyToRunComposite=true); the default per-assembly R2R mode is unchanged.Validation
clr+libs+host) and WebAssembly SDK package rebuilt: 0 warnings, 0 errorsR2rSuffixLibrary.r2rat startup and asserts it loadedFileNotFoundException: Could not load file or assembly 'R2rSuffixLibrary.r2r'git diff --check: cleanFinal packages: browser runtime pack
f07ed33026558a52bd39c9aa36e33cca0c925b67, SDK pack65881c2d3d681c346d1d3427f74e92abe44b8e3f.Benchmark (
IndexOfMax<double>(3079), browser): composite R2R ran the SIMD path with a median of 4,040 ns/op, vs. 1,013,940 ns/op interpreted and 10,652,060 ns/op with per-assembly R2R. These numbers were measured before the final loader cleanup, using a runtime pack whose loader still classified the owner by suffix alone. That difference does not affect the benchmark's codegen or composite load path.Part of testing #134559. That issue's default per-assembly scenario is not changed by this PR.
Note
This pull request description was generated with GitHub Copilot assistance.