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

Performance degradation when navigating to a page and going back several times #12930

Closed
gkamenov opened this issue Jan 26, 2023 · 8 comments · Fixed by #13833
Closed

Performance degradation when navigating to a page and going back several times #12930

gkamenov opened this issue Jan 26, 2023 · 8 comments · Fixed by #13833
Assignees
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout fixed-in-8.0.0-preview.3.8149 Look for this fix in 8.0.0-preview.3.8149! p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with platform/android 🤖 platform/iOS 🍎 platform/windows 🪟 t/bug Something isn't working t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf)

Comments

@gkamenov
Copy link

Description

Each time a page navigates to another created by a dependency service as a result of a call to Shell.Current.GoToAsync a new instance of that page is created and the time to create and/or navigate to that page is incrementally extended.

The attached sample project demonstrates this behavior.

The project is standard MAUI template app with a MainPage and an additional NewPage with some controls on it to speed up the effect of delaying. The idea is that the MainPage navigates to the NewPage and then the NewPage returns back to MainPage in a loop and the loading time of the NewPage is measured and displayed. This loading time get gradually increased.

Timers have been added to MainPage and NewPage. When the timer event occurs in the MainPage the current time is taken and navigation to the NewPage is initiated. When NewPage finishes loading, it displays how long it took to load. The timer in NewPage then triggers a "back" action by executing Shell.Current.GoToAsync(".."). The MainPage timer starts again and the loop continues infinitely.

You can notice that the load time (in milliseconds) presented in NewPage increases with each iteration. After 20-30 iterations, the loading time becomes a few seconds and continues to grow.

This behavior is observed on all three tested platforms – iOS, Android and Windows.

No latency increase is observed when NewPage is added to the dependency service as a AddSingleton<NewPage>() and progressively increasing latency is experienced when added as a AddTransient<NewPage>().

Steps to Reproduce

Please run the attached project and click the button "Start the loop" then observe the number in the page that shows up.

Link to public reproduction project repository

https://github.com/gkamenov/MauiBug

Version with bug

7.0 (current)

Last version that worked well

7.0 (current)

Affected platforms

iOS, Android, Windows

Affected platform versions

iOS 14.2 and up, Android 8 and up, Windows build 22621.1105

Did you find any workaround?

No workaround so far.

Relevant log output

No response

@gkamenov gkamenov added the t/bug Something isn't working label Jan 26, 2023
@jsuarezruiz jsuarezruiz added area-controls-shell Shell Navigation, Routes, Tabs, Flyout legacy-area-perf Startup / Runtime performance labels Jan 26, 2023
@jsuarezruiz jsuarezruiz added this to the Backlog milestone Jan 26, 2023
@ghost
Copy link

ghost commented Jan 26, 2023

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@Vroomer
Copy link

Vroomer commented Jan 26, 2023

I think this is might be related to #10578 and #12930

There is definitely tons of UI bits getting stuck in the memory, even though the overall memory size rises only slightly. I'm not sure if this can lead to such significant performance degradation, but I tried the same thing by pushing the page the old school way:

await Navigation.PushAsync(new NewPage());
await Navigation.PopAsync();

The problem is exactly the same - performance degrades and memory increases, so I wouldn't suspect some Shell routing problem.

@matteo-convert
Copy link

matteo-convert commented Jan 27, 2023

I think this is might be related to #10578 and #12930

There is definitely tons of UI bits getting stuck in the memory, even though the overall memory size rises only slightly. I'm not sure if this can lead to such significant performance degradation, but I tried the same thing by pushing the page the old school way:

await Navigation.PushAsync(new NewPage());
await Navigation.PopAsync();

The problem is exactly the same - performance degrades and memory increases, so I wouldn't suspect some Shell routing problem.

if i can help you, the shell is not the problem MAUI is the problem, i've track all my page destruction / ViewModel / Object, create my own shell, use syncfusion components and application still bug after navigate between pages, all the time the same problem, more and more slow for no reason because the memory usage don't increase (you can see if you attach your debugging on android Studio), and the only thing they do is send problems that prevent production in Backlog milestone

@gkamenov
Copy link
Author

I left the linked app running on Windows for approximately 20 hours, and now it is loading the NewPage for 16 seconds! Allocated memory for the app is 160MB and does not increase.

Although I am not sure, I tend to believe the issue is related with the visual objects created within the page. I think they subscribe to some events and when page is closed those objects event handlers are still triggered by those events. Or this could be the code that is processing the bindings.

Now I run a test with blank content page only with the label presenting the result and for now it looks there is no increasing delay. I will leave this to run for 10-20 hours to check if the content is the problem.

@Vroomer
Copy link

Vroomer commented Jan 27, 2023

You can also try to delete all the style resources. One of the underlying memory leak is because of strong reference to brushes. There is currently PR merged for the next release if I'm not mistaken.

@gkamenov
Copy link
Author

The test with with blank content page failed. It is getting slow again but just need more time.

So far the only workaround I found for my self is to make all pages and view models singletons in DI and to use Loaded and Unloaded events on the page to force update the corresponding view model and page. This way the constructors are called only once and view model is preparing it's properties during OnLoaded, and cleaning if necessary in OnUnloaded. This is ugly workaround but I have to go to production asap.

I could accept this if MAUI is still in preview, but for release version I really hope this issue to be fixed soon because it makes MAUI unusable for applications that do something different than showing "Hello world!"

@davidortinau davidortinau added the partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with label Mar 2, 2023
@samhouts samhouts added the p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint label Mar 3, 2023
@jonathanpeppers jonathanpeppers self-assigned this Mar 30, 2023
@jonathanpeppers
Copy link
Member

I tested this sample with current dotnet/maui/main.

On Windows it seems to show between 650 to 750ms:

image

With the memory usage on Windows:

image

On Android, I can grab the memory usage by doing:

> adb shell dumpsys meminfo | grep com.companyname.mauibug
    325,016K: com.companyname.mauibug (pid 27182 / activities)
> adb shell dumpsys meminfo | grep com.companyname.mauibug
    329,408K: com.companyname.mauibug (pid 27182 / activities)
> adb shell dumpsys meminfo | grep com.companyname.mauibug
    324,848K: com.companyname.mauibug (pid 27182 / activities)
> adb shell dumpsys meminfo | grep com.companyname.mauibug
    324,856K: com.companyname.mauibug (pid 27182 / activities)

I don't see anything going wrong here anymore, because I believe the underlying issue is fixed in: #13833

There are also this many fixes related to memory leaks:

#13260
#13327
#13333
#13400
#13530
#13550
#13656
#13806
#13833
#14108

Navigating back from a Page was leaking, which would explain the behavior you would have seen before.

Let me know if you think I missed anything here, thanks! You can probably try this out again in a future .NET MAUI release.

@dpjha84
Copy link

dpjha84 commented Apr 4, 2023

When will this fix be available to use?

@ghost ghost locked as resolved and limited conversation to collaborators May 4, 2023
@samhouts samhouts added the fixed-in-8.0.0-preview.3.8149 Look for this fix in 8.0.0-preview.3.8149! label May 10, 2023
@samhouts samhouts modified the milestones: Backlog, .NET 8 May 24, 2023
@Eilon Eilon added t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf) and removed legacy-area-perf Startup / Runtime performance labels May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout fixed-in-8.0.0-preview.3.8149 Look for this fix in 8.0.0-preview.3.8149! p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with platform/android 🤖 platform/iOS 🍎 platform/windows 🪟 t/bug Something isn't working t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants