Repository navigation
Trim netcoreapp libraries against implementations - #136495
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @agocke, @dotnet/illink |
63ecb76 to
78656c1
Compare
|
@sbomer Could you please take a look? |
# Conflicts: # src/coreclr/vm/wasm/wasi/callhelpers-pinvoke.cpp
sbomer
left a comment
There was a problem hiding this comment.
Do we need to somehow prevent baking in constants from an OOB library that might change across versions?
- Use the reference assembly of packable libraries, which can be replaced by a newer package at run time. - Trim after CoreCompile with the implementation references as inputs, so changes to them are picked up incrementally. - Don't disable the optimization when BuildAllConfigurations is set, which -pack sets for shipping builds. - Allow --ignore-substitutions in the cacheable ILLink task arguments. - Make not updating reference scopes the default in library mode instead of a new option.
Packable libraries now contribute their reference assembly instead of the implementation. |
- Don't trim packable libraries against implementations, as their assets run on any OS. - Build the best flavor separately so that its implementation isn't added to the closure.
There was a problem hiding this comment.
🟡 Changes recommended
Disabling scope rewriting for all library-mode runs can leave broken references when dependency sweeping removes a required facade or forwarder.
2 open issues
What changed in this PR
This PR generalizes the OS-check trimming approach from #136476 by trimming shared-framework libraries against implementation assemblies, enabling more constant propagation and reporting a 25,088-byte reduction across DLLs.
Changes:
- Resolves target-OS implementation dependencies and reruns trimming when those inputs change.
- Preserves original reference scopes and prevents library builds from folding unset feature switches.
- Updates cache-argument handling, adds task tests, and removes unused WASI interop entries and thunks.
| File | Description |
|---|---|
src/tools/illink/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs |
Tests cache-key handling of substitution options. |
src/tools/illink/src/linker/Linker/MemberActionStore.cs |
Detects feature-switch getters without explicit settings. |
src/tools/illink/src/linker/Linker/LinkContext.cs |
Tracks library roots and reference-scope policy. |
src/tools/illink/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs |
Prevents folding unset feature switches in library mode. |
src/tools/illink/src/linker/Linker.Steps/SweepStep.cs |
Supports retaining original assembly reference scopes. |
src/tools/illink/src/linker/Linker.Steps/RootAssemblyInputStep.cs |
Enables scope preservation for library roots. |
src/tools/illink/src/ILLink.Tasks/LinkTask.cs |
Allows substitution options in cacheable arguments. |
src/coreclr/vm/wasm/wasi/callhelpers-pinvoke.cpp |
Removes unused native entries and updates consumer comments. |
src/coreclr/vm/wasm/wasi/callhelpers-interp-to-managed.cpp |
Removes two unused call-helper signatures. |
eng/illink.targets |
Resolves implementation dependencies and coordinates incremental trimming. |
Review effort: Balanced
|
"The checked-in CoreCLR WebAssembly call helpers" build failure looks related to the changes in the PR |
|
ILLink tests failing |
|
@jkotas that failure was ILTrim (CLR_Tools_Tests) reusing the Mono.Linker case — ILTrim doesn't fold trivial feature-switch constants yet, so the case is on |
|
@sbomer Could you please do another review (additional changes made since your signoff)? |
33bdabc to
26dfdb5
Compare
26dfdb5 to
aad7305
Compare
A feature switch with a constant body can still be substituted at application publish time. Instead, pass IsMultithreadingSupported=false explicitly for the single-threaded browser and wasi flavors, and allow --feature in the cacheable ILLink arguments. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Wasi failure is unrelated (infra issue). |
|
/ba-g "deadletter" |


Generic version of @#136476.
main