Stack<T> optimization of (Try)Peek, (Try)Pop and Push (#26086)

* Stack.Pop RCE

With value types the effect is not so big, because there is still one (manual) check for bounds.
For reference types one bounds-check can be saved, so there is a win.

* Applied same optimizations as for Pop on Peek, TryPeek, TryPop, Push

* Revert change for Push

I can't see a real win, sometimes it's faster and sometimes slower. On Linux the tendency is to be slower.
Therefore the state as is will be remained.

* Stack.Push with hot-/cold-path (PushWithResize)

* Addressed PR feedback

Cf. #26086 (comment)

* Array-copy in Peek is not necessary, JIT can do the same

As of PR-feedback #26086 (comment)

* Reverted b0bfd83

Cf. #26086 (comment)

* Addressed PR feedback

Cf. #26086 (comment)

Author: gfoidl

src/System.Collections/src/System/Collections/Generic/Stack.cs

@@ -202,69 +202,103 @@ public void TrimExcess()
         // is empty, Peek throws an InvalidOperationException.
         public T Peek()
         {
-            if (_size == 0)
+            int size = _size - 1;
+            T[] array = _array;
+
+            if ((uint)size >= (uint)array.Length)
             {
                 ThrowForEmptyStack();
             }
 
-            return _array[_size - 1];
+            return array[size];
         }
 
         public bool TryPeek(out T result)
         {
-            if (_size == 0)
+            int size = _size - 1;
+            T[] array = _array;
+
+            if ((uint)size >= (uint)array.Length)
             {
-                result = default(T);
+                result = default;
                 return false;
             }
-            result = _array[_size - 1];
+            result = array[size];
             return true;
         }
 
         // Pops an item from the top of the stack.  If the stack is empty, Pop
         // throws an InvalidOperationException.
         public T Pop()
         {
-            if (_size == 0)
+            int size = _size - 1;
+            T[] array = _array;
+            
+            // if (_size == 0) is equivalent to if (size == -1), and this case
+            // is covered with (uint)size, thus allowing bounds check elimination 
+            // https://github.com/dotnet/coreclr/pull/9773
+            if ((uint)size >= (uint)array.Length)
             {
                 ThrowForEmptyStack();
             }
 
             _version++;
-            T item = _array[--_size];
+            _size = size;
+            T item = array[size];
             if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())
             {
-                _array[_size] = default(T);     // Free memory quicker.
+                array[size] = default;     // Free memory quicker.
             }
             return item;
         }
 
         public bool TryPop(out T result)
         {
-            if (_size == 0)
+            int size = _size - 1;
+            T[] array = _array;
+
+            if ((uint)size >= (uint)array.Length)
             {
-                result = default(T);
+                result = default;
                 return false;
             }
 
             _version++;
-            result = _array[--_size];
+            _size = size;
+            result = array[size];
             if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())
             {
-                _array[_size] = default(T);     // Free memory quicker.
+                array[size] = default;     // Free memory quicker.
             }
             return true;
         }
 
         // Pushes an item to the top of the stack.
         public void Push(T item)
         {
-            if (_size == _array.Length)
+            int size = _size;
+            T[] array = _array;
+
+            if ((uint)size < (uint)array.Length)
             {
-                Array.Resize(ref _array, (_array.Length == 0) ? DefaultCapacity : 2 * _array.Length);
+                array[size] = item;
+                _version++;
+                _size = size + 1;
             }
-            _array[_size++] = item;
+            else
+            {
+                PushWithResize(item);
+            }
+        }
+        
+        // Non-inline from Stack.Push to improve its code quality as uncommon path
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private void PushWithResize(T item)
+        {
+            Array.Resize(ref _array, (_array.Length == 0) ? DefaultCapacity : 2 * _array.Length);
+            _array[_size] = item;
             _version++;
+            _size++;
         }
 
         // Copies the Stack to an array, in the same order Pop would return the items.