From 276a33963143e02174b85d3165a0e66ff7951b45 Mon Sep 17 00:00:00 2001 From: h3xds1nz <gamingshard@protonmail.com> Date: Sat, 29 Mar 2025 20:39:23 +0100 Subject: [PATCH 1/2] Avoid boxing booleans allocations in UncommonField<bool> --- .../System/Windows/UncommonField.cs | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/WindowsBase/System/Windows/UncommonField.cs b/src/Microsoft.DotNet.Wpf/src/WindowsBase/System/Windows/UncommonField.cs index 89757905d9c..c149054da8c 100644 --- a/src/Microsoft.DotNet.Wpf/src/WindowsBase/System/Windows/UncommonField.cs +++ b/src/Microsoft.DotNet.Wpf/src/WindowsBase/System/Windows/UncommonField.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. @@ -44,24 +44,21 @@ public void SetValue(DependencyObject instance, T value) EntryIndex entryIndex = instance.LookupEntry(_globalIndex); - // Set the value if it's not the default, otherwise remove the value. - if (!object.ReferenceEquals(value, _defaultValue)) + // Special case boolean operations to avoid boxing with (mostly) UIA code paths + if (typeof(T) == typeof(bool)) { - object valueObject; - - if (typeof(T) == typeof(bool)) - { - // Use shared boxed instances rather than creating new objects for each SetValue call. - valueObject = BooleanBoxes.Box(Unsafe.As<T, bool>(ref value)); - } - else - { - valueObject = value; - } + // Use shared boxed instances rather than creating new objects for each SetValue call. + object valueObject = BooleanBoxes.Box(Unsafe.BitCast<T, bool>(value)); instance.SetEffectiveValue(entryIndex, dp: null, _globalIndex, metadata: null, valueObject, BaseValueSourceInternal.Local); _hasBeenSet = true; } + // Set the value if it's not the default, otherwise remove the value. + else if (!ReferenceEquals(value, _defaultValue)) + { + instance.SetEffectiveValue(entryIndex, dp: null, _globalIndex, metadata: null, value, BaseValueSourceInternal.Local); + _hasBeenSet = true; + } else { instance.UnsetEffectiveValue(entryIndex, dp: null, metadata: null); From eb11ad5af192b9356259084b1d592387f0a14b53 Mon Sep 17 00:00:00 2001 From: h3xds1nz <gamingshard@protonmail.com> Date: Sun, 30 Mar 2025 21:05:57 +0200 Subject: [PATCH 2/2] Clean up the class, add some comments --- .../System/Windows/UncommonField.cs | 57 +++++++++---------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/WindowsBase/System/Windows/UncommonField.cs b/src/Microsoft.DotNet.Wpf/src/WindowsBase/System/Windows/UncommonField.cs index c149054da8c..d858a0316f6 100644 --- a/src/Microsoft.DotNet.Wpf/src/WindowsBase/System/Windows/UncommonField.cs +++ b/src/Microsoft.DotNet.Wpf/src/WindowsBase/System/Windows/UncommonField.cs @@ -10,31 +10,44 @@ namespace System.Windows internal class UncommonField<T> { /// <summary> - /// Create a new UncommonField. + /// Zero-based, globally unique index, retrieved via <see cref="DependencyProperty.GetUniqueGlobalIndex"/>. /// </summary> - public UncommonField() : this(default(T)) + internal int GlobalIndex { get; } + + /// <summary> + /// Determines the default value for this field, assigned during initial construction. + /// </summary> + private readonly T _defaultValue; + /// <summary> + /// An optimization detail representing whether a non-default value is currently stored. + /// </summary> + private bool _hasBeenSet; + + /// <summary> + /// Create a new UncommonField. Stores <see langword="default"/>(<typeparamref name="T"/>) value as default. + /// </summary> + public UncommonField() : this(default) { } /// <summary> - /// Create a new UncommonField. + /// Create a new UncommonField. /// </summary> /// <param name="defaultValue">The default value of the field.</param> public UncommonField(T defaultValue) { _defaultValue = defaultValue; - _hasBeenSet = false; lock (DependencyProperty.Synchronized) { - _globalIndex = DependencyProperty.GetUniqueGlobalIndex(null, null); + GlobalIndex = DependencyProperty.GetUniqueGlobalIndex(null, null); DependencyProperty.RegisteredPropertyList.Add(); } } /// <summary> - /// Write the given value onto a DependencyObject instance. + /// Write the given value onto a DependencyObject instance. /// </summary> /// <param name="instance">The DependencyObject on which to set the value.</param> /// <param name="value">The value to set.</param> @@ -42,7 +55,7 @@ public void SetValue(DependencyObject instance, T value) { ArgumentNullException.ThrowIfNull(instance); - EntryIndex entryIndex = instance.LookupEntry(_globalIndex); + EntryIndex entryIndex = instance.LookupEntry(GlobalIndex); // Special case boolean operations to avoid boxing with (mostly) UIA code paths if (typeof(T) == typeof(bool)) @@ -50,13 +63,13 @@ public void SetValue(DependencyObject instance, T value) // Use shared boxed instances rather than creating new objects for each SetValue call. object valueObject = BooleanBoxes.Box(Unsafe.BitCast<T, bool>(value)); - instance.SetEffectiveValue(entryIndex, dp: null, _globalIndex, metadata: null, valueObject, BaseValueSourceInternal.Local); + instance.SetEffectiveValue(entryIndex, dp: null, GlobalIndex, metadata: null, valueObject, BaseValueSourceInternal.Local); _hasBeenSet = true; } // Set the value if it's not the default, otherwise remove the value. else if (!ReferenceEquals(value, _defaultValue)) { - instance.SetEffectiveValue(entryIndex, dp: null, _globalIndex, metadata: null, value, BaseValueSourceInternal.Local); + instance.SetEffectiveValue(entryIndex, dp: null, GlobalIndex, metadata: null, value, BaseValueSourceInternal.Local); _hasBeenSet = true; } else @@ -66,7 +79,7 @@ public void SetValue(DependencyObject instance, T value) } /// <summary> - /// Read the value of this field on a DependencyObject instance. + /// Read the value of this field on a DependencyObject instance. /// </summary> /// <param name="instance">The DependencyObject from which to get the value.</param> /// <returns></returns> @@ -76,7 +89,7 @@ public T GetValue(DependencyObject instance) if (_hasBeenSet) { - EntryIndex entryIndex = instance.LookupEntry(_globalIndex); + EntryIndex entryIndex = instance.LookupEntry(GlobalIndex); if (entryIndex.Found) { @@ -97,32 +110,16 @@ public T GetValue(DependencyObject instance) /// <summary> - /// Clear this field from the given DependencyObject instance. + /// Clear this field from the given DependencyObject instance. /// </summary> - /// <param name="instance"></param> + /// <param name="instance">An instance on which to clear the value from.</param> public void ClearValue(DependencyObject instance) { ArgumentNullException.ThrowIfNull(instance); - EntryIndex entryIndex = instance.LookupEntry(_globalIndex); + EntryIndex entryIndex = instance.LookupEntry(GlobalIndex); instance.UnsetEffectiveValue(entryIndex, dp: null, metadata: null); } - - internal int GlobalIndex - { - get - { - return _globalIndex; - } - } - - #region Private Fields - - private T _defaultValue; - private int _globalIndex; - private bool _hasBeenSet; - - #endregion } }