[dotnet] Default PublishTrimmed=false when there's nothing for the trimmer to do.#25909
[dotnet] Default PublishTrimmed=false when there's nothing for the trimmer to do.#25909rolfbjarne wants to merge 2 commits into
Conversation
…immer to do. Previously we always defaulted PublishTrimmed to true (except when building from Windows without a connection to a Mac), and disallowed setting it to false. Now default PublishTrimmed to false when all of the following are true: * No assemblies are being trimmed (TrimMode is 'copy'). * The link mode is 'None'. * Both PrepareAssemblies and PostProcessAssemblies are 'true'. When both PrepareAssemblies and PostProcessAssemblies are true, all the custom trimmer steps are moved to the assembly-preparer's post-processing step instead of running inside the trimmer. Combined with a link mode of 'None' (nothing is trimmed), there's nothing left for the trimmer (ILLink) to do, so we can skip it entirely. Skipping the trimmer meant the AOT dedup assembly (aot-instances.dll) was no longer created, because _CreateAOTDedupAssembly runs BeforeTargets=PrepareForILLink, and PrepareForILLink doesn't run when the trimmer is skipped. Fix this by making _AddDedupAssemblyToResolvedFileToPublish depend on _CreateAOTDedupAssembly, so the dedup assembly is created before the assemblies are post-processed regardless of whether the trimmer runs. Perf numbers are kind of all over the place, probably due to randomness on my machine, but this should eventually speed up the build once the PrepareAssembly task is optimized. Before: | Project | Runtime identifier | Link mode | PrepareAssemblies=false | PrepareAssemblies=true | Difference | | -------------- | ------------------ | --------- | ----------------------- | ---------------------- | ----------- | | monotouch-test | ios-arm64 | None | 00:02:05.46 | 00:02:09.64 | 00:00:04.18 | | monotouch-test | iossimulator-arm64 | None | 00:01:34.26 | 00:01:38.97 | 00:00:04.70 | | MySimpleApp | ios-arm64 | None | 00:01:20.30 | 00:01:23.26 | 00:00:02.96 | | MySimpleApp | iossimulator-arm64 | None | 00:00:53.64 | 00:00:55.38 | 00:00:01.74 | After: | Project | Runtime identifier | Link mode | PrepareAssemblies=false | PrepareAssemblies=true | Difference | | -------------- | ------------------ | --------- | ----------------------- | ---------------------- | ----------- | | monotouch-test | ios-arm64 | None | 00:02:06.53 | 00:02:12.52 | 00:00:05.99 | | monotouch-test | iossimulator-arm64 | None | 00:01:36.81 | 00:01:36.34 | 00:00:00.46 | | MySimpleApp | ios-arm64 | None | 00:01:14.63 | 00:01:19.11 | 00:00:04.48 | | MySimpleApp | iossimulator-arm64 | None | 00:00:54.26 | 00:00:53.07 | 00:00:01.19 | Also change the TimeSpan formatting in the performance report to print 2 fractional-second digits instead of 7. Even 2 is way too much precision, but it looks nicely symmetrical with the other double-digit numerical values. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the .NET-for-Apple MSBuild trimming pipeline to avoid invoking ILLink when it has no work to do (no trimming + trimmer custom steps moved to assembly-preparer), while ensuring the AOT dedup assembly still gets generated and adjusting perf-report formatting.
Changes:
- Default
PublishTrimmed=falsewhen link mode isNone,TrimMode=copy, and bothPrepareAssemblies+PostProcessAssembliesare enabled (so ILLink can be skipped). - Ensure
aot-instances.dllis created even when ILLink is skipped by making_AddDedupAssemblyToResolvedFileToPublishdepend on_CreateAOTDedupAssembly. - Add a regression test for the new defaulting behavior and reduce
TimeSpanprecision in performance reports.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/dotnet/UnitTests/PublishTrimmedTest.cs | Adds a unit test asserting PublishTrimmed defaults to false when the trimmer can be skipped. |
| tests/dotnet/UnitTests/PerformanceTests.cs | Changes perf-report TimeSpan formatting to show fewer fractional-second digits. |
| dotnet/targets/Xamarin.Shared.Sdk.targets | Implements the new PublishTrimmed defaulting logic and fixes dedup assembly generation ordering when ILLink is skipped. |
| assembly-preparer's post-processing step instead), then there's nothing for the trimmer to do, so | ||
| default PublishTrimmed to false to skip running the trimmer entirely. | ||
| --> | ||
| <_CanSkipTrimmer Condition="'$(_CanSkipTrimmer)' == '' And '$(_LinkMode)' == 'None' And '$(TrimMode)' == 'copy' And '$(PrepareAssemblies)' == 'true' And '$(PostProcessAssemblies)' == 'true'">true</_CanSkipTrimmer> |
| } | ||
| } | ||
|
|
||
| static string FormatTimeSpan (TimeSpan value) => value.ToString (@"hh\:mm\:ss\.ff"); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…king.
The test read the 'PublishTrimmed' property value from the binlog, but that
property is computed inside the '_ComputePublishTrimmed' target. Property
values assigned inside a target are only logged in the binlog when property
tracking is enabled (via the 'MsBuildLogPropertyTracking' environment
variable). This isn't the case on CI, so the test failed there while passing
locally for anyone who happens to have that environment variable set.
Assert that the trimmer ('ILLink' target) didn't execute instead, which is
the actual behavior being verified and is independent of property tracking.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #1bb00c1] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #1bb00c1] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [PR Build #1bb00c1] Build passed (Build macOS tests) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
🔥 [CI Build #1bb00c1] Test results 🔥Test results❌ Tests failed on VSTS: test results 1 tests crashed, 0 tests failed, 184 tests passed. Failures❌ linker tests (iOS)🔥 Failed catastrophically on VSTS: test results - linker_ios (no summary found). Html Report (VSDrops) Download Successes✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Previously we always defaulted PublishTrimmed to true (except when building from Windows without a connection to a Mac), and disallowed setting it to false.
Now default PublishTrimmed to false when all of the following are true:
When both PrepareAssemblies and PostProcessAssemblies are true, all the custom trimmer steps are moved to the assembly-preparer's post-processing step instead of running inside the trimmer. Combined with a link mode of 'None' (nothing is trimmed), there's nothing left for the trimmer (ILLink) to do, so we can skip it entirely.
Skipping the trimmer meant the AOT dedup assembly (aot-instances.dll) was no longer created, because _CreateAOTDedupAssembly runs BeforeTargets=PrepareForILLink, and PrepareForILLink doesn't run when the trimmer is skipped. Fix this by making _AddDedupAssemblyToResolvedFileToPublish depend on _CreateAOTDedupAssembly, so the dedup assembly is created before the assemblies are post-processed regardless of whether the trimmer runs.
Perf numbers are kind of all over the place, probably due to randomness on my machine, but this should eventually speed up the build once the PrepareAssembly task is optimized.
Before:
After:
Also change the TimeSpan formatting in the performance report to print 2 fractional-second digits instead of 7. Even 2 is way too much precision, but it looks nicely symmetrical with the other double-digit numerical values.