From 9d09d7c00d4674ad2705a6737aa277d89ecac818 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 19 Feb 2026 14:18:38 +0100 Subject: [PATCH 1/3] Reduce allocations in inherited BindingContext propagation Fixes #34129 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Controls/src/Core/BindableObject.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Controls/src/Core/BindableObject.cs b/src/Controls/src/Core/BindableObject.cs index 40f12defa5fe..8fb7052f4007 100644 --- a/src/Controls/src/Core/BindableObject.cs +++ b/src/Controls/src/Core/BindableObject.cs @@ -363,7 +363,10 @@ public static void SetInheritedBindingContext(BindableObject bindable, object va } else { - bindable._inheritedContext = new WeakReference(value); + if (bindable._inheritedContext is not null) + bindable._inheritedContext.Target = value; + else + bindable._inheritedContext = new WeakReference(value); bindable.ApplyBindings(fromBindingContextChanged: true); bindable.OnBindingContextChanged(); } @@ -692,11 +695,9 @@ private protected virtual void OnBindablePropertySet(BindableProperty property, void ApplyBindings(bool fromBindingContextChanged) { - var prop = _properties.Values.ToArray(); - - for (int i = 0, propLength = prop.Length; i < propLength; i++) + foreach (var kvp in _properties) { - BindablePropertyContext context = prop[i]; + BindablePropertyContext context = kvp.Value; if (ReferenceEquals(context.Property, BindingContextProperty)) { // BindingContextProperty Binding is handled separately within SetInheritedBindingContext From dcd3a94bb107b6a3aa38f21b9f950c7358b7209c Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 19 Feb 2026 14:36:09 +0100 Subject: [PATCH 2/3] Simplify ApplyBindings to iterate _properties.Values directly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Controls/src/Core/BindableObject.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Controls/src/Core/BindableObject.cs b/src/Controls/src/Core/BindableObject.cs index 8fb7052f4007..a9a5bcf04d20 100644 --- a/src/Controls/src/Core/BindableObject.cs +++ b/src/Controls/src/Core/BindableObject.cs @@ -695,9 +695,8 @@ private protected virtual void OnBindablePropertySet(BindableProperty property, void ApplyBindings(bool fromBindingContextChanged) { - foreach (var kvp in _properties) + foreach (var context in _properties.Values) { - BindablePropertyContext context = kvp.Value; if (ReferenceEquals(context.Property, BindingContextProperty)) { // BindingContextProperty Binding is handled separately within SetInheritedBindingContext From d65b88ead3ea16e5fdbd879cfe7ba199014c72bb Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 11 Mar 2026 11:43:44 +0100 Subject: [PATCH 3/3] Restore ToArray snapshot in ApplyBindings for re-entrancy safety ApplyBinding can trigger property-changed callbacks that modify _properties mid-iteration. Keep the snapshot to avoid InvalidOperationException. Added explanatory comment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Controls/src/Core/BindableObject.cs | 6 +- .../BindableObjectAllocBenchmarker.cs | 194 ++++-------------- 2 files changed, 44 insertions(+), 156 deletions(-) diff --git a/src/Controls/src/Core/BindableObject.cs b/src/Controls/src/Core/BindableObject.cs index a9a5bcf04d20..f2551581477a 100644 --- a/src/Controls/src/Core/BindableObject.cs +++ b/src/Controls/src/Core/BindableObject.cs @@ -695,7 +695,11 @@ private protected virtual void OnBindablePropertySet(BindableProperty property, void ApplyBindings(bool fromBindingContextChanged) { - foreach (var context in _properties.Values) + // Snapshot the property contexts — ApplyBinding can trigger re-entrant + // property changes (via callbacks) that modify _properties mid-iteration. + var contexts = _properties.Values.ToArray(); + + foreach (var context in contexts) { if (ReferenceEquals(context.Property, BindingContextProperty)) { diff --git a/src/Core/tests/Benchmarks/Benchmarks/BindableObjectAllocBenchmarker.cs b/src/Core/tests/Benchmarks/Benchmarks/BindableObjectAllocBenchmarker.cs index 078e00ec8a06..8a896d3a6597 100644 --- a/src/Core/tests/Benchmarks/Benchmarks/BindableObjectAllocBenchmarker.cs +++ b/src/Core/tests/Benchmarks/Benchmarks/BindableObjectAllocBenchmarker.cs @@ -6,52 +6,21 @@ namespace Microsoft.Maui.Handlers.Benchmarks [MemoryDiagnoser] public class BindableObjectAllocBenchmarker { - // --- #34092: Cached PropertyChangedEventArgs / PropertyChangingEventArgs --- - - /// - /// Sets the same property repeatedly to measure PropertyChanged/Changing EventArgs allocations. - /// Before: new PropertyChangedEventArgs + new PropertyChangingEventArgs per call. - /// After: cached per BindableProperty. - /// - [Benchmark] - public void SetProperty_EventArgsAlloc() + Label _label; + Label _child; + VerticalStackLayout _deepTreeLeaf; + VerticalStackLayout _flatLayout; + object _contextA, _contextB; + bool _toggle; + + [GlobalSetup] + public void Setup() { - var label = new Label(); - for (int i = 0; i < 1_000; i++) - { - label.Text = "a"; - label.Text = "b"; - } - } + _label = new Label(); + _contextA = new object(); + _contextB = new object(); - /// - /// Sets multiple different properties to show caching benefit across properties. - /// - [Benchmark] - public void SetMultipleProperties_EventArgsAlloc() - { - var entry = new Entry(); - for (int i = 0; i < 500; i++) - { - entry.Text = "a"; - entry.Placeholder = "p"; - entry.FontSize = 14 + (i % 3); - entry.Text = "b"; - entry.Placeholder = "q"; - } - } - - // --- #34093: Reuse ElementEventArgs in tree propagation --- - - /// - /// Adds children to a deep hierarchy, triggering DescendantAdded propagation. - /// Before: new ElementEventArgs at every tree level. - /// After: single ElementEventArgs reused through recursion. - /// - [Benchmark] - public void AddChildren_DeepTree_ElementEventArgs() - { - // Build a 10-level deep tree + // 10-level deep tree for DescendantAdded/Removed propagation var root = new VerticalStackLayout(); var current = root; for (int depth = 0; depth < 10; depth++) @@ -60,133 +29,48 @@ public void AddChildren_DeepTree_ElementEventArgs() current.Add(child); current = child; } + _child = new Label(); + _deepTreeLeaf = current; - // Add 100 leaves at the bottom — each fires DescendantAdded 10 levels up - for (int i = 0; i < 100; i++) - { - current.Add(new Label()); - } + // Flat layout with 200 children for BindingContext propagation + _flatLayout = new VerticalStackLayout(); + for (int i = 0; i < 200; i++) + _flatLayout.Add(new Label()); } - /// - /// Removes children from a deep hierarchy, triggering DescendantRemoved propagation. - /// - [Benchmark] - public void RemoveChildren_DeepTree_ElementEventArgs() - { - var root = new VerticalStackLayout(); - var current = root; - for (int depth = 0; depth < 10; depth++) - { - var child = new VerticalStackLayout(); - current.Add(child); - current = child; - } - - var labels = new Label[100]; - for (int i = 0; i < 100; i++) - { - labels[i] = new Label(); - current.Add(labels[i]); - } + // --- #34092: Cached PropertyChangedEventArgs / PropertyChangingEventArgs --- - for (int i = 0; i < 100; i++) - { - current.Remove(labels[i]); - } + [Benchmark(Description = "SetValue (EventArgs)")] + public void SetProperty_EventArgs() + { + _toggle = !_toggle; + _label.Text = _toggle ? "a" : "b"; } - // --- #34129: BindingContext propagation (WeakReference reuse + .ToArray() elimination) --- + // --- #34093: Reuse ElementEventArgs in tree propagation --- - class SimpleViewModel + [Benchmark(Description = "Add+Remove child (10-deep tree)")] + public void AddRemoveChild_DeepTree() { - public string Name { get; set; } = "Test"; + _deepTreeLeaf.Add(_child); + _deepTreeLeaf.Remove(_child); } - /// - /// Sets BindingContext on a flat layout with many children. - /// Before: new WeakReference per child + .ToArray() on each child's _properties. - /// After: reuse WeakReference.Target + foreach on dictionary directly. - /// - [Benchmark] + // --- #34129: BindingContext propagation (WeakReference reuse + .ToArray() elimination) --- + + [Benchmark(Description = "Set BindingContext (200 children)")] public void SetBindingContext_FlatTree() { - var layout = new VerticalStackLayout(); - for (int i = 0; i < 200; i++) - { - layout.Add(new Label()); - } - - var vm = new SimpleViewModel(); - for (int i = 0; i < 10; i++) - { - layout.BindingContext = vm; - layout.BindingContext = null; - } - } - - /// - /// Sets BindingContext on a deep tree where children have bindings. - /// This is the worst-case hot path: ApplyBindings + WeakReference for every descendant. - /// - [Benchmark] - public void SetBindingContext_DeepTreeWithBindings() - { - var root = new VerticalStackLayout(); - var current = root; - - for (int depth = 0; depth < 5; depth++) - { - var child = new VerticalStackLayout(); - current.Add(child); - current = child; - } - - for (int i = 0; i < 50; i++) - { - var label = new Label(); - label.SetBinding(Label.TextProperty, new Binding("Name")); - current.Add(label); - } - - var vm = new SimpleViewModel(); - for (int i = 0; i < 20; i++) - { - root.BindingContext = vm; - root.BindingContext = null; - } + _toggle = !_toggle; + _flatLayout.BindingContext = _toggle ? _contextA : _contextB; } // --- #34131: Lazy _triggerSpecificity dictionary --- - /// - /// Creates many BindableObjects (Labels) that never use triggers. - /// Before: each allocates Dictionary<TriggerBase, SetterSpecificity>. - /// After: dictionary is null until first trigger attachment. - /// - [Benchmark] - public Label[] CreateManyLabels_NoTriggers() - { - var labels = new Label[1_000]; - for (int i = 0; i < 1_000; i++) - { - labels[i] = new Label(); - } - return labels; - } + [Benchmark(Description = "new Label()")] + public Label CreateLabel() => new Label(); - /// - /// Creates many Entries (more complex BindableObject) without triggers. - /// - [Benchmark] - public Entry[] CreateManyEntries_NoTriggers() - { - var entries = new Entry[500]; - for (int i = 0; i < 500; i++) - { - entries[i] = new Entry(); - } - return entries; - } + [Benchmark(Description = "new Entry()")] + public Entry CreateEntry() => new Entry(); } }