[dotnet-linker] Don't preserve base type members for [Preserve (AllMembers = true)]. - #26362
Open
rolfbjarne wants to merge 1 commit into
Open
[dotnet-linker] Don't preserve base type members for [Preserve (AllMembers = true)].#26362rolfbjarne wants to merge 1 commit into
rolfbjarne wants to merge 1 commit into
Conversation
…mbers = true)]. When the assembly preparer applies `[Preserve (AllMembers = true)]` (and when it preserves NSObject subclasses in user assemblies), it used to emit a single `[DynamicDependency (DynamicallyAccessedMemberTypes.…, typeof (TheType))]` attribute on the module constructor. That's not equivalent to `[Preserve (AllMembers = true)]`: the trimmer resolves `DynamicallyAccessedMemberTypes` over the entire type hierarchy, so every member of every base type is preserved as well. For an NSObject subclass that means all of `NSObject` is preserved - and `NSObject` in turn drags in `NSNumber`, `NSValue`, `NSArray`, `ObjCRuntime.Messaging`, `System.Drawing.PointF/SizeF/ RectangleF` and, transitively, the `System.Drawing`, `System.Drawing.Primitives`, `System.ObjectModel` and `System.ComponentModel.TypeConverter` assemblies. In practice this was triggered by the `[Preserve (AllMembers = true)]` attribute on the internal `NSObject.NSObject_Disposer` class, which has 12 members of its own but ended up preserving 121 members of `NSObject` and 8 members of `System.Object`. Emit one `DynamicDependency` attribute per member declared on the type instead, which matches `[Preserve (AllMembers = true)]` semantics exactly. This only affects the assembly preparer (PrepareAssemblies=true); when our steps run inside ILLink we use an xml descriptor file, which already had the correct semantics. Size impact for tests/dotnet/SizeTestApp/iOS (net11.0, ios-arm64, Release, CoreCLR + ReadyToRun, trimmable-static + PrepareAssemblies + PostProcessAssemblies): | Configuration | App size | |-------------------------------------|--------------| | Before | 11,523,224 B | | After | 11,380,424 B | | **Difference** | **-142,800 B (-1.24%)** | which brings trimmable-static to within 0.38% of managed-static (11,337,504 B), down from 1.6%. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6aeeddfd-3bd8-4129-ad90-7292860c4b66
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts the dotnet-linker assembly preparer behavior so [Preserve (AllMembers = true)] no longer over-preserves members from base types via a single DynamicDependency(DynamicallyAccessedMemberTypes…, typeof(T)). Instead, it emits one DynamicDependency per member declared on the preserved type, matching Preserve semantics more closely and reducing app size.
Changes:
- Switch Preserve-all-members logic in the assembly preparer to emit per-member
DynamicDependencyattributes. - Keep the “constructors only” behavior for non-
AllMemberspreservation. - Introduce a shared helper (
AddPreserveAllMembersDynamicDependencyAttributes) inAppBundleRewriter.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tools/dotnet-linker/MarkNSObjectsStep.cs | Uses the new per-member preservation helper when allMembers == true. |
| tools/dotnet-linker/ApplyPreserveAttributeStep.cs | Uses the new helper for [Preserve(AllMembers=true)] and keeps enum handling as fields-only. |
| tools/dotnet-linker/AppBundleRewriter.cs | Adds the helper that expands preserve-all-members into per-member DynamicDependency attributes. |
Comment on lines
+1602
to
+1603
| if (signatures.Count == 0) | ||
| return AddAttributeOnlyOnce (addToMethod, CreateDynamicDependencyAttribute (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor, type)); |
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.
Collaborator
🔥 [PR Build #c937fde] Build failed (Build macOS tests) 🔥Build failed for the job 'Build macOS tests' (with job status 'Failed') Pipeline on Agent |
Collaborator
✅ 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 |
Collaborator
🔥 [CI Build #c937fde] Test results 🔥Test results❌ Tests failed on VSTS: test results 5 tests crashed, 68 tests failed, 110 tests passed. Failures❌ dotnettests tests (iOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ dotnettests tests (MacCatalyst)1 tests failed, 0 tests passed.Failed tests
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)5 tests failed, 10 tests passed.Failed tests
Html Report (VSDrops) Download ❌ linker tests (MacCatalyst)5 tests failed, 10 tests passed.Failed tests
Html Report (VSDrops) Download ❌ linker tests (macOS)2 tests failed, 19 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (iOS)19 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (MacCatalyst)6 tests failed, 12 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (macOS)7 tests failed, 12 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (tvOS)19 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ msbuild tests1 tests failed, 1 tests passed.Failed tests
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 |
Collaborator
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When the assembly preparer applies
[Preserve (AllMembers = true)](and whenit preserves NSObject subclasses in user assemblies), it used to emit a single
[DynamicDependency (DynamicallyAccessedMemberTypes.…, typeof (TheType))]attribute on the module constructor.
That's not equivalent to
[Preserve (AllMembers = true)]: the trimmer resolvesDynamicallyAccessedMemberTypesover the entire type hierarchy, so every memberof every base type is preserved as well. For an NSObject subclass that means all
of
NSObjectis preserved - andNSObjectin turn drags inNSNumber,NSValue,NSArray,ObjCRuntime.Messaging,System.Drawing.PointF/SizeF/ RectangleFand, transitively, theSystem.Drawing,System.Drawing.Primitives,System.ObjectModelandSystem.ComponentModel.TypeConverterassemblies.In practice this was triggered by the
[Preserve (AllMembers = true)]attributeon the internal
NSObject.NSObject_Disposerclass, which has 12 members of itsown but ended up preserving 121 members of
NSObjectand 8 members ofSystem.Object.Emit one
DynamicDependencyattribute per member declared on the type instead,which matches
[Preserve (AllMembers = true)]semantics exactly.This only affects the assembly preparer (PrepareAssemblies=true); when our steps
run inside ILLink we use an xml descriptor file, which already had the correct
semantics.
Size impact for tests/dotnet/SizeTestApp/iOS (net11.0, ios-arm64, Release,
CoreCLR + ReadyToRun, trimmable-static + PrepareAssemblies + PostProcessAssemblies):
which brings trimmable-static to within 0.38% of managed-static (11,337,504 B),
down from 1.6%.
Copilot-Session: 6aeeddfd-3bd8-4129-ad90-7292860c4b66