[linker] Convert the EnsureUIThread optimization to a trimmer feature switch. Fixes #26104.#26140
Conversation
… switch Replace the hand-written IL rewrite of `[NS|UI]Application.EnsureUIThread` with an `ObjCRuntime.Runtime.CheckForIllegalCrossThreadCalls` trimmer feature switch, driven by a new `$(CheckForIllegalCrossThreadCalls)` MSBuild property (defaults to the build configuration: on for debug, off for release). * Remove `ProcessEnsureUIThread` (and its case) from the linker's OptimizeGeneratedCode step. * Emit the `ObjCRuntime.Runtime.CheckForIllegalCrossThreadCalls` RuntimeHostConfigurationOption from Xamarin.Shared.Sdk.targets and stub the `EnsureUIThread` method body via ILLink substitutions on all four platforms when the feature is off. * Document the new `$(CheckForIllegalCrossThreadCalls)` property. Also make `[NS|UI]Application.CheckForIllegalCrossThreadCalls` trimmable when the checks are off, without an API break: the field's `= true` initializer rooted it permanently. Instead, add an internal feature-switched `Runtime.CheckForIllegalCrossThreadCalls` getter (with matching substitutions) and set the public field to true from the application startup path, guarded by `if (Runtime.CheckForIllegalCrossThreadCalls)`. When the feature is off the getter substitutes to a constant `false`, so the guarded assignment is dead-code-eliminated and — with `EnsureUIThread` also stubbed — the field can be trimmed away entirely. Add EnsureUIThreadChecksTest (with a dedicated EnsureUIThreadApp project that keeps EnsureUIThread reachable) verifying the feature switch value and that, in the app bundle, EnsureUIThread is stubbed and the field is trimmed when the checks are off (iOS + macOS). Fixes #26104 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The EnsureUIThread IL rewrite (its only consumer) is gone, so the `--optimize=remove-uithread-checks` mtouch/mmp flag no longer does anything. Follow the existing `inline-intptr-size` convention: keep the option name registered (so passing it doesn't error) but empty its valid platforms so it degrades to an MT2003 warning, and drop the now-dead default. Update optimizations.md to point at the replacement `CheckForIllegalCrossThreadCalls` MSBuild property. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR moves the [NS|UI]Application.EnsureUIThread “remove UI thread checks” behavior from a hand-authored linker IL rewrite to an ILLink feature switch (ObjCRuntime.Runtime.CheckForIllegalCrossThreadCalls) driven by a new $(CheckForIllegalCrossThreadCalls) MSBuild property (defaulting to enabled in Debug and disabled in Release). This aligns the behavior with other trimmer feature switches and enables ILLink to stub/check-eliminate more effectively.
Changes:
- Remove the linker’s
ProcessEnsureUIThreadrewrite and mark--optimize=remove-uithread-checksas no longer applicable. - Add the
ObjCRuntime.Runtime.CheckForIllegalCrossThreadCallsruntime feature switch, plus ILLink substitutions to stubEnsureUIThreadwhen disabled. - Add an end-to-end unit test app + test to validate the emitted feature switch and the resulting linked assembly shape; update docs to describe the new MSBuild property and deprecate the old optimize flag.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/linker/OptimizeGeneratedCode.cs | Removes the special-case IL rewrite for EnsureUIThread. |
| tools/common/Optimizations.cs | Keeps the old optimize flag name but makes it non-applicable (removed) and removes its old defaulting logic. |
| tests/dotnet/UnitTests/EnsureUIThreadChecksTest.cs | Adds an end-to-end test validating the feature switch and that EnsureUIThread is stubbed / field trimmed when disabled. |
| tests/dotnet/EnsureUIThreadApp/iOS/EnsureUIThreadApp.csproj | Adds a minimal test app project for iOS. |
| tests/dotnet/EnsureUIThreadApp/tvOS/EnsureUIThreadApp.csproj | Adds a minimal test app project for tvOS. |
| tests/dotnet/EnsureUIThreadApp/macOS/EnsureUIThreadApp.csproj | Adds a minimal test app project for macOS. |
| tests/dotnet/EnsureUIThreadApp/MacCatalyst/EnsureUIThreadApp.csproj | Adds a minimal test app project for Mac Catalyst. |
| tests/dotnet/EnsureUIThreadApp/shared.csproj | Shared project settings + compilation inputs for the test app. |
| tests/dotnet/EnsureUIThreadApp/EnsureUIThreadApp.cs | References EnsureUIThread to keep it reachable for inspection in linked output. |
| tests/dotnet/EnsureUIThreadApp/Directory.Build.props | Imports the parent Directory.Build.props for consistent test infrastructure settings. |
| tests/assembly-preparer/OptimizeGeneratedCodeHandlerTests.cs | Removes tests for the deleted IL rewrite and adjusts coverage to other optimizations. |
| src/UIKit/UIApplication.cs | Makes CheckForIllegalCrossThreadCalls trimmable and ties initialization to the feature switch. |
| src/AppKit/NSApplication.cs | Makes CheckForIllegalCrossThreadCalls trimmable and ties initialization to the feature switch. |
| src/ObjCRuntime/Runtime.cs | Adds an optimizable getter wrapping the feature switch for ILLink substitution. |
| src/ILLink.Substitutions.iOS.xml | Adds substitutions to constant-fold the feature switch and stub UIApplication.EnsureUIThread when disabled. |
| src/ILLink.Substitutions.tvOS.xml | Same substitutions for tvOS. |
| src/ILLink.Substitutions.MacCatalyst.xml | Same substitutions for Mac Catalyst. |
| src/ILLink.Substitutions.macOS.xml | Same substitutions for macOS (NSApplication.EnsureUIThread). |
| dotnet/targets/Xamarin.Shared.Sdk.targets | Emits the RuntimeHostConfigurationOption based on $(CheckForIllegalCrossThreadCalls) with Debug/Release defaults. |
| docs/website/optimizations.md | Notes the optimize flag is replaced and points users to the MSBuild property. |
| docs/building-apps/build-properties.md | Documents the new CheckForIllegalCrossThreadCalls property and its trimming/runtime effects. |
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.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…convert-ensureuithread-optimization-to-a-5f492b
…nstant folding
The DeadCodeElimination and NoOptimizationWithoutBindingAttributes tests used
`if (true) { return 1; } return 2;` as the dead-code sample, but the C# compiler
itself constant-folds `if (true)` and never emits the `return 2` (ldc.i4.2) in
the IL. As a result NoOptimizationWithoutBindingAttributes failed (there was no
dead code to preserve) and DeadCodeElimination passed only trivially (the dead
code was already gone before the optimizer ran).
Use `if (Runtime.IsARM64CallingConvention)` instead: Roslyn can't fold it, so the
`return 2` survives compilation. The assembly-preparer's InlineIsARM64CallingConvention
optimization inlines the field access to a constant, after which dead-code
elimination removes the unreachable branch - but only when the method is
optimizable ([BindingImpl(Optimizable)]). This exercises both the inline and the
dead-code-elimination passes and the [BindingImpl] gating.
The test assembly is now built with <Optimize>true</Optimize> (so the condition
is a direct `ldsfld; brfalse` rather than a debug-mode stloc/ldloc round-trip that
defeats constant folding), and with TargetArchitectures=ARM64 so the ARM64
calling-convention value is known. Add extraCsproj/extraConfig passthrough to
BaseClass.AssertPrepareCode to support this.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4fdcea25-bb9d-4fd5-b750-c47bff394b74
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ 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 |
🔥 [PR Build #ca53f18] Build failed (Build macOS tests) 🔥Build failed for the job 'Build macOS tests' (with job status 'Canceled') Pipeline on Agent |
🔥 [CI Build #ca53f18] Test results 🔥Test results❌ Tests failed on VSTS: test results 8 tests crashed, 3 tests failed, 141 tests passed. Failures❌ dotnettests tests (iOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ dotnettests tests (MacCatalyst)🔥 Failed catastrophically on VSTS: test results - dotnettests_maccatalyst (no summary found). Html Report (VSDrops) Download ❌ dotnettests tests (macOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ dotnettests tests (tvOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ linker tests (iOS)🔥 Failed catastrophically on VSTS: test results - linker_ios (no summary found). Html Report (VSDrops) Download ❌ monotouch tests (MacCatalyst)🔥 Failed catastrophically on VSTS: test results - monotouch_maccatalyst (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Monterey (12) testsHtml Report (VSDrops) Download ❌ Tests on macOS Ventura (13) testsHtml Report (VSDrops) Download ❌ Tests on macOS Sonoma (14) testsHtml Report (VSDrops) Download ❌ Tests on macOS Sequoia (15) testsHtml Report (VSDrops) Download ❌ Tests on macOS Tahoe (26) testsHtml Report (VSDrops) Download Successes✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS testsLinux Build VerificationPipeline on Agent |
Replace the hand-written IL rewrite of
[NS|UI]Application.EnsureUIThreadwith anObjCRuntime.Runtime.CheckForIllegalCrossThreadCallstrimmer feature switch, driven by a new$(CheckForIllegalCrossThreadCalls)MSBuild property (defaults to the build configuration: on for debug, off for release).ProcessEnsureUIThread(and its case) from the linker'sOptimizeGeneratedCodestep, and mark the now-orphaned--optimize=remove-uithread-checksflag as removed (following the existinginline-intptr-sizeconvention).ObjCRuntime.Runtime.CheckForIllegalCrossThreadCallsRuntimeHostConfigurationOptionfromXamarin.Shared.Sdk.targetsand stub theEnsureUIThreadmethod body via ILLink substitutions on all four platforms when the feature is off.[NS|UI]Application.CheckForIllegalCrossThreadCallstrimmable when the checks are off, without an API break: drop the field's= trueinitializer and instead set it from the application startup path, guarded by an internal feature-switchedRuntime.CheckForIllegalCrossThreadCallsgetter. When the feature is off the getter substitutes to a constantfalse, so the assignment is dead-code-eliminated and — withEnsureUIThreadalso stubbed — the field can be trimmed away entirely.$(CheckForIllegalCrossThreadCalls)property and updateoptimizations.md.Because the property is emitted as a runtime feature switch, it also takes effect when trimming is disabled (the field reflects the property value at runtime); trimming is only needed to physically remove the check code.
Add
EnsureUIThreadChecksTest(with a dedicatedEnsureUIThreadAppproject that keepsEnsureUIThreadreachable) verifying the feature switch value and that, in the app bundle,EnsureUIThreadis stubbed and the field is trimmed when the checks are off (iOS + macOS).Fixes #26104
🤖 Pull request created by Copilot