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

TabView: Memory usage does not go down after opening/closing tabs #3597

Closed
lukeblevins opened this issue Nov 11, 2020 · 27 comments
Closed

TabView: Memory usage does not go down after opening/closing tabs #3597

lukeblevins opened this issue Nov 11, 2020 · 27 comments
Labels
area-TabView help wanted Issue ideal for external contributors needs-triage Issue needs to be triaged by the area owners team-Controls Issue for the Controls team
Milestone

Comments

@lukeblevins
Copy link
Contributor

lukeblevins commented Nov 11, 2020

It appears that there are invisible items in the WinUI TabView visual tree which stick around even after they are removed. This behavior isn't desirable because users could have many TabViewItems with memory-intensive pages as their content.

Steps to reproduce the bug

Steps to reproduce the behavior:

  1. Create a project where TabViewItems have a ContentTemplate property of a Frame with somewhat memory-intensive content.
  2. Remove all of the tabs (hopefully destroying their content) by clearing the ObservableCollection.
  3. Inspect memory usage to see persistently high levels

Expected behavior
TabView must support scenarios with robust XAML content inside the TabViewItems by freeing the memory when tabs are closed via removal from ObservableCollection.

Screenshots
image

Version Info

NuGet package version:
Microsoft.UI.Xaml 2.6.1

Windows version Saw the problem?
Insider Build (22000.100) Yes
May 2020 Update (19041) Yes
November 2019 Update (18363) Yes
May 2019 Update (18362) Yes
October 2018 Update (17763)
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)
Device form factor Saw the problem?
Desktop Yes
Xbox
Surface Hub
IoT

Additional context
Old comment: #1332 (comment)

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Nov 11, 2020
@ranjeshj
Copy link
Contributor

@duke7553 can you see if this is resolved in the latest pre-release ? Do you happen to know if this regressed or something that is in older releases as well ?

@lukeblevins
Copy link
Contributor Author

lukeblevins commented Nov 13, 2020

@ranjeshj Thanks for getting back to my issue. This has been an issue with use of TabView in our project for a long time. Probably since we switched to the WinUI TabView. Since this could very well be an issue with my app specifically, I am working on a sample project to demonstrate if the behavior is reproducible or not.

I expect to add it to this issue report soon.

@ranjeshj
Copy link
Contributor

Thanks @duke7553 for the clarification. A repro app would be great.

@StephenLPeters StephenLPeters added area-TabView needs-author-feedback Asked author to supply more information. team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Nov 14, 2020
@lukeblevins
Copy link
Contributor Author

lukeblevins commented Nov 14, 2020

@ranjeshj This is a WinUI v2.6 sample app which reproduces the bug in a minimal way.

Download source code:
https://1drv.ms/u/s!AvgU7aa_XojBgYw8fAvMofoXYS6bfQ?e=LNGhL1

When running this sample, you should expect to see memory usage creep up quite a bit. Closing any tab will clear the collection and now you should see an absent/negligible decline in memory usage of the app.

@ghost ghost added needs-triage Issue needs to be triaged by the area owners and removed needs-author-feedback Asked author to supply more information. labels Nov 14, 2020
@StephenLPeters StephenLPeters added help wanted Issue ideal for external contributors and removed needs-triage Issue needs to be triaged by the area owners labels Nov 17, 2020
@lukeblevins
Copy link
Contributor Author

@ranjeshj @StephenLPeters I've updated the repro project for this bug to WinUI v2.6. The issue is definitely still present and it makes using the TabView for any kind of robust XAML content a nonstarter.

Please give it a look when you get a chance. 😔🙏🏻

@lukeblevins
Copy link
Contributor Author

lukeblevins commented Jul 30, 2021

@ranjeshj @StephenLPeters I did some more detective work on this: It looks like this bug still exists even when completely replacing TabView with a regular ListView in my repro app.

Memory usage stays elevated despite clearing the ListView ItemSource. This includes setting it to null, removing each item one-by-one, and doing .Clear() on it.

Should this behavior be expected of Page and XAML controls in general with C#? How can I free an item's memory usage after it's closed? Am I missing something, or is this just an awful bug reported by many, other users over the years.

@StephenLPeters
Copy link
Contributor

@duke7553 have you ensured that GC has run by calling GC.Collect() during your testing?

@lukeblevins
Copy link
Contributor Author

@StephenLPeters Yes. The repro app does this after it clears the tabs

@lukeblevins
Copy link
Contributor Author

Most of the memory growth is from unmanaged memory btw

@lukeblevins
Copy link
Contributor Author

@StephenLPeters Is the team aware that this is an issue with TabView and ListView or is there something I'm doing incorrectly regarding coding practices with these controls?

@StephenLPeters
Copy link
Contributor

It is on our radar, thank you for raising the issues.

@ghost ghost removed the needs-triage Issue needs to be triaged by the area owners label Oct 13, 2021
@lukeblevins
Copy link
Contributor Author

lukeblevins commented Oct 13, 2021

It was a recent fix so it will be included in a future release. Although, my understanding is the current plan is to service this fix, so some older versions of window 10/11 will receive the fix in a service update at some point.

@StephenLPeters Wow! Thanks for circling back and the transparency here. By any chance, would this fix already be present in WinUI Desktop? Or maybe UWP XAML is the only stack affected.

This made my day!

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Oct 13, 2021
@StephenLPeters
Copy link
Contributor

@RBrid do you know if the data template leak fix has made it into Winui3?

@ghost ghost removed the needs-triage Issue needs to be triaged by the area owners label Oct 13, 2021
@RBrid
Copy link
Contributor

RBrid commented Oct 13, 2021

@StephenLPeters, no I have not ported it to WinUI 3. No ETA for that unfortunately.

@lukeblevins
Copy link
Contributor Author

@StephenLPeters @RBrid Has the OS fix shipped in Windows or the decoupled UI stack yet?

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Apr 14, 2022
@ojhad
Copy link
Contributor

ojhad commented Apr 14, 2022

We have made the fix but it has not shipped yet. We are working on porting it to WinUI3.

@ghost ghost removed the needs-triage Issue needs to be triaged by the area owners label Apr 14, 2022
@lukeblevins
Copy link
Contributor Author

@ojhad @RBrid @StephenLPeters Has this fix shipped yet?

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Aug 12, 2022
@ranjeshj ranjeshj removed the needs-triage Issue needs to be triaged by the area owners label Aug 26, 2022
@lukeblevins
Copy link
Contributor Author

@ojhad @RBrid @StephenLPeters Should this be reopened?

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Jan 19, 2023
@lukeblevins
Copy link
Contributor Author

What a joke.

Hopefully this repository situation improves in the coming years, or else it's abundantly clear that WinUI isn't sustainable nor is it a priority to Microsoft.

@yaira2
Copy link
Contributor

yaira2 commented Jan 23, 2023

This issue appears to be affecting File Explorer and Notepad as well.

@bpulliam bpulliam removed the needs-triage Issue needs to be triaged by the area owners label Mar 13, 2023
Pinguin2001 added a commit to bluebird-developers/browser that referenced this issue Oct 31, 2023
@0x5bfa
Copy link

0x5bfa commented Jun 6, 2024

@ojhad Has that fix been ported into WinUI3?

@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Jun 6, 2024
@neildmd
Copy link

neildmd commented Jun 9, 2024

@ojhad Has that fix been ported into WinUI3?

You would think that it would have been ported by now... This memory leak has been present for an embarrassingly long time. It's not like this is insignificant. Why are they dragging their feet on this?

@0x5bfa
Copy link

0x5bfa commented Jun 9, 2024

lol. Maybe they have been busy for a long time.
Do you think we should reopen? Looks like they have their mirrored issue tracker internally.

@codendone
Copy link
Contributor

I checked with the team, and the fix has been in WinUI3 since WinAppSDK 1.3 (ported via internal bug).

It sounds like there are additional leaks beyond that fix. Please open a new issue with a repro for us to investigate those leaks. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-TabView help wanted Issue ideal for external contributors needs-triage Issue needs to be triaged by the area owners team-Controls Issue for the Controls team
Projects
None yet
Development

No branches or pull requests

10 participants