dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.05k stars 1.17k forks source link

Very slow performance due to internal WPF deferred resource reference list #4468

Open oatkins opened 3 years ago

oatkins commented 3 years ago

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:

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:

batzen commented 3 years ago

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 commented 3 years ago

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

batzen commented 3 years ago

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

Symbai commented 3 years ago

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.

AdamCaviness commented 3 years ago

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

batzen commented 3 years ago

FYI: Will create a PR during Hacktoberfest

jacobgalloway commented 1 year ago

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.

Symbai commented 1 year ago

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

pchaurasia14 commented 1 year ago

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.

ahdung commented 1 year ago

Any news?

batzen commented 5 months ago

@pchaurasia14 Another year passed. Any news?

Kuldeep-MS commented 5 months ago

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

h3xds1nz commented 2 months ago

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

mahara commented 2 months ago

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