MahApps.Metro icon indicating copy to clipboard operation
MahApps.Metro copied to clipboard

CheckBox performance is very bad

Open Nuklon opened this issue 2 years ago • 18 comments

Describe the bug

Compared to the built-in WPF checkbox style, the rendering performance is really bad. Perhaps due to the massive amount of attached properties?

I saw some PRs that added fallback values, and have tried that but it didn't help with rendering speed. Only thing that works is removing the checkboxes or going back to WPF's own style.

Steps to reproduce

A description of how to trigger this bug.

  1. Add a listbox with am item template with a checkbox and some other stuff, such as some of the MahApps icons 🙂
  2. Add several hundred items.
  3. Virtualization can be enabled or disabled, it doesn't matter.
  4. Scroll, and see how laggy it is.

Expected behavior

It should not lag.

Actual behavior

It lags.

Environment

MahApps.Metro version: v2.4.9
Windows build number: Win10 22H1
Visual Studio: 2022
Target Framework: .NET v4.7.2, also .NET 6.0.

Nuklon avatar Jul 05 '22 13:07 Nuklon

@Nuklon Can you create an app where you can show this problem? Because I tested it at the demo app and the current source and have no problems. Thx!

punker76 avatar Jul 05 '22 20:07 punker76

Yeah, can do, will try to do this weekend.

Nuklon avatar Jul 05 '22 20:07 Nuklon

Haven't managed to create a sample yet, but it seems it's due to dynamic resources usage. If I change dynamic resource to static it's OK. This seems related: https://github.com/MahApps/MahApps.Metro/issues/4100

Nuklon avatar Jul 06 '22 11:07 Nuklon

I've created a sample here: https://github.com/Montago/MahAppsCheckboxSample

When using standard WPF it takes 50ms to render Tab 2 When MahApps is added it takes 470ms !!!

I've supplied 2 Profile sessions where you can see the major difference.

Montago avatar Jul 11 '22 12:07 Montago

@Montago Thx for the sample. If I use the virtualized ListView then I can improve the performance significant.

<ListView ItemsSource="{Binding Elements}" Style="{StaticResource MahApps.Styles.ListView.Virtualized}" ScrollViewer.IsDeferredScrollingEnabled="False">
</ListView>

punker76 avatar Jul 11 '22 12:07 punker76

hi - thanks for the tip !

yea - its now 123ms instead of 470 .. still about 2-3x slower than native WPF (50ms)

I'll try adding this to all my ListViews and see how much faster my app gets.

Sadly i suspect that MahApps Metro gives my apps a significant performance penalty besides this case with a simple ListView.

I'm working on 2 big inhouse projects which contains 1000s of checkboxes and tons of usercontrols - and eventhough very simple in nature - these apps are HEAVY and one of them seems to have memory leaks :( Reading up on this issue i found that MahApps which use DynamicRessource contains such memory leak: http://blog.lexique-du-net.com/index.php?post/2011/03/27/What-Dynamic-resources-creates-Memory-leaks-in-WPF-3.5-%28SP1%29

I'm wondering if i should completely remove MahApps and see if it makes a major difference ..

Montago avatar Jul 11 '22 13:07 Montago

@Montago MahApps uses standard WPF like attached properties and DynamicResource, so if it's slow in that case, then it could also still a bug in WPF itself. The DynamicResource in our case is needed because of the Theme system that we have. If we try to replace this with StaticResource then it would be a massive change with other challanges. That's maybe also a reason why Microsoft creates new technique liek UWP, WinUI or MAUI.

And yes, it would be cool if someone find a way to help here for the performance.

I'm working on 2 big inhouse projects which contains 1000s of checkboxes and tons of usercontrols - and eventhough very >simple in nature - these apps are HEAVY and one of them seems to have memory leaks :( Reading up on this issue i found that MahApps which use DynamicRessource contains such memory leak: http://blog.lexique-du-net.com/index.php?post/2011/03/27/What-Dynamic-resources-creates-Memory-leaks-in-WPF-3.5->%28SP1%29

Yeah, but that is not related to MahApps itself and you can do a workaround. https://github.com/MahApps/MahApps.Metro/issues/4100#issuecomment-832755345

So if you don't want or don't can use MahApps then it's ok, try another lib, but I think there are similar issues then.

punker76 avatar Jul 11 '22 13:07 punker76

I just noticed - the MemoryLeak was present in WPF 3.5 and the article is from 2011 .. Since im working in Framework 4.7 im using WPF 4.5+ (probably 4.7)

Like i said - i'm only interested into figuring out why there are performance issues and one thing i could try would just be to disable MahApps and see how big an impact it is...

I like the style, so maybe i could cherrypick the style setters...

Montago avatar Jul 11 '22 13:07 Montago

@Montago @Nuklon Also the latest source has some performance improvements, just tested it out and it renders faster then 2.4.9

punker76 avatar Jul 11 '22 13:07 punker76

My sample is based on latest source (just replace these files in Caliburn.Demo folder) and I don't really notice any difference with the latest official version. See below. On my main PC the performance is OKish (not great, not terrible), but on slower machines it's very noticeable when you replace the MahApps style with x:Null (which then uses WPF style).

MahApps.Metro.Caliburn.Demo.zip

The CheckBox is by far the slowest component, the other controls are mostly OK. They're a bit slower here and there but it's acceptable. With large item collections and several checkboxes, it becomes very very slow.

The style from ModernWpf (https://github.com/Kinnara/ModernWpf/blob/master/ModernWpf/Styles/CheckBox.xaml) is very fast and also supports dynamic theme switching. I think this is largely because it doesn't try to dynamically load all brushes etc at once but rather as a trigger. Perhaps MahApps can adopt a similar structure? You'd loose the ability to define it with a helper, but I'm not sure how useful that is when you can just override it with a style. Basically replace helper with a style, so you can override that instead of with the helper. Hope you understand what I mean here 🙂

Nuklon avatar Jul 11 '22 15:07 Nuklon

Thinking out loud:

Why is it that WPF with a bit of styling becomes so agonizing slow ? The Caliburn demo - as well as ALL / ANY Telerik WPF Demo just feels horrible on modern extreme PC hardware.. With HTML you could easily create something similar and it would run with 400 fps on a toaster... /rant off

@punker76 thanks - but i'll wait for an official nuget package update (release version)

I tried "upgrading" the sample i made and use the output from the Release build of MahApps.Metro as you said.. the Layout time is 100ms - so only 2x slower than WPF native.

Montago avatar Jul 12 '22 08:07 Montago

I've created a style that's the same as the current one, but doesn't use attached properties, and instead just dynamic resources on trigger (see previous comment) and it's not as fast as built-in style but much, much faster. My app goes from pretty much unusable to OK.

@punker76 What do you think about removing most attached properties from MahApps? I really don't see a use for most of these as you can simply restyle it.

Nuklon avatar Jul 18 '22 14:07 Nuklon

@Nuklon I‘ll remove all attached properties which related to Brushes and which can be done by overriding Xaml keyed stuff. I’ll this change after my holiday.

punker76 avatar Jul 18 '22 14:07 punker76

Great, have a nice holiday 🥂

Nuklon avatar Jul 18 '22 14:07 Nuklon

@Nuklon FYI https://github.com/dotnet/wpf/issues/4468

batzen avatar Aug 18 '22 06:08 batzen

@Nuklon FYI dotnet/wpf#4468

Mainly using .NET Framework though :)

Nuklon avatar Aug 20 '22 08:08 Nuklon

The same performance issue is present in Framework, due to general perf improvements in Core it should be even worse in Framework. Just wanted to point out that it's not all MahApps fault.

batzen avatar Aug 20 '22 09:08 batzen

Right :) But it can be considerable better though as written above. I know it's a workaround for a problem with .NET itself, but it'd be appreciated nonetheless :)

Nuklon avatar Aug 22 '22 09:08 Nuklon