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

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

Open
oatkins opened this issue May 3, 2021 · 14 comments
Open
Labels
.NET Framework Performance Performance related issue
Milestone

Comments

@oatkins
Copy link

oatkins commented May 3, 2021

  • .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.
@batzen
Copy link
Contributor

batzen commented Aug 14, 2021

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
Copy link
Contributor

batzen commented Aug 14, 2021

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

@batzen
Copy link
Contributor

batzen commented Aug 14, 2021

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

@Symbai
Copy link
Contributor

Symbai commented Aug 14, 2021

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
Copy link

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

@batzen
Copy link
Contributor

batzen commented Sep 30, 2021

FYI: Will create a PR during Hacktoberfest

@jacobgalloway
Copy link

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
Copy link
Contributor

Symbai commented Jan 11, 2023

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
Copy link
Member

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
Copy link

ahdung commented Jul 6, 2023

Any news?

@batzen
Copy link
Contributor

batzen commented Apr 20, 2024

@pchaurasia14 Another year passed. Any news?

@Kuldeep-MS
Copy link
Member

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

@h3xds1nz
Copy link
Contributor

h3xds1nz commented Aug 6, 2024

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

@mahara
Copy link

mahara commented Aug 7, 2024

@h3xds1nz It's still related to this: #9501.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.NET Framework Performance Performance related issue
Projects
None yet
Development

No branches or pull requests