From 4be14715e7a249853ec328989e08e93aac7329e4 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 5 Aug 2020 18:48:44 +0200 Subject: [PATCH 1/9] Added SetProperty overloads with expression/broadcast --- .../ComponentModel/ObservableObject.cs | 21 +++++++- .../ComponentModel/ObservableRecipient.cs | 48 +++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs index aab9e4d409a..1e55461498e 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs @@ -203,7 +203,7 @@ protected bool SetProperty(T oldValue, T newValue, IEqualityComparer compa /// protected bool SetProperty(Expression> propertyExpression, T newValue, [CallerMemberName] string? propertyName = null) { - return SetProperty(propertyExpression, newValue, EqualityComparer.Default, propertyName); + return SetProperty(propertyExpression, newValue, EqualityComparer.Default, out _, propertyName); } /// @@ -220,6 +220,21 @@ protected bool SetProperty(Expression> propertyExpression, T newValue /// (optional) The name of the property that changed. /// if the property was changed, otherwise. protected bool SetProperty(Expression> propertyExpression, T newValue, IEqualityComparer comparer, [CallerMemberName] string? propertyName = null) + { + return SetProperty(propertyExpression, newValue, comparer, out _, propertyName); + } + + /// + /// Implements the shared logic for + /// + /// The type of property to set. + /// An returning the property to update. + /// The property's value after the change occurred. + /// The instance to use to compare the input values. + /// The resulting initial value for the target property. + /// (optional) The name of the property that changed. + /// if the property was changed, otherwise. + private protected bool SetProperty(Expression> propertyExpression, T newValue, IEqualityComparer comparer, out T oldValue, [CallerMemberName] string? propertyName = null) { PropertyInfo? parentPropertyInfo; FieldInfo? parentFieldInfo = null; @@ -236,13 +251,15 @@ protected bool SetProperty(Expression> propertyExpression, T newValue ThrowArgumentExceptionForInvalidPropertyExpression(); // This is never executed, as the method above always throws + oldValue = default!; + return false; } object parent = parentPropertyInfo is null ? parentFieldInfo!.GetValue(instance) : parentPropertyInfo.GetValue(instance); - T oldValue = (T)targetPropertyInfo.GetValue(parent); + oldValue = (T)targetPropertyInfo.GetValue(parent); if (comparer.Equals(oldValue, newValue)) { diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs index d5b58b52a5b..66189f2fb4a 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs @@ -9,6 +9,7 @@ using System; using System.Collections.Generic; +using System.Linq.Expressions; using System.Runtime.CompilerServices; using Microsoft.Toolkit.Mvvm.Messaging; using Microsoft.Toolkit.Mvvm.Messaging.Messages; @@ -230,5 +231,52 @@ protected bool SetProperty(T oldValue, T newValue, IEqualityComparer compa return false; } + + /// + /// Compares the current and new values for a given nested property. If the value has changed, + /// raises the event, updates the property and then raises the + /// event. The behavior mirrors that of + /// , with the difference being that this + /// method is used to relay properties from a wrapped model in the current instance. For more info, see the docs for + /// . + /// + /// The type of property to set. + /// An returning the property to update. + /// The property's value after the change occurred. + /// If , will also be invoked. + /// (optional) The name of the property that changed. + /// if the property was changed, otherwise. + protected bool SetProperty(Expression> propertyExpression, T newValue, bool broadcast, [CallerMemberName] string? propertyName = null) + { + return SetProperty(propertyExpression, newValue, EqualityComparer.Default, broadcast, propertyName); + } + + /// + /// Compares the current and new values for a given nested property. If the value has changed, + /// raises the event, updates the property and then raises the + /// event. The behavior mirrors that of + /// , + /// with the difference being that this method is used to relay properties from a wrapped model in the + /// current instance. For more info, see the docs for + /// . + /// + /// The type of property to set. + /// An returning the property to update. + /// The property's value after the change occurred. + /// The instance to use to compare the input values. + /// If , will also be invoked. + /// (optional) The name of the property that changed. + /// if the property was changed, otherwise. + protected bool SetProperty(Expression> propertyExpression, T newValue, IEqualityComparer comparer, bool broadcast, [CallerMemberName] string? propertyName = null) + { + bool propertyChanged = SetProperty(propertyExpression, newValue, comparer, out T oldValue, propertyName); + + if (propertyChanged && broadcast) + { + Broadcast(oldValue, newValue, propertyName); + } + + return propertyChanged; + } } } \ No newline at end of file From 3fc26f01ae5da251d40d202f6820b241f8734151 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 5 Aug 2020 19:11:45 +0200 Subject: [PATCH 2/9] Minor performance improvement for Set(ref T) overload --- .../ComponentModel/ObservableObject.cs | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs index 1e55461498e..61f5c41f65e 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs @@ -66,7 +66,26 @@ protected virtual void OnPropertyChanging([CallerMemberName] string? propertyNam /// protected bool SetProperty(ref T field, T newValue, [CallerMemberName] string? propertyName = null) { - return SetProperty(ref field, newValue, EqualityComparer.Default, propertyName); + // We duplicate the code here instead of calling the overload because we can't + // guarantee that the invoked SetProperty will be inlined, and we need the JIT + // to be able to see the full EqualityComparer.Default.Equals call, so that + // it'll use the intrinsics version of it and just replace the whole invocation + // with a direct comparison when possible (eg. for primitive numeric types). + // This is the fastest SetProperty overload so we particularly care about + // the codegen quality here, and the code is small and simple enough so that + // duplicating it still doesn't make the whole class harder to maintain. + if (EqualityComparer.Default.Equals(field, newValue)) + { + return false; + } + + OnPropertyChanging(propertyName); + + field = newValue; + + OnPropertyChanged(propertyName); + + return true; } /// From 8daecadac0dfc8b038016d4c883d45ce40962d5e Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 5 Aug 2020 19:16:03 +0200 Subject: [PATCH 3/9] Minor code tweaks --- .../ComponentModel/ObservableRecipient.cs | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs index 66189f2fb4a..ca847383b87 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs @@ -141,7 +141,18 @@ protected virtual void Broadcast(T oldValue, T newValue, string? propertyName /// protected bool SetProperty(ref T field, T newValue, bool broadcast, [CallerMemberName] string? propertyName = null) { - return SetProperty(ref field, newValue, EqualityComparer.Default, broadcast, propertyName); + T oldValue = field; + + // We duplicate the code as in the base class here to leverage + // the intrinsics support for EqualityComparer.Default.Equals. + bool propertyChanged = SetProperty(ref field, newValue, propertyName); + + if (propertyChanged && broadcast) + { + Broadcast(oldValue, newValue, propertyName); + } + + return propertyChanged; } /// @@ -159,21 +170,16 @@ protected bool SetProperty(ref T field, T newValue, bool broadcast, [CallerMe /// if the property was changed, otherwise. protected bool SetProperty(ref T field, T newValue, IEqualityComparer comparer, bool broadcast, [CallerMemberName] string? propertyName = null) { - if (!broadcast) - { - return SetProperty(ref field, newValue, comparer, propertyName); - } - T oldValue = field; - if (SetProperty(ref field, newValue, comparer, propertyName)) + bool propertyChanged = SetProperty(ref field, newValue, comparer, propertyName); + + if (propertyChanged && broadcast) { Broadcast(oldValue, newValue, propertyName); - - return true; } - return false; + return propertyChanged; } /// @@ -217,19 +223,14 @@ protected bool SetProperty(T oldValue, T newValue, Action callback, bool b /// if the property was changed, otherwise. protected bool SetProperty(T oldValue, T newValue, IEqualityComparer comparer, Action callback, bool broadcast, [CallerMemberName] string? propertyName = null) { - if (!broadcast) - { - return SetProperty(oldValue, newValue, comparer, callback, propertyName); - } + bool propertyChanged = SetProperty(oldValue, newValue, comparer, callback, propertyName); - if (SetProperty(oldValue, newValue, comparer, callback, propertyName)) + if (propertyChanged && broadcast) { Broadcast(oldValue, newValue, propertyName); - - return true; } - return false; + return propertyChanged; } /// From 28fdd20235523bf640fba0a28f5a3184b1242cc5 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 6 Aug 2020 13:14:06 +0200 Subject: [PATCH 4/9] Minor (possible) bug fix --- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index f8e817e0d7e..9c27badd255 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -638,9 +638,14 @@ public Type2(Type tMessage, Type tToken) [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool Equals(Type2 other) { + // We can't just use reference equality, as that's technically not guaranteed + // to work and might fail in very rare cases (eg. with type forwarding between + // different assemblies). Instead, we can use the == operator to compare for + // equality, which still avoids the callvirt overhead of calling Type.Equals, + // and is also implemented as a JIT intrinsic on runtimes such as .NET Core. return - ReferenceEquals(this.tMessage, other.tMessage) && - ReferenceEquals(this.tToken, other.tToken); + this.tMessage == other.tMessage && + this.tToken == other.tToken; } /// From a081b30fcc3c32e03bed48911368098f93454d33 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 8 Aug 2020 01:49:11 +0200 Subject: [PATCH 5/9] Added missing volatile field modifier --- Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs b/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs index facc027ae6e..7546d684da8 100644 --- a/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs +++ b/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs @@ -51,7 +51,7 @@ public sealed class Ioc : IServiceProvider /// /// The instance to use, if initialized. /// - private ServiceProvider? serviceProvider; + private volatile ServiceProvider? serviceProvider; /// object? IServiceProvider.GetService(Type serviceType) From 1360f91b6b73496ed23d3c1b89b9ae4378e37b4f Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 8 Aug 2020 17:26:30 +0200 Subject: [PATCH 6/9] Minor optimization to the MessengerExtensions type --- .../Messaging/MessengerExtensions.cs | 51 +++++++++++-------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs b/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs index ea07f57ac85..a344b76a4f3 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs @@ -17,9 +17,37 @@ namespace Microsoft.Toolkit.Mvvm.Messaging public static partial class MessengerExtensions { /// - /// The instance associated with . + /// A class that acts as a container to load the instance linked to + /// the method. + /// This class is needed to avoid forcing the initialization code in the static constructor to run as soon as + /// the type is referenced, even if that is done just to use methods + /// that do not actually require this instance to be available. + /// We're effectively using this type to leverage the lazy loading of static constructors done by the runtime. /// - private static readonly MethodInfo RegisterIRecipientMethodInfo; + private static class MethodInfos + { + /// + /// Initializes static members of the class. + /// + static MethodInfos() + { + RegisterIRecipient = ( + from methodInfo in typeof(MessengerExtensions).GetMethods() + where methodInfo.Name == nameof(Register) && + methodInfo.IsGenericMethod && + methodInfo.GetGenericArguments().Length == 2 + let parameters = methodInfo.GetParameters() + where parameters.Length == 3 && + parameters[1].ParameterType.IsGenericType && + parameters[1].ParameterType.GetGenericTypeDefinition() == typeof(IRecipient<>) + select methodInfo).First(); + } + + /// + /// The instance associated with . + /// + public static readonly MethodInfo RegisterIRecipient; + } /// /// A class that acts as a static container to associate a instance to each @@ -39,23 +67,6 @@ private static class DiscoveredRecipients = new ConditionalWeakTable[]>(); } - /// - /// Initializes static members of the class. - /// - static MessengerExtensions() - { - RegisterIRecipientMethodInfo = ( - from methodInfo in typeof(MessengerExtensions).GetMethods() - where methodInfo.Name == nameof(Register) && - methodInfo.IsGenericMethod && - methodInfo.GetGenericArguments().Length == 2 - let parameters = methodInfo.GetParameters() - where parameters.Length == 3 && - parameters[1].ParameterType.IsGenericType && - parameters[1].ParameterType.GetGenericTypeDefinition() == typeof(IRecipient<>) - select methodInfo).First(); - } - /// /// Checks whether or not a given recipient has already been registered for a message. /// @@ -110,7 +121,7 @@ public static void RegisterAll(this IMessenger messenger, object recipie where interfaceType.IsGenericType && interfaceType.GetGenericTypeDefinition() == typeof(IRecipient<>) let messageType = interfaceType.GenericTypeArguments[0] - let registrationMethod = RegisterIRecipientMethodInfo.MakeGenericMethod(messageType, typeof(TToken)) + let registrationMethod = MethodInfos.RegisterIRecipient.MakeGenericMethod(messageType, typeof(TToken)) let registrationAction = GetRegistrationAction(type, registrationMethod) select registrationAction).ToArray(); } From d0e0cfa4da2c9df0c7e4b8c2bf3f3a7948ccfa98 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 11 Aug 2020 18:38:13 +0200 Subject: [PATCH 7/9] Updated Microsoft.Toolkit.Mvvm .csproj info --- Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj index c76cef22f3e..12b2a288d8a 100644 --- a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj +++ b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj @@ -4,16 +4,16 @@ netstandard2.0;netstandard2.1 8.0 enable - Windows Community Toolkit Mvvm .NET Standard + Windows Community Toolkit MVVM .NET Standard - This package includes Mvvm .NET Standard code only helpers such as: + This package includes MVVM .NET Standard code only helpers such as: - ObservableObject: a base class for objects implementing the INotifyPropertyChanged interface. - ObservableRecipient: a base class for observable objects with support for the IMessenger service. - RelayCommand: a simple delegate command implementing the ICommand interface. - Messenger: a messaging system to exchange messages through different loosely-coupled objects. - Ioc: a helper class to configure dependency injection service containers. - UWP Toolkit Windows Mvvm observable Ioc dependency injection services extensions helpers + UWP Toolkit Windows MVVM MVVMToolkit observable Ioc dependency injection services extensions helpers From 932450c03ae71191e3eec92798cd472df62e32c3 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 11 Aug 2020 18:49:53 +0200 Subject: [PATCH 8/9] Improved package description Co-authored-by: Michael Hawker MSFT (XAML Llama) <24302614+michael-hawker@users.noreply.github.com> --- Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj index 12b2a288d8a..1540806c5d7 100644 --- a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj +++ b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj @@ -6,7 +6,7 @@ enable Windows Community Toolkit MVVM .NET Standard - This package includes MVVM .NET Standard code only helpers such as: + This package includes a .NET Standard MVVM library with helpers such as: - ObservableObject: a base class for objects implementing the INotifyPropertyChanged interface. - ObservableRecipient: a base class for observable objects with support for the IMessenger service. - RelayCommand: a simple delegate command implementing the ICommand interface. From b5c02726c966b833939ff20a73b0aa32523debe5 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 11 Aug 2020 18:55:29 +0200 Subject: [PATCH 9/9] Updated package title Co-authored-by: Michael Hawker MSFT (XAML Llama) <24302614+michael-hawker@users.noreply.github.com> --- Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj index 1540806c5d7..25371e3abc3 100644 --- a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj +++ b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj @@ -4,7 +4,7 @@ netstandard2.0;netstandard2.1 8.0 enable - Windows Community Toolkit MVVM .NET Standard + Windows Community Toolkit MVVM Toolkit This package includes a .NET Standard MVVM library with helpers such as: - ObservableObject: a base class for objects implementing the INotifyPropertyChanged interface.