From e3c64189a4a47d931338285f157a9c31844bcd78 Mon Sep 17 00:00:00 2001 From: Julien Lebosquain Date: Sun, 15 Sep 2024 01:01:57 +0200 Subject: [PATCH] Extract non-generic members from frequently used generic types (#16585) * Extract EffectiveValue notification methods to reduce code size * Extract non-generic members from frequently used generic types --- src/Avalonia.Base/AvaloniaObject.cs | 47 ++++++++++++++----- src/Avalonia.Base/AvaloniaPropertyRegistry.cs | 22 ++++++--- .../PropertyStore/BindingEntryBase.cs | 8 ++-- .../BindingEntryBaseNonGenericHelper.cs | 14 ++++++ .../PropertyStore/EffectiveValue`1.cs | 29 +++++++----- .../PropertyStore/PropertyNotifying.cs | 29 ++++++------ .../Reactive/AnonymousObserver.cs | 4 +- .../AnonymousObserverNonGenericHelper.cs | 13 +++++ src/Avalonia.Base/StyledProperty.cs | 44 ++++++++--------- .../StyledPropertyNonGenericHelper.cs | 30 ++++++++++++ src/Avalonia.Base/Threading/Dispatcher.cs | 2 +- 11 files changed, 165 insertions(+), 77 deletions(-) create mode 100644 src/Avalonia.Base/PropertyStore/BindingEntryBaseNonGenericHelper.cs create mode 100644 src/Avalonia.Base/Reactive/AnonymousObserverNonGenericHelper.cs create mode 100644 src/Avalonia.Base/StyledPropertyNonGenericHelper.cs diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 853d585232..b7ddc87f73 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -458,6 +458,17 @@ namespace Avalonia IObservable source, BindingPriority priority = BindingPriority.LocalValue) { + return TryBindStyledPropertyUntyped(property, source, priority) + ?? _values.AddBinding(property, source, priority); + } + + // Non-generic path extracted to avoid unnecessary generic code duplication + private BindingExpressionBase? TryBindStyledPropertyUntyped( + AvaloniaProperty property, + IObservable source, + BindingPriority priority) + { + Debug.Assert(!property.IsDirect); ThrowHelper.ThrowIfNull(property, nameof(property)); ThrowHelper.ThrowIfNull(source, nameof(source)); VerifyAccess(); @@ -466,18 +477,20 @@ namespace Avalonia if (source is IBinding2 b) { if (b.Instance(this, property, null) is not UntypedBindingExpressionBase expression) - throw new NotSupportedException("Binding returned unsupported IBindingExpression."); + throw new NotSupportedException($"Binding returned unsupported {nameof(BindingExpressionBase)}."); + if (priority != expression.Priority) + { throw new NotSupportedException( $"The binding priority passed to AvaloniaObject.Bind ('{priority}') " + "conflicts with the binding priority of the provided binding expression " + $" ({expression.Priority}')."); + } + return GetValueStore().AddBinding(property, expression); } - else - { - return _values.AddBinding(property, source, priority); - } + + return null; } /// @@ -539,10 +552,22 @@ namespace Avalonia DirectPropertyBase property, IObservable source) { + AvaloniaProperty untypedProperty = property; + + return TryBindDirectPropertyUntyped(ref untypedProperty, source) + ?? _values.AddBinding((DirectPropertyBase)untypedProperty, source); + } + + // Non-generic path extracted to avoid unnecessary generic code duplication + private BindingExpressionBase? TryBindDirectPropertyUntyped( + ref AvaloniaProperty property, + IObservable source) + { + Debug.Assert(property.IsDirect); ThrowHelper.ThrowIfNull(property, nameof(property)); VerifyAccess(); - property = AvaloniaPropertyRegistry.Instance.GetRegisteredDirect(this, property); + property = AvaloniaPropertyRegistry.Instance.GetRegisteredDirectUntyped(this, property); if (property.IsReadOnly) { @@ -552,13 +577,11 @@ namespace Avalonia if (source is IBinding2 b) { if (b.Instance(this, property, null) is not UntypedBindingExpressionBase expression) - throw new NotSupportedException("Binding returned unsupported IBindingExpression."); + throw new NotSupportedException($"Binding returned unsupported {nameof(BindingExpressionBase)}."); return GetValueStore().AddBinding(property, expression); } - else - { - return _values.AddBinding(property, source); - } + + return null; } /// @@ -638,7 +661,7 @@ namespace Avalonia if (binding is not IBinding2 b) throw new NotSupportedException($"Unsupported IBinding implementation '{binding}'."); if (b.Instance(this, property, anchor) is not UntypedBindingExpressionBase expression) - throw new NotSupportedException("Binding returned unsupported IBindingExpression."); + throw new NotSupportedException($"Binding returned unsupported {nameof(BindingExpressionBase)}."); return GetValueStore().AddBinding(property, expression); } diff --git a/src/Avalonia.Base/AvaloniaPropertyRegistry.cs b/src/Avalonia.Base/AvaloniaPropertyRegistry.cs index a772f32bf0..0215d7a10f 100644 --- a/src/Avalonia.Base/AvaloniaPropertyRegistry.cs +++ b/src/Avalonia.Base/AvaloniaPropertyRegistry.cs @@ -1,10 +1,8 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Linq; -using System.Reflection; using System.Runtime.CompilerServices; -using System.Threading; namespace Avalonia { @@ -259,7 +257,12 @@ namespace Avalonia AvaloniaObject o, DirectPropertyBase property) { - return FindRegisteredDirect(o, property) ?? + return (DirectPropertyBase)GetRegisteredDirectUntyped(o, property); + } + + internal AvaloniaProperty GetRegisteredDirectUntyped(AvaloniaObject o, AvaloniaProperty property) + { + return FindRegisteredDirectUntyped(o, property) ?? throw new ArgumentException($"Property '{property.Name} not registered on '{o.GetType()}"); } @@ -331,7 +334,14 @@ namespace Avalonia AvaloniaObject o, DirectPropertyBase property) { - if (property.Owner == o.GetType()) + return (DirectPropertyBase?)FindRegisteredDirectUntyped(o, property); + } + + private AvaloniaProperty? FindRegisteredDirectUntyped(AvaloniaObject o, AvaloniaProperty property) + { + Debug.Assert(property.IsDirect); + + if (property.OwnerType == o.GetType()) { return property; } @@ -345,7 +355,7 @@ namespace Avalonia if (p == property) { - return (DirectPropertyBase)p; + return p; } } diff --git a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs index 294313a15e..bbcab307ac 100644 --- a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs +++ b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs @@ -1,8 +1,8 @@ using System; using System.Collections.Generic; -using Avalonia.Reactive; using Avalonia.Data; using Avalonia.Threading; +using static Avalonia.PropertyStore.BindingEntryBaseNonGenericHelper; namespace Avalonia.PropertyStore { @@ -11,8 +11,6 @@ namespace Avalonia.PropertyStore IObserver>, IDisposable { - private static readonly IDisposable s_creating = Disposable.Empty; - private static readonly IDisposable s_creatingQuiet = Disposable.Create(() => { }); private IDisposable? _subscription; private bool _hasValue; private TValue? _value; @@ -119,7 +117,7 @@ namespace Avalonia.PropertyStore if (_subscription is not null) return; - _subscription = produceValue ? s_creating : s_creatingQuiet; + _subscription = produceValue ? Creating : CreatingQuiet; _subscription = Source switch { IObservable> bv => bv.Subscribe(this), @@ -153,7 +151,7 @@ namespace Avalonia.PropertyStore { instance._value = value.Value; instance._hasValue = true; - if (instance._subscription is not null && instance._subscription != s_creatingQuiet) + if (instance._subscription is not null && instance._subscription != CreatingQuiet) instance.Frame.Owner?.OnBindingValueChanged(instance, instance.Frame.Priority); } } diff --git a/src/Avalonia.Base/PropertyStore/BindingEntryBaseNonGenericHelper.cs b/src/Avalonia.Base/PropertyStore/BindingEntryBaseNonGenericHelper.cs new file mode 100644 index 0000000000..71b529e24c --- /dev/null +++ b/src/Avalonia.Base/PropertyStore/BindingEntryBaseNonGenericHelper.cs @@ -0,0 +1,14 @@ +using System; +using Avalonia.Reactive; + +namespace Avalonia.PropertyStore; + +/// +/// Contains fields for that aren't using generic arguments. +/// Separated to avoid unnecessary generic instantiations. +/// +internal static class BindingEntryBaseNonGenericHelper +{ + public static readonly IDisposable Creating = Disposable.Empty; + public static readonly IDisposable CreatingQuiet = Disposable.Create(() => { }); +} diff --git a/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs b/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs index b50935692e..15ca3f8a8a 100644 --- a/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs +++ b/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs @@ -192,7 +192,7 @@ namespace Avalonia.PropertyStore } protected override object? GetBoxedValue() => Value; - + private static T GetValue(IValueEntry entry) { if (entry is IValueEntry typed) @@ -240,14 +240,11 @@ namespace Avalonia.PropertyStore if (valueChanged) { - using var notifying = PropertyNotifying.Start(owner.Owner, property); - owner.Owner.RaisePropertyChanged(property, oldValue, Value, Priority, true); - if (property.Inherits) - owner.OnInheritedEffectiveValueChanged(property, oldValue, this); + NotifyValueChanged(owner, property, oldValue); } else if (baseValueChanged) { - owner.Owner.RaisePropertyChanged(property, default, _baseValue!, BasePriority, false); + NotifyBaseValueChanged(owner, property); } } @@ -296,19 +293,27 @@ namespace Avalonia.PropertyStore if (valueChanged) { - using var notifying = PropertyNotifying.Start(owner.Owner, property); - owner.Owner.RaisePropertyChanged(property, oldValue, Value, Priority, true); - if (property.Inherits) - owner.OnInheritedEffectiveValueChanged(property, oldValue, this); + NotifyValueChanged(owner, property, oldValue); } if (baseValueChanged) { - owner.Owner.RaisePropertyChanged(property, default, _baseValue!, BasePriority, false); + NotifyBaseValueChanged(owner, property); } } - private class UncommonFields + private void NotifyValueChanged(ValueStore owner, StyledProperty property, T oldValue) + { + using var notifying = PropertyNotifying.Start(owner.Owner, property); + owner.Owner.RaisePropertyChanged(property, oldValue, Value, Priority, true); + if (property.Inherits) + owner.OnInheritedEffectiveValueChanged(property, oldValue, this); + } + + private void NotifyBaseValueChanged(ValueStore owner, StyledProperty property) + => owner.Owner.RaisePropertyChanged(property, default, _baseValue!, BasePriority, false); + + private sealed class UncommonFields { public Func? _coerce; public T? _uncoercedValue; diff --git a/src/Avalonia.Base/PropertyStore/PropertyNotifying.cs b/src/Avalonia.Base/PropertyStore/PropertyNotifying.cs index f508059a74..504692aada 100644 --- a/src/Avalonia.Base/PropertyStore/PropertyNotifying.cs +++ b/src/Avalonia.Base/PropertyStore/PropertyNotifying.cs @@ -1,5 +1,4 @@ using System; -using System.Diagnostics; namespace Avalonia.PropertyStore { @@ -10,26 +9,28 @@ namespace Avalonia.PropertyStore /// Uses the disposable pattern to ensure that the closing Notifying call is made even in the /// presence of exceptions. /// - internal readonly struct PropertyNotifying : IDisposable + internal struct PropertyNotifying : IDisposable { - private readonly AvaloniaObject _owner; - private readonly AvaloniaProperty _property; + private readonly AvaloniaObject? _owner; + private Action? _notifying; - private PropertyNotifying(AvaloniaObject owner, AvaloniaProperty property) + private PropertyNotifying(AvaloniaObject owner, Action? notifying) { - Debug.Assert(property.Notifying is not null); _owner = owner; - _property = property; - _property.Notifying!(owner, true); + _notifying = notifying; + notifying?.Invoke(owner, true); } - public void Dispose() => _property.Notifying!(_owner, false); - - public static PropertyNotifying? Start(AvaloniaObject owner, AvaloniaProperty property) + public void Dispose() { - if (property.Notifying is null) - return null; - return new PropertyNotifying(owner, property); + if (_notifying is null) + return; + + _notifying(_owner!, false); + _notifying = null; } + + public static PropertyNotifying Start(AvaloniaObject owner, AvaloniaProperty property) + => new(owner, property.Notifying); } } diff --git a/src/Avalonia.Base/Reactive/AnonymousObserver.cs b/src/Avalonia.Base/Reactive/AnonymousObserver.cs index 6c458713dc..e3baeff105 100644 --- a/src/Avalonia.Base/Reactive/AnonymousObserver.cs +++ b/src/Avalonia.Base/Reactive/AnonymousObserver.cs @@ -1,6 +1,6 @@ using System; -using System.Runtime.CompilerServices; using System.Threading.Tasks; +using static Avalonia.Reactive.AnonymousObserverNonGenericHelper; namespace Avalonia.Reactive; @@ -10,8 +10,6 @@ namespace Avalonia.Reactive; /// The type of the elements in the sequence. public class AnonymousObserver : IObserver { - private static readonly Action ThrowsOnError = ex => throw ex; - private static readonly Action NoOpCompleted = () => { }; private readonly Action _onNext; private readonly Action _onError; private readonly Action _onCompleted; diff --git a/src/Avalonia.Base/Reactive/AnonymousObserverNonGenericHelper.cs b/src/Avalonia.Base/Reactive/AnonymousObserverNonGenericHelper.cs new file mode 100644 index 0000000000..134c67b28d --- /dev/null +++ b/src/Avalonia.Base/Reactive/AnonymousObserverNonGenericHelper.cs @@ -0,0 +1,13 @@ +using System; + +namespace Avalonia.Reactive; + +/// +/// Contains fields for that aren't using generic arguments. +/// Separated to avoid unnecessary generic instantiations. +/// +internal static class AnonymousObserverNonGenericHelper +{ + public static readonly Action ThrowsOnError = ex => throw ex; + public static readonly Action NoOpCompleted = () => { }; +} diff --git a/src/Avalonia.Base/StyledProperty.cs b/src/Avalonia.Base/StyledProperty.cs index a008ba4092..c498c3130c 100644 --- a/src/Avalonia.Base/StyledProperty.cs +++ b/src/Avalonia.Base/StyledProperty.cs @@ -4,6 +4,7 @@ using System.Runtime.CompilerServices; using Avalonia.Data; using Avalonia.PropertyStore; using Avalonia.Utilities; +using static Avalonia.StyledPropertyNonGenericHelper; namespace Avalonia { @@ -44,9 +45,7 @@ namespace Avalonia if (validate?.Invoke(metadata.DefaultValue) == false) { - throw new ArgumentException( - $"'{metadata.DefaultValue}' is not a valid default value for '{name}'.", - nameof(metadata)); + ThrowInvalidDefaultValue(name, metadata.DefaultValue, name); } _singleDefaultValue = metadata.DefaultValue; @@ -175,8 +174,7 @@ namespace Avalonia { if (!ValidateValue(metadata.DefaultValue)) { - throw new ArgumentException( - $"'{metadata.DefaultValue}' is not a valid default value for '{Name}'."); + ThrowInvalidDefaultValue(Name, metadata.DefaultValue, nameof(metadata)); } } @@ -273,27 +271,25 @@ namespace Avalonia [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = TrimmingMessages.ImplicitTypeConversionSupressWarningMessage)] private bool ShouldSetValue(AvaloniaObject target, object? value, [NotNullWhen(true)] out TValue? converted) { - if (value == BindingOperations.DoNothing) + if (value != BindingOperations.DoNothing) { - converted = default; - return false; - } - if (value == UnsetValue) - { - target.ClearValue(this); - converted = default; - return false; - } - else if (TypeUtilities.TryConvertImplicit(PropertyType, value, out var v)) - { - converted = (TValue)v!; - return true; - } - else - { - var type = value?.GetType().FullName ?? "(null)"; - throw new ArgumentException($"Invalid value for Property '{Name}': '{value}' ({type})"); + if (value == UnsetValue) + { + target.ClearValue(this); + } + else if (TypeUtilities.TryConvertImplicit(PropertyType, value, out var v)) + { + converted = (TValue)v!; + return true; + } + else + { + ThrowInvalidValue(Name, value, nameof(value)); + } } + + converted = default; + return false; } } } diff --git a/src/Avalonia.Base/StyledPropertyNonGenericHelper.cs b/src/Avalonia.Base/StyledPropertyNonGenericHelper.cs new file mode 100644 index 0000000000..944c619bb6 --- /dev/null +++ b/src/Avalonia.Base/StyledPropertyNonGenericHelper.cs @@ -0,0 +1,30 @@ +using System; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; + +namespace Avalonia; + +/// +/// Contains methods for that aren't using generic arguments. +/// Separated to avoid unnecessary generic instantiations. +/// +internal static class StyledPropertyNonGenericHelper +{ + [DoesNotReturn] + [MethodImpl(MethodImplOptions.NoInlining)] + public static void ThrowInvalidValue(string propertyName, object? value, string paramName) + { + var type = value?.GetType().FullName ?? "(null)"; + + throw new ArgumentException( + $"Invalid value for Property '{propertyName}': '{value}' ({type})", + paramName); + } + + public static void ThrowInvalidDefaultValue(string propertyName, object? defaultValue, string paramName) + { + throw new ArgumentException( + $"'{defaultValue}' is not a valid default value for '{propertyName}'.", + paramName); + } +} diff --git a/src/Avalonia.Base/Threading/Dispatcher.cs b/src/Avalonia.Base/Threading/Dispatcher.cs index ec5f7307ef..07e09c3d58 100644 --- a/src/Avalonia.Base/Threading/Dispatcher.cs +++ b/src/Avalonia.Base/Threading/Dispatcher.cs @@ -64,7 +64,7 @@ public partial class Dispatcher : IDispatcher /// /// Checks that the current thread is the UI thread. /// - public bool CheckAccess() => _impl?.CurrentThreadIsLoopThread ?? true; + public bool CheckAccess() => _impl.CurrentThreadIsLoopThread; /// /// Checks that the current thread is the UI thread and throws if not.