dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.36k stars 4.74k forks source link

DesignerAttribute/EditorAttribute size implications #92043

Closed MichalStrehovsky closed 8 months ago

MichalStrehovsky commented 1 year ago

We annotated DesignerAttribute/EditorAttribute such that they preserve things on the type that is referred from the attribute.

These two attributes have huge size impact on WinForms - I'm seeing 25+%.

We should ideally do something about this.

1) Should they actually be annotated? Would we ever expect a trimmed app to use these? (Removing the annotation would be a breaking change, but the break might be warranted.) 2) If they should be annotated, do we want a feature switch to drop the attributes? I experimentally tried the thing in details below and saw 25+% savings. 3) Should the default value of the feature switch be to remove them?

Diff (click to expand) ```diff diff --git a/src/libraries/System.ComponentModel.Primitives/src/ILLink/ILLink.LinkAttributes.xml b/src/libraries/System.ComponentModel.Primitives/src/ILLink/ILLink.LinkAttributes.xml new file mode 100644 index 00000000000..69c54060589 --- /dev/null +++ b/src/libraries/System.ComponentModel.Primitives/src/ILLink/ILLink.LinkAttributes.xml @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/src/libraries/System.ComponentModel.Primitives/src/System.ComponentModel.Primitives.csproj b/src/libraries/System.ComponentModel.Primitives/src/System.ComponentModel.Primitives.csproj index fe840c389fd..16e8cc1ef0f 100644 --- a/src/libraries/System.ComponentModel.Primitives/src/System.ComponentModel.Primitives.csproj +++ b/src/libraries/System.ComponentModel.Primitives/src/System.ComponentModel.Primitives.csproj @@ -7,6 +7,9 @@ --> true + + + ```
ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-system-componentmodel See info in area-owners.md if you want to be subscribed.

Issue Details
We annotated `DesignerAttribute`/`EditorAttribute` such that they preserve things on the type that is referred from the attribute. These two attributes have huge size impact on WinForms - I'm seeing 25+%. We should ideally do something about this. 1) Should they actually be annotated? Would we ever expect a trimmed app to use these? (Removing the annotation would be a breaking change, but the break might be warranted.) 2) If they should be annotated, do we want a feature switch to drop the attributes? I experimentally tried the thing in details below and saw 25+% savings. 3) Should the default value of the feature switch be to remove them?
Diff (click to expand) ```diff diff --git a/src/libraries/System.ComponentModel.Primitives/src/ILLink/ILLink.LinkAttributes.xml b/src/libraries/System.ComponentModel.Primitives/src/ILLink/ILLink.LinkAttributes.xml new file mode 100644 index 00000000000..69c54060589 --- /dev/null +++ b/src/libraries/System.ComponentModel.Primitives/src/ILLink/ILLink.LinkAttributes.xml @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/src/libraries/System.ComponentModel.Primitives/src/System.ComponentModel.Primitives.csproj b/src/libraries/System.ComponentModel.Primitives/src/System.ComponentModel.Primitives.csproj index fe840c389fd..16e8cc1ef0f 100644 --- a/src/libraries/System.ComponentModel.Primitives/src/System.ComponentModel.Primitives.csproj +++ b/src/libraries/System.ComponentModel.Primitives/src/System.ComponentModel.Primitives.csproj @@ -7,6 +7,9 @@ --> true + + + ```
Author: MichalStrehovsky
Assignees: -
Labels: `area-System.ComponentModel`, `linkable-framework`
Milestone: -
Wraith2 commented 1 year ago

If you use a PropertyGrid then having editor attributes is required in order to customize the UI of types in the grid. So I think they're often required.

steveharter commented 1 year ago

These two attributes have huge size impact on WinForms - I'm seeing 25+%.

What is the total size savings, and is it just in System.ComponentModel.Primitives.dll (80K, so 20K savings)?

Both of these I assume are not needed at runtime (only design-time). Do we have a mechanism during publish to distinguish between that and remove such artifacts? If not, could\should we add one?

Some history: Newer (2020) PR for DesignerAttribute: https://github.com/dotnet/runtime/pull/40425/files Older (2016) commit for EditorAttribute: https://github.com/dotnet/corefx/commit/09d2d1bf1259ff1ae775b36001f16b4f0814cff7

MichalStrehovsky commented 1 year ago

What is the total size savings, and is it just in System.ComponentModel.Primitives.dll (80K, so 20K savings)?

The savings is 6 MB for an empty WinForms app. (21 MB to 15 MB). This is not the size of ComponentModel.Primitives: that's tiny.

To expand on the "We annotated DesignerAttribute/EditorAttribute such that they preserve things on the type that is referred from the attribute." in the top post: the problem is in the DynamicallyAccessedMembers annotations on these, e.g.:

https://github.com/dotnet/runtime/blob/e1ca02f9eccac5531dafa5d506b0846d9bd8ad6b/src/libraries/System.ComponentModel.Primitives/src/System/ComponentModel/DesignerAttribute.cs#L20

These tell trimming to keep the referenced type, and that's where all the costs come from.

MichalStrehovsky commented 1 year ago

Cc @eerhardt

MichalStrehovsky commented 8 months ago

Fixed in #98378.