[net10.0] [dotnet] Rework aggressive attribute trimming. Fixes #20906. #21314

rolfbjarne · 2024-09-26T15:41:31Z

The aggressive attribute trimming as exposed by ILLink is undocumented and has
problematic behavior, so disable it (it can't be statically validated) -
ILLink already removed support for it anyway.

However, this has a somewhat significant size increase for our apps, and it
would affect pretty much every app, so re-implement the aggressive attribute
trimming ourselves.

This is accomplished by copying ILLink's list of attributes to remove into our
own xml definition, which we then pass to ILLink.

I tested a release build of monotouch-test for all platforms, and the final
size is almost entirely unchanged, except for a minimal size decrease.

Here's the results for iOS: https://gist.github.com/rolfbjarne/dac3e08620a3ae499ad1fc134765e0d2

Fixes #20906.

dalexsoto · 2024-09-27T12:13:30Z

src/TrimAttributes.LinkDescription.xml

+        Note that removing this attribute can change runtime behavior. For example,
+        System.Xml.Serialization will behave differently if a ctor is Obsolete.
+        This is low enough risk on when 'ObjCRuntime.AggressiveAttributeTrimming' is enabled to justify
+        removing the attribute for size savings. The app developer can override this setting
+        to keep all ObsoleteAttributes.


Should we document this somewhere outside this file? /docs maybe? and maybe link it from build-properties.md entry you added

@dalexsoto I expanded a bit on this in the build-properties.md doc.

The aggressive attribute trimming as exposed by ILLink is undocumented and has problematic behavior, so disable it (it can't be statically validated) - ILLink already removed support for it anyway. However, this has a somewhat significant size increase for our apps, and it would affect pretty much every app, so re-implement the aggressive attribute trimming ourselves. This is accomplished by copying ILLink's list of attributes to remove into our own xml definition, which we then pass to ILLink. I tested a release build of monotouch-test for all platforms, and the final size is almost entirely unchanged, except for a minimal size decrease. Here's the results for iOS: https://gist.github.com/rolfbjarne/dac3e08620a3ae499ad1fc134765e0d2 Fixes #20906.

…-attribute-trimming

vs-mobiletools-engineering-service2 · 2024-10-12T10:54:25Z

✅ API diff for current PR / commit

.NET (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

✅ API diff vs stable

.NET (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 7abc46143b71b2947bbf0c346555f771ef91fedf [PR build]

vs-mobiletools-engineering-service2 · 2024-10-12T10:55:10Z

💻 [CI Build] Windows Integration Tests passed 💻

✅ All Windows Integration Tests passed.

Pipeline on Agent
Hash: 7abc46143b71b2947bbf0c346555f771ef91fedf [PR build]

vs-mobiletools-engineering-service2 · 2024-10-12T10:59:25Z

💻 [CI Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

✅ All tests on macOS X64 - Mac Sonoma (14) passed.

Pipeline on Agent
Hash: 7abc46143b71b2947bbf0c346555f771ef91fedf [PR build]

vs-mobiletools-engineering-service2 · 2024-10-12T11:00:45Z

💻 [CI Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

✅ All tests on macOS M1 - Mac Monterey (12) passed.

Pipeline on Agent
Hash: 7abc46143b71b2947bbf0c346555f771ef91fedf [PR build]

vs-mobiletools-engineering-service2 · 2024-10-12T11:01:48Z

💻 [CI Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

✅ All tests on macOS M1 - Mac Ventura (13) passed.

Pipeline on Agent
Hash: 7abc46143b71b2947bbf0c346555f771ef91fedf [PR build]

vs-mobiletools-engineering-service2 · 2024-10-14T09:34:51Z

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 101 tests passed 🎉

Tests counts

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ generator: All 1 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 4 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ linker: All 40 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 7 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 10 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 9 tests passed. [attempt 3] Html Report (VSDrops) Download
✅ monotouch (tvOS): All 7 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. [attempt 2] Html Report (VSDrops) Download

Pipeline on Agent
Hash: 7abc46143b71b2947bbf0c346555f771ef91fedf [PR build]

This comment was marked as outdated.

Sign in to view

This comment has been minimized.

Sign in to view

dalexsoto reviewed Sep 27, 2024

View reviewed changes

This comment was marked as outdated.

Sign in to view

This comment has been minimized.

Sign in to view

This comment was marked as outdated.

Sign in to view

rolfbjarne added the do-not-merge Do not merge this pull request label Oct 1, 2024

rolfbjarne changed the base branch from net9.0 to main October 4, 2024 08:09

This comment has been minimized.

Sign in to view

rolfbjarne marked this pull request as ready for review October 10, 2024 09:18

rolfbjarne changed the base branch from main to net10.0 October 10, 2024 09:19

rolfbjarne added 2 commits October 10, 2024 11:20

Improve docs.

ec9c808

rolfbjarne force-pushed the dev/rolf/issue-20906-disable-undocumented-attribute-trimming branch from 9f56935 to ec9c808 Compare October 10, 2024 09:20

rolfbjarne changed the title ~~[dotnet] Rework aggressive attribute trimming. Fixes #20906.~~ [net10.0] [dotnet] Rework aggressive attribute trimming. Fixes #20906. Oct 10, 2024

This comment has been minimized.

Sign in to view

Merge branch 'net10.0' into dev/rolf/issue-20906-disable-undocumented…

7abc461

…-attribute-trimming

This comment has been minimized.

Sign in to view

dalexsoto approved these changes Oct 15, 2024

View reviewed changes

rolfbjarne merged commit b7a9096 into net10.0 Oct 15, 2024
21 checks passed

rolfbjarne deleted the dev/rolf/issue-20906-disable-undocumented-attribute-trimming branch October 15, 2024 13:59

[net10.0] [dotnet] Rework aggressive attribute trimming. Fixes #20906. #21314

[net10.0] [dotnet] Rework aggressive attribute trimming. Fixes #20906. #21314

Conversation

rolfbjarne commented Sep 26, 2024 • edited Loading

This comment was marked as outdated.

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.

This comment has been minimized.

dalexsoto Sep 27, 2024

Choose a reason for hiding this comment

rolfbjarne Oct 10, 2024

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment has been minimized.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

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.

vs-mobiletools-engineering-service2 commented Oct 12, 2024

✅ API diff for current PR / commit

✅ API diff vs stable

ℹ️ Generator diff

vs-mobiletools-engineering-service2 commented Oct 12, 2024

💻 [CI Build] Windows Integration Tests passed 💻

vs-mobiletools-engineering-service2 commented Oct 12, 2024

💻 [CI Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

vs-mobiletools-engineering-service2 commented Oct 12, 2024

💻 [CI Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

vs-mobiletools-engineering-service2 commented Oct 12, 2024

💻 [CI Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Oct 14, 2024

🚀 [CI Build] Test results 🚀

Test results

Tests counts

rolfbjarne commented Sep 26, 2024 •

edited

Loading