From c6a793e833d60512fbe1d3700dd4478aba01d95d Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 20 Mar 2024 09:26:18 -0500 Subject: [PATCH] [controls] improve perf of "merged" ResourceDictionary lookups Applies to: https://github.com/dotnet/maui/issues/18505 Context: https://github.com/dotnet/maui/files/13251041/MauiCollectionView.zip I profiled the above sample with `dotnet-trace` with the following PRs applied locally: * https://github.com/dotnet/maui/pull/21229 * https://github.com/dotnet/maui/pull/21291 While scrolling, a lot of time is spent in `ResourceDictionary` lookups on an Android Pixel 5 device: 2.0% Microsoft.Maui.Controls!Microsoft.Maui.Controls.ResourceDictionary.TryGetValue(string,object&) Drilling in, I can see System.Linq's `Reverse()` method: 0.56% System.Linq!System.Linq.Enumerable.ReverseIterator.MoveNext() 0.14% System.Linq!System.Linq.Enumerable.Reverse(System.Collections.Generic.IEnumerable`1) 0.04% System.Linq!System.Linq.Enumerable.ReverseIterator..ctor(System.Collections.Generic.IEnumerable`1) 0.04% System.Linq!System.Linq.Enumerable.ReverseIterator.Dispose() `Reverse()` can be problematic as it can sometimes create a copy of the entire collection, in order to sort in reverse. We can juse use a reverse `for`-loop instead. The indexer, we can also avoid a double-lookup: if (dict.ContainsKey(index)) return dict[index]; And instead do: if (dict.TryGetValue(index, out var value)) return value; The MAUI project template seems to setup a few "merged" `ResourceDictionary` as it contains `Styles.xaml`, so this is why this code path is being hit. I wrote a BenchmarkDotNet benchmark, and it indicates the collection is being copied, as the 872 bytes of allocation occur: | Method | key | Mean | Error | StdDev | Gen0 | Allocated | |------------ |------------ |----------:|---------:|---------:|-------:|----------:| | TryGetValue | key0 | 11.45 ns | 0.026 ns | 0.023 ns | - | - | | Indexer | key0 | 24.72 ns | 0.133 ns | 0.118 ns | - | - | | TryGetValue | merged99,99 | 117.06 ns | 2.334 ns | 2.497 ns | 0.1042 | 872 B | | Indexer | merged99,99 | 145.60 ns | 2.737 ns | 2.286 ns | 0.1042 | 872 B | With these changes in place, I see less time spent inside: 0.91% Microsoft.Maui.Controls!Microsoft.Maui.Controls.ResourceDictionary.TryGetValue(string,object&) The benchmark no longer allocates either: | Method | key | Mean | Error | StdDev | Allocated | |------------ |------------ |----------:|----------:|----------:|----------:| | TryGetValue | key0 | 11.92 ns | 0.094 ns | 0.084 ns | - | | Indexer | merged99,99 | 23.12 ns | 0.418 ns | 0.391 ns | - | | Indexer | key0 | 24.20 ns | 0.485 ns | 0.453 ns | - | | TryGetValue | merged99,99 | 29.09 ns | 0.296 ns | 0.262 ns | - | This should improve the performance "parenting" of any MAUI view on all platforms -- as well as scrolling `CollectionView`. --- src/Controls/src/Core/ResourceDictionary.cs | 20 +++++++-- .../ResourceDictionaryBenchmarker.cs | 45 +++++++++++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 src/Core/tests/Benchmarks/Benchmarks/ResourceDictionaryBenchmarker.cs diff --git a/src/Controls/src/Core/ResourceDictionary.cs b/src/Controls/src/Core/ResourceDictionary.cs index be4ff7684758..5cb81399d0df 100644 --- a/src/Controls/src/Core/ResourceDictionary.cs +++ b/src/Controls/src/Core/ResourceDictionary.cs @@ -195,9 +195,17 @@ public object this[string index] if (_mergedInstance != null && _mergedInstance.ContainsKey(index)) return _mergedInstance[index]; if (_mergedDictionaries != null) - foreach (var dict in MergedDictionaries.Reverse()) - if (dict.ContainsKey(index)) - return dict[index]; + { + var dictionaries = (ObservableCollection)MergedDictionaries; + for (int i = dictionaries.Count - 1; i >= 0 ; i--) + { + if (dictionaries[i].TryGetValue(index, out var value)) + { + return value; + } + } + } + throw new KeyNotFoundException($"The resource '{index}' is not present in the dictionary."); } set @@ -286,12 +294,16 @@ internal bool TryGetValueAndSource(string key, out object value, out ResourceDic bool TryGetMergedDictionaryValue(string key, out object value, out ResourceDictionary source) { - foreach (var dictionary in MergedDictionaries.Reverse()) + var dictionaries = (ObservableCollection)MergedDictionaries; + for (int i = dictionaries.Count - 1; i >= 0 ; i--) + { + var dictionary = dictionaries[i]; if (dictionary.TryGetValue(key, out value)) { source = dictionary; return true; } + } value = null; source = null; diff --git a/src/Core/tests/Benchmarks/Benchmarks/ResourceDictionaryBenchmarker.cs b/src/Core/tests/Benchmarks/Benchmarks/ResourceDictionaryBenchmarker.cs new file mode 100644 index 000000000000..897a54fe17bd --- /dev/null +++ b/src/Core/tests/Benchmarks/Benchmarks/ResourceDictionaryBenchmarker.cs @@ -0,0 +1,45 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Order; +using Microsoft.Maui.Controls; + +namespace Microsoft.Maui.Handlers.Benchmarks +{ + [MemoryDiagnoser] + [Orderer(SummaryOrderPolicy.FastestToSlowest)] + public class ResourceDictionaryBenchmarker + { + const int Size = 100; + readonly ResourceDictionary _resourceDictionary; + + public ResourceDictionaryBenchmarker() + { + _resourceDictionary = new ResourceDictionary(); + for (var i = 0; i < Size; i++) + { + _resourceDictionary.Add($"key{i}", i); + } + + for (var j = 0; j < Size; j++) + { + var merged = new ResourceDictionary(); + for (var i = 0; i < Size; i++) + { + merged.Add($"merged{i},{j}", i); + } + _resourceDictionary.MergedDictionaries.Add(merged); + } + } + + [Benchmark] + [Arguments("key0")] + [Arguments("merged50,50")] + [Arguments("merged99,99")] + public void TryGetValue(string key) => _resourceDictionary.TryGetValue(key, out _); + + [Benchmark] + [Arguments("key0")] + [Arguments("merged50,50")] + [Arguments("merged99,99")] + public void Indexer(string key) => _ = _resourceDictionary[key]; + } +} \ No newline at end of file