Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

🚨 7.1 Package Size Regressions #4218

Closed
3 of 5 tasks
michael-hawker opened this issue Sep 1, 2021 · 3 comments
Closed
3 of 5 tasks

🚨 7.1 Package Size Regressions #4218

michael-hawker opened this issue Sep 1, 2021 · 3 comments
Assignees
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior build 🔥 in progress 🚧 optimization ☄ Performance or memory usage improvements priority 🚩
Milestone

Comments

@michael-hawker
Copy link
Member

michael-hawker commented Sep 1, 2021

Describe the bug

Analyzing our package sizes between 7.1-rc1 and 7.0 stable has revealed some increases to our base package sizes, some minimal amounts can be expected from new features, but these seem more significant.

  • Is this bug a regression in the toolkit? If so, what toolkit version did you last see it work:

Steps to Reproduce

Mar 16 - This is a report generated from a newly built copy of the rel/7.0.0 branch:
7-0-0.txt

Jul 23 - WinUI 2.6 Update (compare against 7.0, overall this has minimal impact on toolkit compare at this point (and includes MetadataControl; though does show where we have bug in smoke test where WinUI is over included to projects which don't use it (CI issue only)):
WinUI26.txt

Aug 3 - Constrained Box (compare against WinUI, seems to be minimal impact):
ConstrainedBox.txt

Aug 16 - This is 7.1-preview1 from the CI we still have a copy of:
7-1-0-preview1.txt

Aug 26 - This is the RichSuggestBox PR (compare against 7.1-preview1):
RichSuggestBox.txt

Aug 27 - This is the Shadow API PR (compare against RichSuggestBox):
ShadowAPI.txt

Aug 30 - TwoPaneView ListDetailsView PR (compare against ShadowAPI, seems minimal impact):
TwoPaneView.txt

Aug 30 - And this is 7.1-RC1:
7-1-0-rc1.txt

It appears updating to WinUI 2.6 itself has caused a max increase of 50k and a nominal/min increase of 143k on its own, so that accounts for some of the discrepancy.

However the Primitives package as a whole has increased 500k and Input 759k!

Expected behavior

Would have expected 100-200kb increase overall for the number of controls we added.

Additional context

I did notice we're not including WinUI 2.6 right in the smoke test project so I do some differences in how the comparison is, not sure if that's being accounted for twice or something, so going to fix that and see the difference. Though that would still be only half.

It does seem comparing 7.1-preview1 to the RichSuggestBox implies it's a 263kb add which is on the heavier side. That's a bit surprising unless pulling in the whole RichEditBox is part of that somehow, we'd have to add that to the UWPWinUIBaseline and compare...

The Shadow API seems rather minimal overall, so that one seems safe?

Experiments to Run

  • Record changes over time in list above to narrow in on change period
  • Adjust WinUI config in SmokeTest.csproj to not be included in non-WinUI based projects (this just effects raw reported number but not actual impact)
  • Try adding RichEditBox to the UWPWinUIBaseline and compare...
  • ???

Not sure if anyone else wants to compare above and check my numbers. Beyond Compare is working pretty good for comparison.

Probably means we should figure out how to best implement #4113 for 7.2/8.0...

@michael-hawker michael-hawker added bug 🐛 An unexpected issue that highlights incorrect behavior build 🔥 optimization ☄ Performance or memory usage improvements priority 🚩 labels Sep 1, 2021
@michael-hawker michael-hawker added this to the 7.1 milestone Sep 1, 2021
@michael-hawker michael-hawker self-assigned this Sep 1, 2021
@michael-hawker
Copy link
Member Author

Crunched some more numbers today. In the pipeline half the increase is due to how WinUI dependency seemed to be totaled. I'm not sure what has been happening there, but at least that can be discarded.

In some more practical tests, it seems like the optimizations at the AppX .NET Native level remain, so even if the Primitives package has increased, the overall impact is actually nil currently.

Updating from a prior version of WinUI to 2.6.2 itself causes a general increase of 294kb, so that's out of our hands - though the underlying size of WinUI itself is abstracted as a framework package generally. Using a RichEditBox itself seems to add a fairly negligible 12-16kb.

Seems like the Input package is about 225kb greater in practical usage, which is a bit more reasonable than 750 from the CI; though still on the larger size as we seemed to know we have an average of 100kb per control.

Going to check other packages including the Media ones more tomorrow.

@michael-hawker
Copy link
Member Author

Alright, did some more poking. An interesting fact it seems is like the Microsoft.UI.Xaml.winmd file wasn't being included in the packaged output with WinUI 2.5/WCT 7.0, but it is with WinUI 2.6/WCT 7.1. So this seems like an inherent change to how WinUI was configured and something we can factor out of our numbers.

With this in mind, even our Primitives package and most others the size changes in practical usage are nominal.

The main impact is still the RichSuggestBox increasing the impact of the Input package by those 200kb. I'll see if I can dig into that specific PR a bit more to understand what is causing the extra overhead... but it may not be worth it for now.

@ghost ghost added the in progress 🚧 label Sep 9, 2021
@michael-hawker
Copy link
Member Author

Discussing with the Toolkit team, we think this is acceptable for now. We'll monitor in the future, but at least have pin pointed this to something in the RichSuggestBox PR to investigate later. It is a bit odd as I don't think there's any super special APIs we'd be calling there when RichEditBox itself didn't have a bit impact on footprint.

Also, we need to continue to investigate how this works for WinUI 3 next milestone and if optimization is better for unused controls on .NET 5.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior build 🔥 in progress 🚧 optimization ☄ Performance or memory usage improvements priority 🚩
Projects
None yet
Development

No branches or pull requests

1 participant