Switches object pool IDs from GUIDs to Int32.

This cuts out the expensive Guid.op_Equality step from ThrowDisposedIfNotOwned.
hutchinson · Nov 11, 2014 · 7f84859 · 7f84859
1 parent ab1e10f
commit 7f84859
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 14 deletions.
diff --git a/src/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList`1.cs b/src/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList`1.cs
@@ -1505,7 +1505,7 @@ public struct Enumerator : IEnumerator<T>, ISecurePooledObjectUser
             /// A unique ID for this instance of this enumerator.
             /// Used to protect pooled objects from use after they are recycled.
             /// </summary>
-            private readonly Guid poolUserId;
+            private readonly int poolUserId;
 
             /// <summary>
             /// The starting index of the collection at which to begin enumeration.
@@ -1571,7 +1571,7 @@ internal Enumerator(IBinaryTree<T> root, Builder builder = null, int startIndex
                 this.remainingCount = this.count;
                 this.reversed = reversed;
                 this.enumeratingBuilderVersion = builder != null ? builder.Version : -1;
-                this.poolUserId = Guid.NewGuid();
+                this.poolUserId = SecureObjectPool.NewId();
                 this.stack = null;
                 if (this.count > 0)
                 {
@@ -1585,7 +1585,7 @@ internal Enumerator(IBinaryTree<T> root, Builder builder = null, int startIndex
             }
 
             /// <inheritdoc/>
-            Guid ISecurePooledObjectUser.PoolUserId
+            int ISecurePooledObjectUser.PoolUserId
             {
                 get { return this.poolUserId; }
             }

diff --git a/...tem.Collections.Immutable/src/System/Collections/Immutable/ImmutableSortedDictionary`2.cs b/...tem.Collections.Immutable/src/System/Collections/Immutable/ImmutableSortedDictionary`2.cs
@@ -957,7 +957,7 @@ public struct Enumerator : IEnumerator<KeyValuePair<TKey, TValue>>, ISecurePoole
             /// A unique ID for this instance of this enumerator.
             /// Used to protect pooled objects from use after they are recycled.
             /// </summary>
-            private readonly Guid poolUserId;
+            private readonly int poolUserId;
 
             /// <summary>
             /// The set being enumerated.
@@ -992,7 +992,7 @@ internal Enumerator(IBinaryTree<KeyValuePair<TKey, TValue>> root, Builder builde
                 this.builder = builder;
                 this.current = null;
                 this.enumeratingBuilderVersion = builder != null ? builder.Version : -1;
-                this.poolUserId = Guid.NewGuid();
+                this.poolUserId = SecureObjectPool.NewId();
                 this.stack = null;
                 if (!this.root.IsEmpty)
                 {
@@ -1023,7 +1023,7 @@ public KeyValuePair<TKey, TValue> Current
             }
 
             /// <inheritdoc/>
-            Guid ISecurePooledObjectUser.PoolUserId
+            int ISecurePooledObjectUser.PoolUserId
             {
                 get { return this.poolUserId; }
             }

diff --git a/src/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableSortedSet`1.cs b/src/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableSortedSet`1.cs
@@ -1131,7 +1131,7 @@ public struct Enumerator : IEnumerator<T>, ISecurePooledObjectUser
             /// A unique ID for this instance of this enumerator.
             /// Used to protect pooled objects from use after they are recycled.
             /// </summary>
-            private readonly Guid poolUserId;
+            private readonly int poolUserId;
 
             /// <summary>
             /// A flag indicating whether this enumerator works in reverse sort order.
@@ -1181,7 +1181,7 @@ internal Enumerator(IBinaryTree<T> root, Builder builder = null, bool reverse =
                 this.current = null;
                 this.reverse = reverse;
                 this.enumeratingBuilderVersion = builder != null ? builder.Version : -1;
-                this.poolUserId = Guid.NewGuid();
+                this.poolUserId = SecureObjectPool.NewId();
                 this.stack = null;
                 if (!enumeratingStacks.TryTake(this, out this.stack))
                 {
@@ -1192,7 +1192,7 @@ internal Enumerator(IBinaryTree<T> root, Builder builder = null, bool reverse =
             }
 
             /// <inheritdoc/>
-            Guid ISecurePooledObjectUser.PoolUserId
+            int ISecurePooledObjectUser.PoolUserId
             {
                 get { return this.poolUserId; }
             }

diff --git a/src/System.Collections.Immutable/src/System/Collections/Immutable/SecureObjectPool.cs b/src/System.Collections.Immutable/src/System/Collections/Immutable/SecureObjectPool.cs
@@ -11,6 +11,36 @@
 
 namespace System.Collections.Immutable
 {
+    /// <summary>
+    /// Object pooling utilities.
+    /// </summary>
+    internal class SecureObjectPool
+    {
+        /// <summary>
+        /// The ever-incrementing (and wrap-on-overflow) integer for owner id's.
+        /// </summary>
+        private static int poolUserIdCounter;
+
+        /// <summary>
+        /// The ID reserved for unassigned objects.
+        /// </summary>
+        internal const int UnassignedId = -1;
+
+        /// <summary>
+        /// Returns a new ID.
+        /// </summary>
+        internal static int NewId()
+        {
+            int result = Interlocked.Increment(ref poolUserIdCounter);
+            if (result == UnassignedId)
+            {
+                result = Interlocked.Increment(ref poolUserIdCounter);
+            }
+
+            return result;
+        }
+    }
+
     internal class SecureObjectPool<T, TCaller>
         where TCaller : ISecurePooledObjectUser
     {
@@ -23,15 +53,15 @@ public void TryAdd(TCaller caller, SecurePooledObject<T> item)
                 // Only allow the caller to recycle this object if it is the current owner.
                 if (caller.PoolUserId == item.Owner)
                 {
-                    item.Owner = Guid.Empty;
+                    item.Owner = SecureObjectPool.UnassignedId;
                     this.pool.TryAdd(item);
                 }
             }
         }
 
         public bool TryTake(TCaller caller, out SecurePooledObject<T> item)
         {
-            if (caller.PoolUserId != Guid.Empty && this.pool.TryTake(out item))
+            if (caller.PoolUserId != SecureObjectPool.UnassignedId && this.pool.TryTake(out item))
             {
                 item.Owner = caller.PoolUserId;
                 return true;
@@ -54,21 +84,21 @@ public SecurePooledObject<T> PrepNew(TCaller caller, T newValue)
 
     internal interface ISecurePooledObjectUser
     {
-        Guid PoolUserId { get; }
+        int PoolUserId { get; }
     }
 
     internal class SecurePooledObject<T>
     {
         private readonly T value;
-        private Guid owner;
+        private int owner;
 
         internal SecurePooledObject(T newValue)
         {
             Requires.NotNullAllowStructs(newValue, "newValue");
             this.value = newValue;
         }
 
-        internal Guid Owner
+        internal int Owner
         {
             get
             {