MahApps / MahApps.Metro

A framework that allows developers to cobble together a better UI for their own WPF applications with minimal effort.
https://mahapps.com
MIT License
9.3k stars 2.45k forks source link

MahApps styles cause poor performance because of internal WPF deferred resource reference list #4100

Open oatkins opened 3 years ago

oatkins commented 3 years ago

Describe the bug

We believe we have discovered a bottleneck (bug?) in WPF that causes extremely poor performance under some circumstances. For us, styles coming from MahApps are the primary culprits in triggering the conditions that cause this poor performance.

Steps to reproduce

I'm creating an issue here to see whether this could be mitigated in MahApps itself. It seems as though the "helper" pattern (e.g. CheckBoxHelper is particularly prone to causing this—because you end up with a lot of DPs being set but never read.

My repro project here contains a branch called repro which reproduces the problem without using any external libraries, and a simpler branch called mahapps that demonstrates how MahApps styles tend to trigger it. The project is admittedly contrived, but we experience delays of several seconds in a real application too.

Expected behavior

Switching between views in an application does not slow down as more views are opened.

Actual behavior

The application becomes very slow.

Environment

MahApps.Metro version: v2.4.4 Windows build number: Win10 20H2 [Version 10.0.19042.928] Visual Studio: 2019 16.10.0 Preview 2.1 Target Framework: .Net Framework 4.8


Just to be clear, this is not a gripe: we love MahApps 😁. It seems as though the library is moving away from the "helper" pattern, which creates all these additional DPs, towards more conventional overloading of brushes, etc. using resource replacement. Is that right? From our experiments, that would help a lot with this problem.

oatkins commented 3 years ago

@punker76 I updated my repro with a branch that demos the problem using MahApps instead of "vanilla" WPF controls.

punker76 commented 3 years ago

@oatkins Thx. It's interesting that this tipp here http://blog.lexique-du-net.com/index.php?post/2011/03/27/What-Dynamic-resources-creates-Memory-leaks-in-WPF-3.5-%28SP1%29 still works. After using this, the deferred resource are only 180 after adding the tabs which needs 5 seconds on my machine.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Configuration;
using System.Data;
using System.Linq;
using System.Threading.Tasks;
using System.Windows;

namespace WpfDeferredResourceLookupRepro
{
    /// <summary>
    /// Interaction logic for App.xaml
    /// </summary>
    public partial class App : Application
    {
        override protected void OnStartup(StartupEventArgs e)
        {
            base.OnStartup(e);
            WalkDictionary(this.Resources);
        }

        private static void WalkDictionary(ResourceDictionary resources)
        {
            foreach (DictionaryEntry entry in resources) { }

            foreach (ResourceDictionary rd in resources.MergedDictionaries)
                WalkDictionary(rd);
        }
    }
}
punker76 commented 3 years ago

@oatkins Using the MetroTabControl helps also when switching between the Tabs, because we can keep the controls inside the VisualTree.

<mah:MetroTabControl ItemsSource="{Binding TabViewModels}" KeepVisualTreeInMemoryWhenChangingTabs="True">
...
oatkins commented 3 years ago

@punker76 That blog post is a great link, thank you! I don't think I'd seen that before. I'm investigating adding that workaround in my app. It takes about 1 second to run, which is a shame during startup, but will be worth it if it results in a big improvement.

We're actually using Avalon Dock (this version) rather than a plain tab control. This does something similar to MetroTabControl to prevent tabs from being recreated when you switch between them. We're actually seeing the slowness opening new tabs and closing old ones, rather than when moving between them.

Where a MahApps style has a lot of these "helper" properties, we've seen noticeable improvements by replacing those with dynamic resource references inside the template itself. This has the side-effect of making it harder to customize the control's appearance, but is fine for us. Would you be interested in having that contributed back to the MahApps project? If would be great from my perspective (because I wouldn't need to maintain my own copies of those styles any more), but I appreciate that it would be a breaking change for anyone depending on setting those DPs directly.

timunie commented 3 years ago

Hi @punker76

your blogpost seems to be a good option. Is it worth the effort to add this into the docs as a general tip?

Happy coding Tim

batzen commented 3 years ago

@timunie I don't think we should solve it in MahApps.Metro but in WPF itself, as this not only affects MahApps.Metro but every WPF application using a reasonable amount of dynamic resources. The changes from https://github.com/batzen/wpf/tree/issues/weakreferencelist should fix most of the perf hit caused by the usage of DynamicResource. Let's wait what the WPF team thinks about my proposed solution.

@punker76 FYI