Merge pull request #4078 from MarchingCube/perf-inpc-before-method

Try matching INPC properties before methods

src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs

@@ -12,7 +12,7 @@ namespace Avalonia.Data.Core.Plugins
     public class InpcPropertyAccessorPlugin : IPropertyAccessorPlugin
     {
         /// <inheritdoc/>
-        public bool Match(object obj, string propertyName) => true;
+        public bool Match(object obj, string propertyName) => GetPropertyWithName(obj.GetType(), propertyName) != null;
 
         /// <summary>
         /// Starts monitoring the value of a property on an object.
@@ -30,10 +30,7 @@ namespace Avalonia.Data.Core.Plugins
 
             reference.TryGetTarget(out object instance);
 
-            const BindingFlags bindingFlags = BindingFlags.NonPublic | BindingFlags.Public |
+            var p = GetPropertyWithName(instance.GetType(), propertyName);
-                                              BindingFlags.Static | BindingFlags.Instance;
-
-            var p = instance.GetType().GetProperty(propertyName, bindingFlags);
 
             if (p != null)
             {
@@ -47,6 +44,14 @@ namespace Avalonia.Data.Core.Plugins
             }
         }
 
+        private static PropertyInfo GetPropertyWithName(Type type, string propertyName)
+        {
+            const BindingFlags bindingFlags = BindingFlags.NonPublic | BindingFlags.Public |
+                                              BindingFlags.Static | BindingFlags.Instance;
+
+            return type.GetProperty(propertyName, bindingFlags);
+        }
+
         private class Accessor : PropertyAccessorBase, IWeakSubscriber<PropertyChangedEventArgs>
         {
             private readonly WeakReference<object> _reference;

src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs

@@ -62,8 +62,13 @@ namespace Avalonia.Data.Core
 
             if (accessor == null)
             {
-                throw new NotSupportedException(
+                reference.TryGetTarget(out object instance);
-                    $"Could not find a matching property accessor for {PropertyName}.");
+
+                var message = $"Could not find a matching property accessor for '{PropertyName}' on '{instance}'";
+
+                var exception = new MissingMemberException(message);
+
+                accessor = new PropertyError(new BindingNotification(exception, BindingErrorType.Error));
             }
 
             _accessor = accessor;

tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_Property.cs

@@ -322,7 +322,7 @@ namespace Avalonia.Base.UnitTests.Data.Core
                 {
                     "bar",
                     new BindingNotification(
-                        new MissingMemberException("Could not find CLR property 'Bar' on 'Avalonia.Base.UnitTests.Data.Core.ExpressionObserverTests_Property+WithoutBar'"),
+                        new MissingMemberException("Could not find a matching property accessor for 'Bar' on 'Avalonia.Base.UnitTests.Data.Core.ExpressionObserverTests_Property+WithoutBar'"),
                         BindingErrorType.Error),
                     "baz",
                 },

tests/Avalonia.Benchmarks/Data/AccessorTestObject.cs

@@ -0,0 +1,47 @@
+using System.ComponentModel;
+using System.Runtime.CompilerServices;
+using JetBrains.Annotations;
+
+namespace Avalonia.Benchmarks.Data
+{
+    internal class AccessorTestObject : INotifyPropertyChanged
+    {
+        private string _test;
+
+        public string Test
+        {
+            get => _test;
+            set
+            {
+                if (_test == value)
+                {
+                    return;
+                }
+
+                _test = value;
+
+                OnPropertyChanged();
+            }
+        }
+
+        public event PropertyChangedEventHandler PropertyChanged;
+
+        public void Execute()
+        {
+        }
+
+        public void Execute(object p0)
+        {
+        }
+
+        public void Execute(object p0, object p1)
+        {
+        }
+
+        [NotifyPropertyChangedInvocator]
+        protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
+        {
+            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
+        }
+    }
+}

tests/Avalonia.Benchmarks/Data/PropertyAccessorBenchmarks.cs

@@ -1,9 +1,6 @@
 using System;
-using System.ComponentModel;
-using System.Runtime.CompilerServices;
 using Avalonia.Data.Core.Plugins;
 using BenchmarkDotNet.Attributes;
-using JetBrains.Annotations;
 
 namespace Avalonia.Benchmarks.Data
 {
@@ -12,7 +9,7 @@ namespace Avalonia.Benchmarks.Data
     {
         private readonly InpcPropertyAccessorPlugin _inpcPlugin = new InpcPropertyAccessorPlugin();
         private readonly MethodAccessorPlugin _methodPlugin = new MethodAccessorPlugin();
-        private readonly TestObject _targetStrongRef = new TestObject();
+        private readonly AccessorTestObject _targetStrongRef = new AccessorTestObject();
         private readonly WeakReference<object> _targetWeakRef;
 
         public PropertyAccessorBenchmarks()
@@ -23,66 +20,25 @@ namespace Avalonia.Benchmarks.Data
         [Benchmark]
         public void InpcAccessorMatch()
         {
-            _inpcPlugin.Match(_targetWeakRef, nameof(TestObject.Test));
+            _inpcPlugin.Match(_targetWeakRef, nameof(AccessorTestObject.Test));
         }
 
         [Benchmark]
         public void InpcAccessorStart()
         {
-            _inpcPlugin.Start(_targetWeakRef, nameof(TestObject.Test));
+            _inpcPlugin.Start(_targetWeakRef, nameof(AccessorTestObject.Test));
         }
 
         [Benchmark]
         public void MethodAccessorMatch()
         {
-            _methodPlugin.Match(_targetWeakRef, nameof(TestObject.Execute));
+            _methodPlugin.Match(_targetWeakRef, nameof(AccessorTestObject.Execute));
         }
 
         [Benchmark]
         public void MethodAccessorStart()
         {
-            _methodPlugin.Start(_targetWeakRef, nameof(TestObject.Execute));
+            _methodPlugin.Start(_targetWeakRef, nameof(AccessorTestObject.Execute));
-        }
-
-        private class TestObject : INotifyPropertyChanged
-        {
-            private string _test;
-
-            public string Test
-            {
-                get => _test;
-                set
-                {
-                    if (_test == value)
-                    {
-                        return;
-                    }
-
-                    _test = value;
-
-                    OnPropertyChanged();
-                }
-            }
-
-            public void Execute()
-            {
-            }
-
-            public void Execute(object p0)
-            {
-            }
-
-            public void Execute(object p0, object p1)
-            {
-            }
-
-            public event PropertyChangedEventHandler PropertyChanged;
-
-            [NotifyPropertyChangedInvocator]
-            protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
-            {
-                PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
-            }
         }
     }
 }

tests/Avalonia.Benchmarks/Data/PropertyAccessorPluginBenchmarks.cs

@@ -0,0 +1,60 @@
+using System.Collections.Generic;
+using Avalonia.Data.Core.Plugins;
+using BenchmarkDotNet.Attributes;
+
+namespace Avalonia.Benchmarks.Data
+{
+    [MemoryDiagnoser, InProcess]
+    public class PropertyAccessorPluginBenchmarks
+    {
+        private readonly AccessorTestObject _targetStrongRef = new AccessorTestObject();
+
+        private readonly List<IPropertyAccessorPlugin> _oldPlugins;
+        private readonly List<IPropertyAccessorPlugin> _newPlugins;
+
+        public PropertyAccessorPluginBenchmarks()
+        {
+            _oldPlugins = new List<IPropertyAccessorPlugin>
+            {
+                new AvaloniaPropertyAccessorPlugin(),
+                new MethodAccessorPlugin(),
+                new InpcPropertyAccessorPlugin()
+            };
+
+            _newPlugins = new List<IPropertyAccessorPlugin>
+            {
+                new AvaloniaPropertyAccessorPlugin(),
+                new InpcPropertyAccessorPlugin(),
+                new MethodAccessorPlugin()
+            };
+        }
+
+        [Benchmark]
+        public void MatchAccessorOld()
+        {
+            var propertyName = nameof(AccessorTestObject.Test);
+
+            foreach (IPropertyAccessorPlugin x in _oldPlugins)
+            {
+                if (x.Match(_targetStrongRef, propertyName))
+                {
+                    break;
+                }
+            }
+        }
+
+        [Benchmark]
+        public void MatchAccessorNew()
+        {
+            var propertyName = nameof(AccessorTestObject.Test);
+
+            foreach (IPropertyAccessorPlugin x in _newPlugins)
+            {
+                if (x.Match(_targetStrongRef, propertyName))
+                {
+                    break;
+                }
+            }
+        }
+    }
+}

tests/Avalonia.Markup.UnitTests/Parsers/ExpressionObserverBuilderTests_Property.cs

@@ -22,7 +22,7 @@ namespace Avalonia.Markup.UnitTests.Parsers
 
             Assert.Equal(
                 new BindingNotification(
-                    new MissingMemberException("Could not find CLR property 'Baz' on '1'"), BindingErrorType.Error),
+                    new MissingMemberException("Could not find a matching property accessor for 'Baz' on '1'"), BindingErrorType.Error),
                 result);
 
             GC.KeepAlive(data);