wpf icon indicating copy to clipboard operation
wpf copied to clipboard

Very slow performance due to internal WPF deferred resource reference list

Open oatkins opened this issue 3 years ago • 14 comments

  • .NET Core Version: 5.0.5
  • Windows version: 10.0.19042
  • Does the bug reproduce also in WPF for .NET Framework 4.8?: Yes
  • Is this bug related specifically to tooling in Visual Studio (e.g. XAML Designer, Code editing, etc...)? No

Problem description: Opening, closing and moving between views in a WPF application becomes extremely slow.

Actual behavior: Debugging tools show that considerable time is spent in FindWeakReference() iterating over items in ResourceDictionary._deferredResourceReferences.

image

The list accumulates tens of thousands of entries.

Expected behavior: No delays as described above. From browsing the source code, we believe that entries in _deferredResourceReferences are intended to be short lived.

Minimal repro: https://github.com/oatkins/WpfDeferredResourceLookupRepro/tree/repro

The above repro is a bit contrived, but reproduces delays of the same order of magnitude as we are seeing in a real LOB application. It seems that all of the following are needed to reproduce this:

  1. Controls whose style has dependency properties set to DynamicResource references.
  2. Some of those dependency properties supply values that are not used straight away or at all. (This apparently causes evaluation of the property to be deferred).
  3. At least one resource is redefined: that is, in the resource dictionaries merged into the application's resources, it is defined in more than one place.
  4. For the controls referenced in 1. above, at least one other dependency property that is used is set using a DynamicResource reference.

Steps 1-3 cause the list of _deferredResourceReferences to become long, but this doesn't actually affect performance without step 4.

This is easy to reproduce using the popular MahApps.Metro UI toolkit for the following reasons:

  • It uses dynamic resources extensively to support theme switching [1 and 4 above].
  • Some styles adopt a pattern that involves setting many dependency properties [2 above].
  • Some dependency properties are rarely accessed, e.g. Validation.ErrorTemplate and ContextMenuService.ContextMenu [2 above].
  • It's common to import all the MahApps styles into the application's resources, then override some of them with customized versions [3 above].

In our application, this performance bottleneck is typically responsible for adding an additional 7 seconds to the time it takes to open a form, bringing the total time to 10 seconds. It accounts for an even longer delay closing the same form.

Note that, while an application using MahApps is particularly likely to exhibit this behaviour, I'm reporting it here because its quite easily reproducible in an application that doesn't deviate from normal WPF coding practices.

We are hoping to work around this by modifying our styles (and overriding default MahApps styles) to reduce the number of dependency properties that are assigned a value using DynamicResource when that value is not read almost immediately. Our application currently runs on .NET Framework 4.8, but we hope, in time, to migrate it to .NET 5 or 6. However, the repro linked above shows that the problem is currently manifest in both flavours of WPF.

This issue is to do a few things:

  • Track discussion about whether this could be improved in WPF itself.
    • Is it expected that the list of deferred resources should grow to contain tens of thousands of items? Is retaining these necessary in this situation, or could the code be adjusted to release them earlier?
    • If not, could the list be re-implemented as some kind of hash set to reduce the time spent in FindWeakReference()?
  • Solicit any other suggestions on how we might avoid hitting this scenario.
    • We realise that changing this in WPF itself might be considered high risk for you (especially in .NET Framework), but we'd appreciate your observations, especially on whether this is a "known issue" with WPF.

oatkins avatar May 03 '21 14:05 oatkins

I am currently trying to solve this. Original (add): Count: 8000 Duration: 00:00:05.6561398 Memory: 212 MB

Original (remove): Count: 3 (temporarily 16000) Duration: 00:00:11.6930969 Memory: 202 MB

Modified (add): Count: 1 Duration: 00:00:03.4468477 Memory: 165 MB

Modified (remove): Count: 2 Duration: 00:00:00.9332898 Memory: 166 MB

Switching tabs in the repro application takes about 42 seconds in the original version and about 4 seconds in the modified version.

Please note: The modified WPF version was built in debug mode as i am unable to build in release mode locally. I guess it would be even faster when built in release mode. I also used the changes from #4964. The pure changes can be seen at https://github.com/batzen/wpf/tree/issues/weakreferencelist

If the WPF team would be interested in a PR i would be happy to create one. The changes required only affect a few lines in ResourceDictionary.cs and a new special class which replaces WeakReferenceList for the storage of DeferredResourceReference.

batzen avatar Aug 14 '21 09:08 batzen

Allocations are also down from 121 MB to 95 MB during add. Didn't profile the other operations.

batzen avatar Aug 14 '21 09:08 batzen

And i guess perf could be further improved when CanBeAccessedAcrossThreads from ResourceDictionary would be taken into account in the new "list" class.

batzen avatar Aug 14 '21 09:08 batzen

If the WPF team would be interested in a PR i would be happy to create one.

You know the WPF team batzen... If you write this here you never get an answer. Create a PR and see how it goes.

Symbai avatar Aug 14 '21 10:08 Symbai

We have been facing this issue for years. I have hoped for such a long to see this corrected.

AdamCaviness avatar Sep 30 '21 15:09 AdamCaviness

FYI: Will create a PR during Hacktoberfest

batzen avatar Sep 30 '21 17:09 batzen

Is there any news about this issue? We are running our application in .NET 4.6.2 and it seems to be this same issue. We seemed to have started seeing it once we migrated to using DynamicResources in more places for common components, but it only gets worse and worse over time after extended use of the application.

jacobgalloway avatar Jan 11 '23 19:01 jacobgalloway

As you can see in batzen's PR #5610, the WPF team is ignoring him. Not only the PR is not getting reviewed, they even don't answer his questions. Also see #6556

Symbai avatar Jan 11 '23 20:01 Symbai

Apologies, this got dropped from our radar.  We will review this in upcoming CTP(s) cycles. 

Since some of the PR(s) affect WPF core, we believe our existing tests may not be adequate in highlighting any problems early-on. That being said, is still not an excuse for extended radio silence on such PR(s). We will work on improving the situation.

pchaurasia14 avatar Jan 12 '23 14:01 pchaurasia14

Any news?

ahdung avatar Jul 06 '23 02:07 ahdung

@pchaurasia14 Another year passed. Any news?

batzen avatar Apr 20 '24 08:04 batzen

@batzen Apologies for the delay. #5610 has been included for the upcoming CTP.

Kuldeep-MS avatar Apr 22 '24 10:04 Kuldeep-MS

@dipeshmsft Should this be closed now as #5610 has been merged?

h3xds1nz avatar Aug 06 '24 22:08 h3xds1nz

@h3xds1nz It's still related to this: https://github.com/dotnet/wpf/pull/9501.

mahara avatar Aug 07 '24 08:08 mahara