Improving DynamicResource performance (second try) (#10532)

* Using alternative storage for dynamic resources

* Fixing removal

* Removing not required check

Co-authored-by: Copilot <[email protected]>

* Improving Contains

* Addressing PR feedback

* Adjusting code to false positive IDE warning...

* Addressing PR feedback

---------

Co-authored-by: Copilot <[email protected]>

Author: batzen

src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Markup/Baml2006/Baml2006KeyRecord.cs

@@ -5,7 +5,7 @@
 
 namespace System.Windows.Baml2006
 {
-    [DebuggerDisplay("{DebuggerString}")]
+    [DebuggerDisplay("{_data}")]
     internal class KeyRecord
     {
         public KeyRecord(bool shared, bool sharedSet, int valuePosition, Type keyType) :
@@ -70,7 +70,7 @@ public StaticResource LastStaticResource
 
         public string KeyString
         {
-            get { return _data as String; }
+            get { return _data as string; }
         }
 
         public Type KeyType

src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs

@@ -695,6 +695,8 @@ private void ClearWithoutLock()
                 // to the old resource before we clear it.
                 ValidateDeferredResourceReferences(null);
 
+                _weakDeferredResourceReferencesMap?.Clear();
+
                 // remove inheritance context from all values that got it from
                 // this dictionary
                 RemoveInheritanceContextFromValues();
@@ -706,6 +708,61 @@ private void ClearWithoutLock()
             }
         }
 
+        // optimized contains method used in FetchResource
+        private void Contains(object key, bool mustReturnDeferredResourceReference, out bool contains, out bool containsBamlObjectFactory)
+        {
+            contains = false;
+            containsBamlObjectFactory = false;
+
+            bool result = _baseDictionary.Contains(key);
+
+            if (result)
+            {
+                KeyRecord keyRecord = _baseDictionary[key] as KeyRecord;
+                if (keyRecord != null && _deferredLocationList.Contains(keyRecord))
+                {
+                    contains = false;
+                }
+                else
+                {
+                    contains = true;
+                }
+
+                if (mustReturnDeferredResourceReference
+                    && contains)
+                {
+                    return;
+                }
+
+                containsBamlObjectFactory = keyRecord is not null;
+
+                if (containsBamlObjectFactory)
+                {
+                    return;
+                }
+            }
+
+            //Search for the value in the Merged Dictionaries
+            if (_mergedDictionaries != null)
+            {
+                for (int i = MergedDictionaries.Count - 1; i > -1; i--)
+                {
+                    // Note that MergedDictionaries collection can also contain null values
+                    ResourceDictionary mergedDictionary = MergedDictionaries[i];
+                    if (mergedDictionary != null)
+                    {
+                        mergedDictionary.Contains(key, mustReturnDeferredResourceReference, out contains, out containsBamlObjectFactory);
+
+                        if (containsBamlObjectFactory
+                            || (mustReturnDeferredResourceReference && contains))
+                        {
+                            return;
+                        }
+                    }
+                }
+            }
+        }
+
         /// <summary>
         ///     Determines whether the IDictionary contains an element with the specified key.
         ///     if the Key is not contained in this ResourceDictionary, it will check in the MergedDictionaries too
@@ -1718,8 +1775,10 @@ private object FetchResource(
 
             if (allowDeferredResourceReference)
             {
-                if (ContainsBamlObjectFactory(resourceKey) ||
-                    (mustReturnDeferredResourceReference && Contains(resourceKey)))
+                Contains(resourceKey, mustReturnDeferredResourceReference, out bool contains, out bool containsBamlObjectFactory);
+
+                if (containsBamlObjectFactory
+                    || (mustReturnDeferredResourceReference && contains))
                 {
                     canCache = false;
 
@@ -1750,19 +1809,8 @@ private object FetchResource(
                         {
                             // Cache the deferredResourceReference so that it can be validated
                             // in case of a dictionary change prior to its inflation
-                            _deferredResourceReferencesList ??= new DeferredResourceReferenceList();
-
-                            if (_deferredResourceReferencesList.Get(resourceKey) is { } existingDeferredResourceReference
-                                && existingDeferredResourceReference.Dictionary == this)
-                            {
-                                deferredResourceReference = existingDeferredResourceReference;
-                            }
-                            else
-                            {
-                                deferredResourceReference = _ownerApps is not null ? new DeferredAppResourceReference(this, resourceKey) : new DeferredResourceReference(this, resourceKey);
-
-                                _deferredResourceReferencesList.AddOrSet(deferredResourceReference);
-                            }
+                            deferredResourceReference = _ownerApps is not null ? new DeferredAppResourceReference(this, resourceKey) : new DeferredResourceReference(this, resourceKey);
+                            GetOrCreateWeakReferenceList(resourceKey).Add(deferredResourceReference, true /*SkipFind*/);
                         }
                     }
                     else
@@ -1779,6 +1827,35 @@ private object FetchResource(
             return GetValue(resourceKey, out canCache);
         }
 
+        private WeakReferenceList GetOrCreateWeakReferenceList(object resourceKey)
+        {
+            this._weakDeferredResourceReferencesMap ??= new();
+
+            if (!this._weakDeferredResourceReferencesMap.TryGetValue(resourceKey, out var weakDeferredResourceReferences))
+            {
+                weakDeferredResourceReferences = new WeakReferenceList();
+                this._weakDeferredResourceReferencesMap[resourceKey] = weakDeferredResourceReferences;
+            }
+
+            return weakDeferredResourceReferences;
+        }
+
+        internal void RemoveDeferredResourceReference(DeferredResourceReference deferredResourceReference)
+        {
+            if (FrameworkAppContextSwitches.DisableDynamicResourceOptimization)
+            {
+                _weakDeferredResourceReferences?.Remove(deferredResourceReference);
+            }
+            else
+            {
+                //GetWeakReferenceList(deferredResourceReference.Key)?.Remove(deferredResourceReference);
+                if (this._weakDeferredResourceReferencesMap?.TryGetValue(deferredResourceReference.Key, out var weakDeferredResourceReferences) is true)
+                {
+                    weakDeferredResourceReferences.Remove(deferredResourceReference);
+                }
+            }
+        }
+
         /// <summary>
         /// Validate the deferredResourceReference with the given key. Key could be null meaning
         /// some catastrophic operation occurred so simply validate all DeferredResourceReferences
@@ -1804,34 +1881,45 @@ private void ValidateDeferredResourceReferences(object resourceKey)
             }
             else
             {
-                if (_deferredResourceReferencesList is null)
+                if (_weakDeferredResourceReferencesMap is null)
                 {
                     return;
                 }
 
                 if (resourceKey is null)
                 {
-                    foreach (DeferredResourceReference deferredResourceReference in _deferredResourceReferencesList)
+                    foreach (var weakDeferredResourceReferences in _weakDeferredResourceReferencesMap.Values)
                     {
-                        Inflate(deferredResourceReference);
+                        foreach (var weakResourceReference in weakDeferredResourceReferences)
+                        {
+                            DeferredResourceReference deferredResourceReference = weakResourceReference as DeferredResourceReference;
+
+                            Inflate(deferredResourceReference);
+                        }
                     }
                 }
                 else
                 {
-                    DeferredResourceReference deferredResourceReference = _deferredResourceReferencesList.Get(resourceKey);
+                    if (_weakDeferredResourceReferencesMap.TryGetValue(resourceKey, out var weakDeferredResourceReferences))
+                    {
+                        foreach (var weakResourceReference in weakDeferredResourceReferences)
+                        {
+                            DeferredResourceReference deferredResourceReference = weakResourceReference as DeferredResourceReference;
 
-                    Inflate(deferredResourceReference);
+                            Inflate(deferredResourceReference);
+                        }
+                    }
                 }
+            }
 
-                return;
+            return;
 
-                void Inflate(DeferredResourceReference deferredResourceReference)
-                {
-                    // This will inflate the deferred reference, causing it
-                    // to be removed from the list.  The list may also be
-                    // purged of dead references.
-                    deferredResourceReference?.GetValue(BaseValueSourceInternal.Unknown);
-                }
+            void Inflate(DeferredResourceReference deferredResourceReference)
+            {
+                // This will inflate the deferred reference, causing it
+                // to be removed from the list.  The list may also be
+                // purged of dead references.
+                deferredResourceReference?.GetValue(BaseValueSourceInternal.Unknown);
             }
         }
 
@@ -2095,20 +2183,6 @@ internal WeakReferenceList ApplicationOwners
 
         #endregion HelperMethods
 
-        #region Properties
-
-        internal WeakReferenceList WeakDeferredResourceReferences
-        {
-            get { return _weakDeferredResourceReferences; }
-        }
-
-        internal DeferredResourceReferenceList DeferredResourceReferencesList
-        {
-            get { return _deferredResourceReferencesList; }
-        }
-
-        #endregion Properties
-
         #region Enumeration
 
         /// <summary>
@@ -2519,11 +2593,11 @@ private void CopyDeferredContentFrom(ResourceDictionary loadedRD)
             IsUnsafe = loadedRD.IsUnsafe;
         }
 
-        private void  MoveDeferredResourceReferencesFrom(ResourceDictionary loadedRD)
+        private void MoveDeferredResourceReferencesFrom(ResourceDictionary loadedRD)
         {
             if (FrameworkAppContextSwitches.DisableDynamicResourceOptimization)
             {
-                // copy the list
+                // move the list
                 _weakDeferredResourceReferences = loadedRD._weakDeferredResourceReferences;
 
                 // redirect each entry toward its new owner
@@ -2537,11 +2611,20 @@ private void  MoveDeferredResourceReferencesFrom(ResourceDictionary loadedRD)
             }
             else
             {
-                // copy the list
-                _deferredResourceReferencesList = loadedRD._deferredResourceReferencesList;
+                // move the map and thus the lists
+                _weakDeferredResourceReferencesMap = loadedRD._weakDeferredResourceReferencesMap;
 
-                // redirect each entry toward its new owner
-                _deferredResourceReferencesList?.ChangeDictionary(this);
+                if (_weakDeferredResourceReferencesMap is not null)
+                {
+                    // redirect each entry toward its new owner
+                    foreach (var weakDeferredResourceReferences in _weakDeferredResourceReferencesMap.Values)
+                    {
+                        foreach (DeferredResourceReference drr in weakDeferredResourceReferences)
+                        {
+                            drr.Dictionary = this;
+                        }
+                    }
+                }
             }
         }
 
@@ -2609,7 +2692,7 @@ private enum FallbackState
         private WeakReferenceList                         _ownerFCEs = null;
         private WeakReferenceList                         _ownerApps = null;
         private WeakReferenceList                         _weakDeferredResourceReferences = null;
-        private DeferredResourceReferenceList             _deferredResourceReferencesList = null;
+        private Dictionary<object, WeakReferenceList>     _weakDeferredResourceReferencesMap = null;
         private ObservableCollection<ResourceDictionary>  _mergedDictionaries = null;
         private Uri                                       _source = null;
         private Uri                                       _baseUri = null;

src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/StyleHelper.cs

@@ -662,18 +662,19 @@ private static bool RequiresInstanceStorage(ref object value)
                 Type valueType = deferredReference.GetValueType();
                 if (valueType != null)
                 {
-                    if (typeof(MarkupExtension).IsAssignableFrom(valueType))
+                    // Check for Freezable first, as that's way more common than MarkupExtension
+                    if (typeof(Freezable).IsAssignableFrom(valueType))
+                    {
+                        freezable = (Freezable)deferredReference.GetValue(BaseValueSourceInternal.Style);
+                    }
+                    else if (typeof(MarkupExtension).IsAssignableFrom(valueType))
                     {
                         value = deferredReference.GetValue(BaseValueSourceInternal.Style);
                         if ((markupExtension = value as MarkupExtension) == null)
                         {
                             freezable = value as Freezable;
                         }
                     }
-                    else if (typeof(Freezable).IsAssignableFrom(valueType))
-                    {
-                        freezable = (Freezable)deferredReference.GetValue(BaseValueSourceInternal.Style);
-                    }
                 }
 
             }