Optimize GC.AllocateUninitializedArray and use it in StringBuilder (#…

…27364)

* use GC.AllocateUninitializedArray for allocating internal char buffers

* force inlining of AllocateUninitializedArray to have no perf hit on the small buffers hot path

* insrease the threshold from 256 to 2048 bytes

* use Unsafe.As instead of a cast

* remove the size precondition from AllocateNewArray method, the called AllocateSzArray is responsible for handling negative size

src/System.Private.CoreLib/shared/System/Text/StringBuilder.cs

@@ -144,7 +144,7 @@ public StringBuilder(string? value, int startIndex, int length, int capacity)
             }
             capacity = Math.Max(capacity, length);
 
-            m_ChunkChars = new char[capacity];
+            m_ChunkChars = GC.AllocateUninitializedArray<char>(capacity);
             m_ChunkLength = length;
 
             unsafe
@@ -182,7 +182,7 @@ public StringBuilder(int capacity, int maxCapacity)
             }
 
             m_MaxCapacity = maxCapacity;
-            m_ChunkChars = new char[capacity];
+            m_ChunkChars = GC.AllocateUninitializedArray<char>(capacity);
         }
 
         private StringBuilder(SerializationInfo info, StreamingContext context)
@@ -242,7 +242,7 @@ private StringBuilder(SerializationInfo info, StreamingContext context)
 
             // Assign
             m_MaxCapacity = persistedMaxCapacity;
-            m_ChunkChars = new char[persistedCapacity];
+            m_ChunkChars = GC.AllocateUninitializedArray<char>(persistedCapacity);
             persistedString.CopyTo(0, m_ChunkChars, 0, persistedString.Length);
             m_ChunkLength = persistedString.Length;
             m_ChunkPrevious = null;
@@ -314,7 +314,7 @@ public int Capacity
                 if (Capacity != value)
                 {
                     int newLen = value - m_ChunkOffset;
-                    char[] newArray = new char[newLen];
+                    char[] newArray = GC.AllocateUninitializedArray<char>(newLen);
                     Array.Copy(m_ChunkChars, 0, newArray, 0, m_ChunkLength);
                     m_ChunkChars = newArray;
                 }
@@ -479,7 +479,7 @@ public int Length
                         {
                             // We crossed a chunk boundary when reducing the Length. We must replace this middle-chunk with a new larger chunk,
                             // to ensure the capacity we want is preserved.
-                            char[] newArray = new char[newLen];
+                            char[] newArray = GC.AllocateUninitializedArray<char>(newLen);
                             Array.Copy(chunk.m_ChunkChars, 0, newArray, 0, chunk.m_ChunkLength);
                             m_ChunkChars = newArray;
                         }
@@ -2423,7 +2423,7 @@ private void ExpandByABlock(int minBlockCharCount)
             }
 
             // Allocate the array before updating any state to avoid leaving inconsistent state behind in case of out of memory exception
-            char[] chunkChars = new char[newBlockLength];
+            char[] chunkChars = GC.AllocateUninitializedArray<char>(newBlockLength);
 
             // Move all of the data from this chunk to a new one, via a few O(1) pointer adjustments.
             // Then, have this chunk point to the new one as its predecessor.
@@ -2569,7 +2569,7 @@ private StringBuilder(int size, int maxCapacity, StringBuilder? previousBlock)
             Debug.Assert(size > 0);
             Debug.Assert(maxCapacity > 0);
 
-            m_ChunkChars = new char[size];
+            m_ChunkChars = GC.AllocateUninitializedArray<char>(size);
             m_MaxCapacity = maxCapacity;
             m_ChunkPrevious = previousBlock;
             if (previousBlock != null)

src/System.Private.CoreLib/src/System/GC.cs

@@ -17,9 +17,7 @@
 using System.Runtime.InteropServices;
 using System.Diagnostics;
 using System.Collections.Generic;
-#if !DEBUG
 using Internal.Runtime.CompilerServices;
-#endif
 
 namespace System
 {
@@ -651,32 +649,32 @@ internal static void UnregisterMemoryLoadChangeNotification(Action notification)
             }
         }
 
-        // Skips zero-initialization of the array if possible. If T contains object references,
-        // the array is always zero-initialized.
+        /// <summary>
+        /// Skips zero-initialization of the array if possible.
+        /// If T contains object references, the array is always zero-initialized.
+        /// </summary>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)] // forced to ensure no perf drop for small memory buffers (hot path)
         internal static T[] AllocateUninitializedArray<T>(int length)
         {
             if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())
             {
                 return new T[length];
             }
 
-            if (length < 0)
-                ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.lengths, 0, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum);
-#if DEBUG
-            // in DEBUG arrays of any length can be created uninitialized
-#else
-            // otherwise small arrays are allocated using `new[]` as that is generally faster.
-            //
-            // The threshold was derived from various simulations.
-            // As it turned out the threshold depends on overal pattern of all allocations and is typically in 200-300 byte range.
-            // The gradient around the number is shallow (there is no perf cliff) and the exact value of the threshold does not matter a lot.
-            // So it is 256 bytes including array header.
-            if (Unsafe.SizeOf<T>() * length < 256 - 3 * IntPtr.Size)
+            // for debug builds we always want to call AllocateNewArray to detect AllocateNewArray bugs
+#if !DEBUG
+            // small arrays are allocated using `new[]` as that is generally faster.
+            if (length < 2048 / Unsafe.SizeOf<T>())
             {
                 return new T[length];
             }
 #endif
-            return (T[])AllocateNewArray(typeof(T[]).TypeHandle.Value, length, zeroingOptional: true);
+            // kept outside of the small arrays hot path to have inlining without big size growth
+            return AllocateNewUninitializedArray(length);
+
+            // remove the local function when https://github.com/dotnet/coreclr/issues/5329 is implemented
+            T[] AllocateNewUninitializedArray(int length)
+                => Unsafe.As<T[]>(AllocateNewArray(typeof(T[]).TypeHandle.Value, length, zeroingOptional: true));
         }
     }
 }

src/vm/comutilnative.cpp

@@ -1131,7 +1131,6 @@ FCIMPL3(Object*, GCInterface::AllocateNewArray, void* arrayTypeHandle, INT32 len
 {
     CONTRACTL {
         FCALL_CHECK;
-        PRECONDITION(length >= 0);
     } CONTRACTL_END;
 
     OBJECTREF pRet = NULL;

tests/src/GC/API/GC/AllocateUninitializedArray.cs

@@ -76,15 +76,37 @@ public static int Main() {
 
         // negative size
         {
+            int GetNegativeValue() => -1;
+            int negativeSize = GetNegativeValue();
+            Type expectedExceptionType = null;
+
+            try
+            {
+                GC.KeepAlive(new byte[negativeSize]);
+
+                Console.WriteLine("Scenario 5 Expected exception (new operator)!");
+                return 1;
+            }
+            catch (Exception newOperatorEx)
+            {
+                expectedExceptionType = newOperatorEx.GetType();
+            }
+
             try
             {
                 var arr = AllocUninitialized<byte>.Call(-1);
 
-                Console.WriteLine("Scenario 5 Expected exception!");
+                Console.WriteLine("Scenario 5 Expected exception (GC.AllocateUninitializedArray)!");
                 return 1;
             }
-            catch (ArgumentOutOfRangeException)
+            catch (Exception allocUninitializedEx) when (allocUninitializedEx.GetType() == expectedExceptionType)
             {
+                // OK
+            }
+            catch (Exception other)
+            {
+                Console.WriteLine($"Scenario 5 Expected exception type mismatch: expected {expectedExceptionType}, but got {other.GetType()}!");
+                return 1;
             }
         }