Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

DesignerAttribute/EditorAttribute size implications #92043

Closed
Tracked by #4649
MichalStrehovsky opened this issue Sep 14, 2023 · 6 comments
Closed
Tracked by #4649

DesignerAttribute/EditorAttribute size implications #92043

MichalStrehovsky opened this issue Sep 14, 2023 · 6 comments
Labels
area-System.ComponentModel linkable-framework Issues associated with delivering a linker friendly framework
Milestone

Comments

@MichalStrehovsky
Copy link
Member

MichalStrehovsky commented Sep 14, 2023

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 --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 @@
+<linker>
+  <assembly fullname="System.ComponentModel.Primitives" feature="System.ComponentModel.Designer.IsSupported" featurevalue="false">
+    <type fullname="System.ComponentModel.DesignerAttribute">
+      <attribute internal="RemoveAttributeInstances" />
+    </type>
+    <type fullname="System.ComponentModel.EditorAttribute">
+      <attribute internal="RemoveAttributeInstances" />
+    </type>
+  </assembly>
+</linker>
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 @@
     -->
     <GenerateResxSourceIncludeDefaultValues>true</GenerateResxSourceIncludeDefaultValues>
   </PropertyGroup>
+  <ItemGroup>
+    <ILLinkLinkAttributesXmls Include="ILLink\ILLink.LinkAttributes.xml" />
+  </ItemGroup>
   <ItemGroup>
     <Compile Include="System\ComponentModel\ISynchronizeInvoke.cs" />
     <Compile Include="System\ComponentModel\BrowsableAttribute.cs" />
@MichalStrehovsky MichalStrehovsky added area-System.ComponentModel linkable-framework Issues associated with delivering a linker friendly framework labels Sep 14, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 14, 2023
@ghost
Copy link

ghost commented Sep 14, 2023

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
Copy link
Contributor

Wraith2 commented Sep 14, 2023

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
Copy link
Member

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: dotnet/corefx@09d2d1b

@steveharter steveharter removed the untriaged New issue has not been triaged by the area owner label Sep 18, 2023
@steveharter steveharter added this to the Future milestone Sep 18, 2023
@MichalStrehovsky
Copy link
Member Author

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.:

public DesignerAttribute([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] string designerTypeName)

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

@MichalStrehovsky
Copy link
Member Author

Cc @eerhardt

@MichalStrehovsky
Copy link
Member Author

Fixed in #98378.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 24, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area-System.ComponentModel linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

No branches or pull requests

3 participants