Remove locks on Style/TriggerBase counters, use atomic increments instead

src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Style.cs

@@ -50,7 +50,8 @@ static Style()
         /// </summary>
         public Style()
         {
-            GetUniqueGlobalIndex();
+            // Get globally unique ID
+            GlobalIndex = Interlocked.Increment(ref s_globalStyleIndex);
         }
 
         /// <summary>
@@ -61,7 +62,8 @@ public Style(Type targetType)
         {
             TargetType = targetType;
 
-            GetUniqueGlobalIndex();
+            // Get globally unique ID
+            GlobalIndex = Interlocked.Increment(ref s_globalStyleIndex);
         }
 
         /// <summary>
@@ -74,7 +76,8 @@ public Style(Type targetType, Style basedOn)
             TargetType = targetType;
             BasedOn = basedOn;
 
-            GetUniqueGlobalIndex();
+            // Get globally unique ID
+            GlobalIndex = Interlocked.Increment(ref s_globalStyleIndex);
         }
 
         #region INameScope
@@ -118,19 +121,6 @@ object INameScope.FindName(string name)
         private NameScope _nameScope = new NameScope();
         #endregion IIdScope
 
-        /// <summary>
-        /// Each Style gets its own unique index used for Style.GetHashCode
-        /// </summary>
-        private void GetUniqueGlobalIndex()
-        {
-            lock (Synchronized)
-            {
-                // Setup unqiue global index
-                StyleInstanceCount++;
-                GlobalIndex = StyleInstanceCount;
-            }
-        }
-
         /// <summary>
         ///     Style mutability state
         /// </summary>
@@ -944,14 +934,16 @@ internal bool HasLoadedChangeHandler
             set { _hasLoadedChangeHandler = value; }
         }
 
-        // Special equality check that takes into account 'null'
-        private static bool IsEqual(object a, object b)
+        /// <summary>
+        /// Each Style gets its own unique index used for <see cref="GetHashCode"/>.
+        /// </summary>
+        internal int GlobalIndex { get; }
+
+        internal bool IsBasedOnModified
         {
-            return (a != null) ? a.Equals(b) : (b == null);
+            get => IsModified(BasedOnID);
         }
 
-        internal bool IsBasedOnModified { get { return IsModified(BasedOnID); } }
-
         private EventHandlersStore _eventHandlersStore = null;
 
         private bool _sealed;
@@ -972,8 +964,6 @@ private static bool IsEqual(object a, object b)
         // of this style and its sub-tree.
         internal ResourceDictionary _resources = null;
 
-        /* property */ internal int GlobalIndex;
-
         // Style tables
         // Synchronized (write locks, lock-free reads): Covered by Style instance lock
         /* property */ internal FrugalStructList<ChildRecord> ChildRecordFromChildIndex = new FrugalStructList<ChildRecord>(); // Indexed by Child.ChildIndex
@@ -1022,12 +1012,13 @@ private static bool IsEqual(object a, object b)
         //   A DataTrigger can have Setters but no EnterAction/ExitAction.  (The reverse can also be true.)
         internal HybridDictionary DataTriggersWithActions = null;
 
-        // Unique index for every instance of Style
-        // Synchronized: Covered by Style.Synchronized
-        private static int StyleInstanceCount = 0;
-
-        // Global, cross-object synchronization
-        internal static object Synchronized = new object();
+        /// <summary>
+        /// Global indexer for <see cref="Style"/> and its derivates.
+        /// </summary>
+        /// <remarks>
+        /// Access must be done atomically, currently only written via constructors.
+        /// </remarks>
+        private static int s_globalStyleIndex = 0;
 
         private const int TargetTypeID = 0x01;
         internal const int BasedOnID    = 0x02;

src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/TriggerBase.cs

@@ -2,16 +2,16 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System.Collections.Specialized;
-using System.IO;
-using System.Windows.Markup;
+using StoryboardLayers = System.Windows.Media.Animation.Storyboard.Layers;
 
-using MS.Utility;
-using MS.Internal;
+using System.Collections.Specialized;
 
-using System;
 using System.ComponentModel;            // DesignerSerializationVisibilityAttribute & DefaultValue
 using System.Diagnostics;
+using System.Threading;
+
+using MS.Utility;
+using MS.Internal;
 
 namespace System.Windows
 {
@@ -348,25 +348,20 @@ internal override bool HasMultipleInheritanceContexts
         // This ranking is used when trigger needs to be sorted relative to
         //  the ordering, as when determining precedence for enter/exit
         //  animation composition.  Otherwise, it stays at default value of zero.
-        internal Int64 Layer
+        internal long Layer
         {
-            get { return _globalLayerRank; }
+            get => _globalLayerRank;
         }
 
         // Set self rank to current number, increment global static.
         internal void EstablishLayer()
         {
-            if( _globalLayerRank == 0 )
+            if (_globalLayerRank == 0)
             {
-                lock(Synchronized)
-                {
-                    _globalLayerRank = _nextGlobalLayerRank++;
-                }
+                _globalLayerRank = Interlocked.Increment(ref s_nextGlobalLayerRank);
 
-                if( _nextGlobalLayerRank == Int64.MaxValue )
-                {
+                if (s_nextGlobalLayerRank == long.MaxValue)
                     throw new InvalidOperationException(SR.PropertyTriggerLayerLimitExceeded);
-                }
             }
         }
 
@@ -389,9 +384,6 @@ internal TriggerCondition[] TriggerConditions
         // Synchronized (write locks, lock-free reads): Covered by the TriggerBase instance
         /* property */ internal FrugalStructList<System.Windows.PropertyValue> PropertyValues = new FrugalStructList<System.Windows.PropertyValue>();
 
-        // Global, cross-object synchronization
-        private static readonly object Synchronized = new object();
-
         // Conditions
         TriggerCondition[] _triggerConditions;
 
@@ -404,10 +396,20 @@ internal TriggerCondition[] TriggerConditions
         private TriggerActionCollection _exitActions = null;
 
 //      On hold - is this a new public API we want to do?
-//        private bool _executeEnterActionsOnApply = false;
-//        private bool _executeExitActionsOnApply = false;
+//      private bool _executeEnterActionsOnApply = false;
+//      private bool _executeExitActionsOnApply = false;
+
+        /// <summary>
+        /// Global index of this particular instance.
+        /// </summary>
+        private long _globalLayerRank = 0;
 
-        private Int64 _globalLayerRank = 0;
-        private static Int64 _nextGlobalLayerRank = System.Windows.Media.Animation.Storyboard.Layers.PropertyTriggerStartLayer;
+        /// <summary>
+        /// Global indexer for <see cref="TriggerBase"/> and its derivates.
+        /// </summary>
+        /// <remarks>
+        /// Access must be done atomically, currently only written via <see cref="EstablishLayer"/>.
+        /// </remarks>
+        private static long s_nextGlobalLayerRank = StoryboardLayers.PropertyTriggerStartLayer; // 2
     }
 }