Change BitHelper in System.Collections to be a ref struct
authorStephen Toub <stoub@microsoft.com>
Sun, 6 Jan 2019 15:25:16 +0000 (10:25 -0500)
committerStephen Toub <stoub@microsoft.com>
Tue, 8 Jan 2019 14:49:57 +0000 (09:49 -0500)
Simplifies BitHelper, basing it on span instead of having to accomodate both pointers and arrays, and avoids allocating the BitHelper instance.  We now also clear the input optionally.

Commit migrated from https://github.com/dotnet/corefx/commit/82ce02db86987eb8947b9f09b76cd0658aaf2dce

src/libraries/System.Collections/src/System/Collections/Generic/BitHelper.cs
src/libraries/System.Collections/src/System/Collections/Generic/HashSet.cs
src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.cs

index 6025b13..e36f6a2 100644 (file)
 
 namespace System.Collections.Generic
 {
-    /// <summary>
-    /// ABOUT:
-    /// Helps with operations that rely on bit marking to indicate whether an item in the 
-    /// collection should be added, removed, visited already, etc. 
-    /// 
-    /// BitHelper doesn't allocate the array; you must pass in an array or ints allocated on the 
-    /// stack or heap. ToIntArrayLength() tells you the int array size you must allocate. 
-    /// 
-    /// USAGE:
-    /// Suppose you need to represent a bit array of length (i.e. logical bit array length)
-    /// BIT_ARRAY_LENGTH. Then this is the suggested way to instantiate BitHelper:
-    /// ***************************************************************************
-    /// int intArrayLength = BitHelper.ToIntArrayLength(BIT_ARRAY_LENGTH);
-    /// BitHelper bitHelper;
-    /// if (intArrayLength less than stack alloc threshold)
-    ///     int* m_arrayPtr = stackalloc int[intArrayLength];
-    ///     bitHelper = new BitHelper(m_arrayPtr, intArrayLength);
-    /// else
-    ///     int[] m_arrayPtr = new int[intArrayLength];
-    ///     bitHelper = new BitHelper(m_arrayPtr, intArrayLength);
-    /// ***************************************************************************
-    /// 
-    /// IMPORTANT:
-    /// The second ctor args, length, should be specified as the length of the int array, not
-    /// the logical bit array. Because length is used for bounds checking into the int array,
-    /// it's especially important to get this correct for the stackalloc version. See the code 
-    /// samples above; this is the value gotten from ToIntArrayLength(). 
-    /// 
-    /// The length ctor argument is the only exception; for other methods -- MarkBit and 
-    /// IsMarked -- pass in values as indices into the logical bit array, and it will be mapped
-    /// to the position within the array of ints.
-    /// 
-    /// FUTURE OPTIMIZATIONS:
-    /// A method such as FindFirstMarked/Unmarked Bit would be useful for callers that operate 
-    /// on a bit array and then need to loop over it. In particular, if it avoided visiting 
-    /// every bit, it would allow good perf improvements when the bit array is sparse.
-    /// </summary>
-    internal unsafe sealed class BitHelper
-    {   // should not be serialized
-        private const byte MarkedBitFlag = 1;
-        private const byte IntSize = 32;
+    internal ref struct BitHelper
+    {
+        private const int IntSize = sizeof(int) * 8;
+        private readonly Span<int> _span;
 
-        // m_length of underlying int array (not logical bit array)
-        private readonly int _length;
-
-        // ptr to stack alloc'd array of ints
-        private readonly int* _arrayPtr;
-
-        // array of ints
-        private readonly int[] _array;
-
-        // whether to operate on stack alloc'd or heap alloc'd array 
-        private readonly bool _useStackAlloc;
-
-        /// <summary>
-        /// Instantiates a BitHelper with a heap alloc'd array of ints
-        /// </summary>
-        /// <param name="bitArrayPtr">Pointer to int array to hold bits</param>
-        /// <param name="length">length of int array</param>
-        internal BitHelper(int* bitArrayPtr, int length)
-        {
-            _arrayPtr = bitArrayPtr;
-            _length = length;
-            _useStackAlloc = true;
-        }
-
-        /// <summary>
-        /// Instantiates a BitHelper with a heap alloc'd array of ints
-        /// </summary>
-        /// <param name="bitArray">int array to hold bits</param>
-        /// <param name="length">length of int array</param>
-        internal BitHelper(int[] bitArray, int length)
+        internal BitHelper(Span<int> span, bool clear)
         {
-            _array = bitArray;
-            _length = length;
+            if (clear)
+            {
+                span.Clear();
+            }
+            _span = span;
         }
 
-        /// <summary>
-        /// Mark bit at specified position
-        /// </summary>
-        /// <param name="bitPosition"></param>
         internal void MarkBit(int bitPosition)
         {
             int bitArrayIndex = bitPosition / IntSize;
-            if (bitArrayIndex < _length && bitArrayIndex >= 0)
+            if ((uint)bitArrayIndex < (uint)_span.Length)
             {
-                int flag = (MarkedBitFlag << (bitPosition % IntSize));
-                if (_useStackAlloc)
-                {
-                    _arrayPtr[bitArrayIndex] |= flag;
-                }
-                else
-                {
-                    _array[bitArrayIndex] |= flag;
-                }
+                _span[bitArrayIndex] |= (1 << (bitPosition % IntSize));
             }
         }
 
-        /// <summary>
-        /// Is bit at specified position marked?
-        /// </summary>
-        /// <param name="bitPosition"></param>
-        /// <returns></returns>
         internal bool IsMarked(int bitPosition)
         {
             int bitArrayIndex = bitPosition / IntSize;
-            if (bitArrayIndex < _length && bitArrayIndex >= 0)
-            {
-                int flag = (MarkedBitFlag << (bitPosition % IntSize));
-                if (_useStackAlloc)
-                {
-                    return ((_arrayPtr[bitArrayIndex] & flag) != 0);
-                }
-                else
-                {
-                    return ((_array[bitArrayIndex] & flag) != 0);
-                }
-            }
-            return false;
+            return 
+                (uint)bitArrayIndex < (uint)_span.Length &&
+                (_span[bitArrayIndex] & (1 << (bitPosition % IntSize))) != 0;
         }
 
-        /// <summary>
-        /// How many ints must be allocated to represent n bits. Returns (n+31)/32, but 
-        /// avoids overflow
-        /// </summary>
-        /// <param name="n"></param>
-        /// <returns></returns>
-        internal static int ToIntArrayLength(int n)
-        {
-            return n > 0 ? ((n - 1) / IntSize + 1) : 0;
-        }
+        /// <summary>How many ints must be allocated to represent n bits. Returns (n+31)/32, but avoids overflow.</summary>
+        internal static int ToIntArrayLength(int n) => n > 0 ? ((n - 1) / IntSize + 1) : 0;
     }
 }
index 0734424..3bba0a9 100644 (file)
@@ -1360,17 +1360,10 @@ namespace System.Collections.Generic
             int originalLastIndex = _lastIndex;
             int intArrayLength = BitHelper.ToIntArrayLength(originalLastIndex);
 
-            BitHelper bitHelper;
-            if (intArrayLength <= StackAllocThreshold)
-            {
-                int* bitArrayPtr = stackalloc int[intArrayLength];
-                bitHelper = new BitHelper(bitArrayPtr, intArrayLength);
-            }
-            else
-            {
-                int[] bitArray = new int[intArrayLength];
-                bitHelper = new BitHelper(bitArray, intArrayLength);
-            }
+            Span<int> span = stackalloc int[StackAllocThreshold];
+            BitHelper bitHelper = intArrayLength <= StackAllocThreshold ?
+                new BitHelper(span.Slice(0, intArrayLength), clear: true) :
+                new BitHelper(new int[intArrayLength], clear: false);
 
             // mark if contains: find index of in slots array and mark corresponding element in bit array
             foreach (T item in other)
@@ -1465,24 +1458,15 @@ namespace System.Collections.Generic
             int originalLastIndex = _lastIndex;
             int intArrayLength = BitHelper.ToIntArrayLength(originalLastIndex);
 
-            BitHelper itemsToRemove;
-            BitHelper itemsAddedFromOther;
-            if (intArrayLength <= StackAllocThreshold / 2)
-            {
-                int* itemsToRemovePtr = stackalloc int[intArrayLength];
-                itemsToRemove = new BitHelper(itemsToRemovePtr, intArrayLength);
-
-                int* itemsAddedFromOtherPtr = stackalloc int[intArrayLength];
-                itemsAddedFromOther = new BitHelper(itemsAddedFromOtherPtr, intArrayLength);
-            }
-            else
-            {
-                int[] itemsToRemoveArray = new int[intArrayLength];
-                itemsToRemove = new BitHelper(itemsToRemoveArray, intArrayLength);
+            Span<int> itemsToRemoveSpan = stackalloc int[StackAllocThreshold / 2];
+            BitHelper itemsToRemove = intArrayLength <= StackAllocThreshold / 2 ?
+                new BitHelper(itemsToRemoveSpan.Slice(0, intArrayLength), clear: true) :
+                new BitHelper(new int[intArrayLength], clear: false);
 
-                int[] itemsAddedFromOtherArray = new int[intArrayLength];
-                itemsAddedFromOther = new BitHelper(itemsAddedFromOtherArray, intArrayLength);
-            }
+            Span<int> itemsAddedFromOtherSpan = stackalloc int[StackAllocThreshold / 2];
+            BitHelper itemsAddedFromOther = intArrayLength <= StackAllocThreshold / 2 ?
+                new BitHelper(itemsAddedFromOtherSpan.Slice(0, intArrayLength), clear: true) :
+                new BitHelper(new int[intArrayLength], clear: false);
 
             foreach (T item in other)
             {
@@ -1629,17 +1613,10 @@ namespace System.Collections.Generic
             int originalLastIndex = _lastIndex;
             int intArrayLength = BitHelper.ToIntArrayLength(originalLastIndex);
 
-            BitHelper bitHelper;
-            if (intArrayLength <= StackAllocThreshold)
-            {
-                int* bitArrayPtr = stackalloc int[intArrayLength];
-                bitHelper = new BitHelper(bitArrayPtr, intArrayLength);
-            }
-            else
-            {
-                int[] bitArray = new int[intArrayLength];
-                bitHelper = new BitHelper(bitArray, intArrayLength);
-            }
+            Span<int> span = stackalloc int[StackAllocThreshold];
+            BitHelper bitHelper = intArrayLength <= StackAllocThreshold ?
+                new BitHelper(span.Slice(0, intArrayLength), clear: true) :
+                new BitHelper(new int[intArrayLength], clear: false);
 
             // count of items in other not found in this
             int unfoundCount = 0;
index 0d8a54a..a731422 100644 (file)
@@ -1449,17 +1449,10 @@ namespace System.Collections.Generic
             int originalLastIndex = Count;
             int intArrayLength = BitHelper.ToIntArrayLength(originalLastIndex);
 
-            BitHelper bitHelper;
-            if (intArrayLength <= StackAllocThreshold)
-            {
-                int* bitArrayPtr = stackalloc int[intArrayLength];
-                bitHelper = new BitHelper(bitArrayPtr, intArrayLength);
-            }
-            else
-            {
-                int[] bitArray = new int[intArrayLength];
-                bitHelper = new BitHelper(bitArray, intArrayLength);
-            }
+            Span<int> span = stackalloc int[StackAllocThreshold];
+            BitHelper bitHelper = intArrayLength <= StackAllocThreshold ?
+                new BitHelper(span.Slice(0, intArrayLength), clear: true) :
+                new BitHelper(new int[intArrayLength], clear: false);
 
             // count of items in other not found in this
             int UnfoundCount = 0;