Apply feedback to grow methods of generic collections (#49167)

* Apply feedback to Grow methods of generic collections

* add assertion

diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/Queue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/Queue.cs
index f467628..c8c01ab 100644 (file)
--- a/src/libraries/System.Collections/src/System/Collections/Generic/Queue.cs
+++ b/src/libraries/System.Collections/src/System/Collections/Generic/Queue.cs
@@ -180,7 +180,7 @@ namespace System.Collections.Generic
         {
             if (_size == _array.Length)
             {
-                EnsureCapacityCore(_size + 1);
+                Grow(_size + 1);
             }
 
             _array[_tail] = item;
@@ -390,38 +390,35 @@ namespace System.Collections.Generic
 
             if (_array.Length < capacity)
             {
-                EnsureCapacityCore(capacity);
+                Grow(capacity);
             }
 
             return _array.Length;
         }
 
-        private void EnsureCapacityCore(int capacity)
+        private void Grow(int capacity)
         {
-            Debug.Assert(capacity >= 0);
+            Debug.Assert(_array.Length < capacity);
 
-            if (_array.Length < capacity)
-            {
-                // Array.MaxArrayLength is internal to S.P.CoreLib, replicate here.
-                const int MaxArrayLength = 0X7FEFFFFF;
-                const int GrowFactor = 2;
-                const int MinimumGrow = 4;
+            // Array.MaxArrayLength is internal to S.P.CoreLib, replicate here.
+            const int MaxArrayLength = 0X7FEFFFFF;
+            const int GrowFactor = 2;
+            const int MinimumGrow = 4;
 
-                int newcapacity = GrowFactor * _array.Length;
+            int newcapacity = GrowFactor * _array.Length;
 
-                // Allow the list to grow to maximum possible capacity (~2G elements) before encountering overflow.
-                // Note that this check works even when _items.Length overflowed thanks to the (uint) cast
-                if ((uint)newcapacity > MaxArrayLength) newcapacity = MaxArrayLength;
+            // Allow the list to grow to maximum possible capacity (~2G elements) before encountering overflow.
+            // Note that this check works even when _items.Length overflowed thanks to the (uint) cast
+            if ((uint)newcapacity > MaxArrayLength) newcapacity = MaxArrayLength;
 
-                // Ensure minimum growth is respected.
-                newcapacity = Math.Max(newcapacity, _array.Length + MinimumGrow);
+            // Ensure minimum growth is respected.
+            newcapacity = Math.Max(newcapacity, _array.Length + MinimumGrow);
 
-                // If the computed capacity is still less than specified, set to the original argument.
-                // Capacities exceeding MaxArrayLength will be surfaced as OutOfMemoryException by Array.Resize.
-                if (newcapacity < capacity) newcapacity = capacity;
+            // If the computed capacity is still less than specified, set to the original argument.
+            // Capacities exceeding MaxArrayLength will be surfaced as OutOfMemoryException by Array.Resize.
+            if (newcapacity < capacity) newcapacity = capacity;
 
-                SetCapacity(newcapacity);
-            }
+            SetCapacity(newcapacity);
         }
 
         // Implements an enumerator for a Queue.  The enumerator uses the

diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/Stack.cs b/src/libraries/System.Collections/src/System/Collections/Generic/Stack.cs
index 8c7889c..b2ba475 100644 (file)
--- a/src/libraries/System.Collections/src/System/Collections/Generic/Stack.cs
+++ b/src/libraries/System.Collections/src/System/Collections/Generic/Stack.cs
@@ -283,7 +283,7 @@ namespace System.Collections.Generic
         private void PushWithResize(T item)
         {
             Debug.Assert(_size == _array.Length);
-            EnsureCapacityCore(_size + 1);
+            Grow(_size + 1);
             _array[_size] = item;
             _version++;
             _size++;
@@ -304,34 +304,31 @@ namespace System.Collections.Generic
 
             if (_array.Length < capacity)
             {
-                EnsureCapacityCore(capacity);
+                Grow(capacity);
                 _version++;
             }
 
             return _array.Length;
         }
 
-        private void EnsureCapacityCore(int capacity)
+        private void Grow(int capacity)
         {
-            Debug.Assert(capacity >= 0);
+            Debug.Assert(_array.Length < capacity);
 
-            if (_array.Length < capacity)
-            {
-                // Array.MaxArrayLength is internal to S.P.CoreLib, replicate here.
-                const int MaxArrayLength = 0X7FEFFFFF;
+            // Array.MaxArrayLength is internal to S.P.CoreLib, replicate here.
+            const int MaxArrayLength = 0X7FEFFFFF;
 
-                int newcapacity = _array.Length == 0 ? DefaultCapacity : 2 * _array.Length;
+            int newcapacity = _array.Length == 0 ? DefaultCapacity : 2 * _array.Length;
 
-                // Allow the list to grow to maximum possible capacity (~2G elements) before encountering overflow.
-                // Note that this check works even when _items.Length overflowed thanks to the (uint) cast.
-                if ((uint)newcapacity > MaxArrayLength) newcapacity = MaxArrayLength;
+            // Allow the list to grow to maximum possible capacity (~2G elements) before encountering overflow.
+            // Note that this check works even when _items.Length overflowed thanks to the (uint) cast.
+            if ((uint)newcapacity > MaxArrayLength) newcapacity = MaxArrayLength;
 
-                // If computed capacity is still less than specified, set to the original argument.
-                // Capacities exceeding MaxArrayLength will be surfaced as OutOfMemoryException by Array.Resize.
-                if (newcapacity < capacity) newcapacity = capacity;
+            // If computed capacity is still less than specified, set to the original argument.
+            // Capacities exceeding MaxArrayLength will be surfaced as OutOfMemoryException by Array.Resize.
+            if (newcapacity < capacity) newcapacity = capacity;
 
-                Array.Resize(ref _array, newcapacity);
-            }
+            Array.Resize(ref _array, newcapacity);
         }
 
         // Copies the Stack to an array, in the same order Pop would return the items.

diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/List.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/List.cs
index dba0781..5bfc45d 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/List.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/List.cs
@@ -213,8 +213,9 @@ namespace System.Collections.Generic
         [MethodImpl(MethodImplOptions.NoInlining)]
         private void AddWithResize(T item)
         {
+            Debug.Assert(_size == _items.Length);
             int size = _size;
-            EnsureCapacityCore(size + 1);
+            Grow(size + 1);
             _size = size + 1;
             _items[size] = item;
         }
@@ -405,7 +406,7 @@ namespace System.Collections.Generic
             }
             if (_items.Length < capacity)
             {
-                EnsureCapacityCore(capacity);
+                Grow(capacity);
                 _version++;
             }
 
@@ -413,27 +414,24 @@ namespace System.Collections.Generic
         }
 
         /// <summary>
-        /// Increase the capacity of this list to at least the specified <paramref name="capacity"/> by continuously twice current capacity.
+        /// Increase the capacity of this list to at least the specified <paramref name="capacity"/>.
         /// </summary>
         /// <param name="capacity">The minimum capacity to ensure.</param>
-        private void EnsureCapacityCore(int capacity)
+        private void Grow(int capacity)
         {
-            Debug.Assert(capacity >= 0);
+            Debug.Assert(_items.Length < capacity);
 
-            if (_items.Length < capacity)
-            {
-                int newcapacity = _items.Length == 0 ? DefaultCapacity : 2 * _items.Length;
+            int newcapacity = _items.Length == 0 ? DefaultCapacity : 2 * _items.Length;
 
-                // Allow the list to grow to maximum possible capacity (~2G elements) before encountering overflow.
-                // Note that this check works even when _items.Length overflowed thanks to the (uint) cast
-                if ((uint)newcapacity > Array.MaxArrayLength) newcapacity = Array.MaxArrayLength;
+            // Allow the list to grow to maximum possible capacity (~2G elements) before encountering overflow.
+            // Note that this check works even when _items.Length overflowed thanks to the (uint) cast
+            if ((uint)newcapacity > Array.MaxArrayLength) newcapacity = Array.MaxArrayLength;
 
-                // If the computed capacity is still less than specified, set to the original argument.
-                // Capacities exceeding MaxArrayLength will be surfaced as OutOfMemoryException by Array.Resize.
-                if (newcapacity < capacity) newcapacity = capacity;
+            // If the computed capacity is still less than specified, set to the original argument.
+            // Capacities exceeding MaxArrayLength will be surfaced as OutOfMemoryException by Array.Resize.
+            if (newcapacity < capacity) newcapacity = capacity;
 
-                Capacity = newcapacity;
-            }
+            Capacity = newcapacity;
         }
 
         public bool Exists(Predicate<T> match)
@@ -695,7 +693,7 @@ namespace System.Collections.Generic
             {
                 ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.index, ExceptionResource.ArgumentOutOfRange_ListInsert);
             }
-            if (_size == _items.Length) EnsureCapacityCore(_size + 1);
+            if (_size == _items.Length) Grow(_size + 1);
             if (index < _size)
             {
                 Array.Copy(_items, index, _items, index + 1, _size - index);
@@ -741,7 +739,10 @@ namespace System.Collections.Generic
                 int count = c.Count;
                 if (count > 0)
                 {
-                    EnsureCapacityCore(_size + count);
+                    if (_items.Length - _size < count)
+                    {
+                        Grow(_size + count);
+                    }
                     if (index < _size)
                     {
                         Array.Copy(_items, index, _items, index + count, _size - index);