Reduce the toolkit impact on application size #3145
Comments
vgromfeld
added
the
feature request 📬
|
Thanks for submitting a new feature request! I've automatically added a vote 👍 reaction to help get things started. Other community members can vote to help us prioritize this feature in the future! |
Kyaa-dost
added
the
help wanted
|
Removing the Json.NET dependency should help in the future, that's tracked by this item: #3060, not sure if that'll line up for the 7.0 release though as there's a blocking bug in the There may be an opportunity to investigate where we're using this dependency and clean that up or isolate it though. |
Created a new Microsoft.Toolkit.Uwp.UI.Controls.Markdown package, moving the MarkdownTextBlock from the Controls package and decoupling the TextToolbar from the MarkDownFormatter. ## Helps with #3145 ## PR Type What kind of change does this PR introduce? <!-- Please uncomment one or more that apply to this PR. --> <!-- - Bugfix --> <!-- - Feature --> <!-- - Code style update (formatting) --> - Refactoring <!-- - Build or CI related changes --> <!-- - Documentation content changes --> <!-- - Sample app changes --> <!-- - Other... Please describe: --> ## What is the current behavior? Having a dependency on Microsoft.Toolkit.Uwp.UI.Controls pulls too many packages and dependencies. ## What is the new behavior? Microsoft.Toolkit.Uwp.UI.Controls no longer has a dependency on ColorCode.UWP, nor on the Microsoft.Toolkit.Parsers package. ## PR Checklist Please check if your PR fulfills the following requirements: - [X] Tested code with current [supported SDKs](../readme.md#supported) - [ ] Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR link --> - [X] Sample in sample app has been added / updated (for bug fixes / features) - [ ] Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets) - [ ] Tests for the changes have been added (for bug fixes / features) (if applicable) - [X] Header has been added to all new source files (run *build/UpdateHeaders.bat*) - [ ] Contains **NO** breaking changes If you are using MarkdownTextBlock, or the TextToolbar with `Format = Format.Markdown`, you will need to add a dependency to the new Microsoft.Toolkit.Uwp.UI.Controls.Markdown. The MarkdownTextBlock implementation is unchanged, but the TextToolbar no longer has a Format property. The markdown parser is now provided just as any other custom parser, and need to be added in Xaml, or code behind. For example: ```xaml <Page ... xmlns:markDown="using:Microsoft.Toolkit.Uwp.UI.Controls.TextToolbarFormats.MarkDown" xmlns:richText="using:Microsoft.Toolkit.Uwp.UI.Controls.TextToolbarFormats.RichText" ... > ... <controls:TextToolbar ...> <controls:TextToolbar.Formatter> <richText:RichTextFormatter /> <!--<markDown:MarkDownFormatter />--> <!--<local:CustomFormatter />--> </controls:TextToolbar.Formatter> </controls:TextToolbar> ... ```
|
@vgromfeld can you add some more details here about your recent investigations, especially in regards to the XAML Metadata stuff you looked at recently? |
|
Yes, looking at the symbols available in the final binary, I've found that even without any runtime directive, the binary still contains a lot of symbols related to the community toolkit. My initial guess was that since they are not used, they should have been automatically removed. But, it is not the case. Looking at the app The XAML compiler has a I first try with a blank app containing only a
I also tried with a more "complicated" control:
I suspect all the static dependency properties to be the remaining roots to the controls symbols. Because they are static fields, they are not removed by .Net Native. I t guess that this is why changing the XAML compiler behavior is not helping that much. We can try to replace the dependency properties fields by properties as we've done for some classes but I'm not sure if the XAML runtime will like that... |
|
FYI @vgromfeld @JustinXinLiu @rudyhuyn Obviously if you use multiple packages, the additional overhead starts to minimize with shared dependencies between some of the higher-up packages of Controls. If you just needed one individual package, this gives us a good idea of the overhead that brings. Here's the whole latest breakdown from the final bit of work we're doing in #3676: |
|
I've rebuilt my sample app with the 7.0.0-preview 4 nuget package and the app's dll size has decreased from 6,171Kb to 1,800Kb (x86) 🎉 |
|
@vgromfeld preview4 was even before some of these refactors, so that's good to hear! There's the latest instructions about grabbing PR packages here. Build 7.0.0-build.749.ge410992e4e has the new 'Core' package being worked on in #3676, so I'd be curious what different that makes, and if you feel that is acceptable or if we should split 'Core' into 2 packages of roughly half the size each? |
|
Hum, when I tried using the |
Well, that's confusing... Preview4 added both the Behaviors dependency and There shouldn't be anything different in the build process between the PR and preview branches, it's the same pipeline... |
|
Looking at the content of both binaries, using the 7.0.0-build.749.ge410992e4e package pulls a lot of Xml related types which were not part of preview 4. I can see |
|
@vgromfeld hmmm, I wonder if that's because of #3637? I didn't think that using the existing platform stuff should matter to overhead? Is this because we construct one by default, is there a better pattern we can use here if it's not being used? |
|
@vgromfeld alright, I've been able to dig into this a little bit. I can grab the .NET Native size diff in the Smoke Test script now too. Definitely seems like some of the initial bloat is coming from the I'm trying to get the older 7.0.0-preview4 branch to build locally to grab the numbers to compare, but there were minimal changes to this package done between then and now:
It's mainly the change we did in #3637, which is a bit more confusing. I'll see what the numbers show for 7.0.0-preview4, I may have to configure pulling the raw NuGet packages down as building is troublesome since we updated to .NET 5 after that preview so those changes are all missing. |
|
Took a while as our build system for the Smoke Tests is forcing to use the Nerdbank version, so I had to undo that stuff so I could force a build from the 7.0.0-preview4 nugets, but was able to finally get a proper comparison between the current main and 7.0.0-preview4 (both attached). Definitely can see the difference in the minimum impact. The main question is to understand why, as I tried commenting out the changes entirely to the |
|
I just did a quick investigation on C# based Windows Runtime Components (WRC). Converting <OutputType>winmdobj</OutputType>
<AllowCrossPlatformRetargeting>false</AllowCrossPlatformRetargeting>However, that introduces 440+ errors that need to be resolved, most of which are individually significant and require breaking changes. It's definitely not something we would tackle for 7.0. And we'd want to think about the pros vs. cons or if we pull out smaller chunks at a time which are more easily transferrable (this may make more sense in the UI package for things like Converters and Triggers, etc...) I also don't know how this world changes for .NET 5+ and C#/WinRT @azchohfi? Doing some quick tests on relatively empty components, it does appear that a C# WRC is compiled into a winmd and would live outside the application dll. Therefore, it could potentially be more easily shared on the overall footprint of an app. If that WRC references a regular .NET Standard component, it appear that still lives outside of it and is included in the app dll, though the winmd file will still remain separate. Going to look at seeing if we need the Uwp package as a dependency for the UI package. |
|
Since generics in the UWP package were difficult, I decided to do some more investigations trying to just pull over the Converters and Triggers from the UI package to a Windows Runtime Component. However, the lack of inheritance and abstract classes made that fairly difficult for some. We were able to maybe relatively easily convert over the In the end though, the winmd produced was only like 40kb and there may be some trimming going on as the one in the AppX folder is only 18kb. I'm not sure where that ends up on disk, so I couldn't test dropping the full winmd in its place to see what would happen. Even then, the impact on the app due to system dependencies via .NET Native still seems to be the larger impact here compared to teasing out bits and pieces within a winmd. Especially with the extra restrictions that places on them? i.e. If we had something large like the Controls package as whole which could be a winmd, then maybe it would make more sense. I think we need to understand more of how the new world works in .NET 5+ before making a decision on this front as well. |
ghost
removed
the
help wanted
ghost
locked as resolved and limited conversation to collaborators
Describe the problem this feature would solve
Trying to know what is the impact of the toolkit on the application final size, I've created two blank applications using Visual Studio. On one of them, I've added a dependency to
Microsoft.Toolkit.Uwp.UI. I've only added the nuget dependency. I'm not using anything from the library.Here are the sizes of the applications' dll when compiled in release using .Net Native:
Values are in KB. .Net Native directives' files are empty..
This huge impact comes from the fact that the blank app template is super minimal.
On our application, we are seeing a 2MB increase on both x86 and x64. This is still an important impact (knowing that nothing is used).
Is there a way to reduce the impact of the toolkit on the final binary size ?
Sample projects
UwpToolkitBinaryImpact.zip
The text was updated successfully, but these errors were encountered: